Is it possible to erase modified element from std::set?

Question

I need a sorted collection where elements can be modified. Is it safe to erase element after modification? Sorting key can be modified.

auto it=s.find(e)
modify(e)
s.erase(it)

I have made some tests in VS2010, and it worked. I think erase(it) does not need to search for element, so there is no need to call compare on element being erased.

It is hard to modify whole program to remove elements before modification, that is why I am looking for an alternative solution.

EDIT: adding working sample to make it more clear

#include <iostream>
#include <algorithm>
#include <set>

template <typename T>
struct PtrCmp
{
    bool operator()(const T* x, const T* y) const
    {
        return *x<*y;
    }
};

int main()
{
    std::set<int*, PtrCmp<int>> aset;
    int t[]={1,2,3,4};
    for(int i=0;i<4;++i)
    aset.insert(&t[i]);

    auto it=aset.find(&t[2]);
    t[2]=5;
    aset.erase(it);

    for(auto it=aset.begin(); it!=aset.end(); ++it)
        std::cout<<**it<<std::endl;
}

What's `e`? Why would the set be affected when you modify it? — chris, Jul 14 '15 at 07:51
Should your code say `modify(*it)`? `modify(e)` won't affect the set at all, since you aren't changing the copy of `e` that the set contains. — Aakash Jain, Jul 14 '15 at 07:54
e may be pointer to element in set, I can post whole working sample program instead of such fragment, if it could help — Roman, Jul 14 '15 at 07:54
@Roman `set::find` does not take an argument of pointer to element type. Maybe you should post some actual code. — M.M, Jul 14 '15 at 07:57

Serge Rogatch · Answer 1 · 2015-07-14T10:23:09.923

1

In your example t[2]=5; modifies the value which is used in comparator which is used e.g. for rebalancing the tree data structure behind the set container. Therefore such modification is not safe because the balanced-tree algorithm may fail in case the factual key at a node is changed, therefore does not match the expectation for that tree node. erase operation is likely to trigger tree rebalancing, and that is how you get undefined behavior. So by modification of a value used in comparator you factually break the balancedness of the tree behind a set container.

edited Jul 14 '15 at 10:23

answered Jul 14 '15 at 08:15

Serge Rogatch

11,119
4
58
117

I modify the value, which is used in comparator for set sorting. Is it still safe? – Roman Jul 14 '15 at 10:03
@Roman, ah, I missed that custom comparator. Then no, it is not safe because erase() will likely use the value for rebalancing the tree behind the set. – Serge Rogatch Jul 14 '15 at 10:19
@Roman, I've adjusted my answer taking into account the effects of using the custom comparator. – Serge Rogatch Jul 14 '15 at 10:25
I have made more testing, it works on VS2010, gcc4.4 and stlport. erase(it) does not call comparator, it does not need it to erase element. I may stay with stlport implementation, where I verified sources and I know it works correctly. – Roman Jul 14 '15 at 12:26
@Roman, the situation in which comparator is called may be hard to reproduce. To reproduce it, you need a case in which rebalancing of the tree is needed in order to or as a result of `erase`. Different `set` (tree) sizes tried (1-1000 nodes) and different nodes deleted make it more likely that the case of interest is triggered. – Serge Rogatch Jul 14 '15 at 12:50

score 0 · Answer 2 · answered Jul 14 '15 at 07:55

modify(e) (maybe) doesn't modify the set. My guess is that e is something outside the set, and that s.find(e) searches within the set for (a copy of) e. Basically, your code is equivalent to:

auto it=s.find(e)
s.erase(it)
modify(e)

And so there won't be any problem.

Unless e is actually a raw pointer to data.

score 0 · Answer 3 · answered Jul 14 '15 at 07:55

Element in a std::set<T> are considered const. If you change an element, e.g., by having a mutable member or a pointer to some internal data and chnage the key, the invariants for std::set<T> are violated and you'll get undefined behavior when accessing the std::set<T> in any way.

score 0 · Answer 4 · answered Jul 14 '15 at 07:55

The element in the set doesn't (or shouldn't) have anything to do with e. If you modify e and it doesn't affect the set element, you're fine.

If the modification does affect the set element in a way that modifies its sort order, you've technically got undefined behavior at this very moment, and the erase afterwards is no longer required to work.

See C++11 23.2.4 paragraphs 4 and 5.

score 0 · Answer 5 · edited May 23 '17 at 12:21

0

It is safe unless your modify() does any operation that invalidates the iterator. If it cannot access the svariable, there is no problem.

edited May 23 '17 at 12:21

Community

1
1

answered Jul 14 '15 at 07:56

Melebius

4,692
3
32
43

The first part is wrong C++11 onwards. Modifying the contents in a set isn't allowed even when it doesn't affect the element's position in the set. – Aakash Jain Jul 14 '15 at 07:58
@aakashjain I haven’t seen any sign of that the question is about C++11 or newer. For example, `gcc` still uses C++98 by default. – Melebius Jul 14 '15 at 08:04
...the question uses the `auto` keyword – Aakash Jain Jul 14 '15 at 08:09

score 0 · Answer 6 · answered Jul 14 '15 at 08:08

0

Your code is correct. The set contains four pointers. It does not contain the things being pointed to by those pointers.

t[2] = 5; does not modify the set in any way.

answered Jul 14 '15 at 08:08

M.M

130,300
18
171
314

1

it modifies the key used in sorting – Roman Jul 14 '15 at 08:18

score 0 · Answer 7 · edited May 23 '17 at 12:14

Verbalise it:

                  // I have this key 'e'...
auto it=s.find(e) // I look through my set for another like 'e'
                  // and 'it' now points to it
modify(e)         // now I 'invert' e.
s.erase(it)       // and I remove object pointed to by 'it'

*it and e are two different objects.

In general, the object pointed to by an STL set/map iterator and an object used as a search key are different objects. So, you would still be working on a copy of the container's object.

Now your other questions boil down to two:

Can I modify the object pointed to by itr? Would it affect sorting order?
- Theoretically possible. The contents of a set are const and the iterator doesnt allow direct assignment -
```
test.cpp10:9: error: read-only variable is not assignable.
*it = 4;
~~~ ^
```
  - but using mutable or a combination of const_cast and pointer-magic, it is possible to modify const-data. Is it dangerous? You betcha. Each container has its own set of assumptions that its API leaves invariant. So, a general rule - NEVER - do stuff to an STL container that its API doesn't allow.
Can I get a sorted container whose elements can be modified?
- Of course! You just do it outside the container and put it back.
```
auto it=s.find(e);
// possible optimisation (see below)
s.erase(it);
modify(e);
auto position = s.insert(e);
if (position.first) // no existing values matching e's comparison!
     assert(*position.second.key == e.key);
```
  With C++11, depending on what 'e' is, you can use move-semantics to move non-comparison elements from *it to e, in tandem with set emplace to optimise the number of copies done.

score 0 · Answer 8 · answered Jul 14 '15 at 10:08

In your sample, you modify the ordering of the set and so have undefined behaviour (and 'working as wanted' is one possible behaviour :-/).

You may use the following to have correct code:

auto it = aset.find(&t[2]);
assert(it != aset.end()); // or any other error check methods
aset.erase(it);
t[2] = 5; // No longer used by set

or

auto it = aset.find(&t[2]);
assert(it != aset.end()); // or any other error check methods
auto* ref = *it;
aset.erase(it);
assert(ref != nullptr); // or any other error check methods
*ref = 5; // No longer used by set.

Is it possible to erase modified element from std::set?

8 Answers8