Bug in C++ Set?

Question

When I tried this part of the code:

int x=*(s.rbegin());
while(!s.empty()&&0<x)
{
    s.erase(x);
    x=*(s.rbegin());
}

It runs into an infinite loop because it doesn't actually erase anything when I call erase. This seems weird because *(s.rbegin()) should definitely be in s.

You have undefined behavior after the last value of the set is erased, and then you dereference an iterator from it. Unless you can guarantee that the `0 — Benjamin Lindley, Jan 28 '17 at 01:46
What Benjamin Lindley said; segfaults for me on last item (but doesn't go in infinite loop. You're going to need to provide a bit more details on your c++ version and the rest of your code (e.g. what is s containing? ) — Foon, Jan 28 '17 at 01:53
When `erase()` is used to remove the last element in the set (after which the set is empty), `s.rbegin()` will be equal to `s.rend()`, and evaluating `x` then gives undefined behaviour. — Peter, Jan 28 '17 at 02:27

Swift - Friday Pie · Answer 1 · 2017-01-28T02:07:27.283

Change order of actions, you delete element you wanted to get.. for example:

 std::set<int> s;

 int x;
 s.insert(5);
 s.insert(15);
 s.insert(25);
 s.insert(0);
 s.insert(20);
 while(!s.empty()&& (0<(x=*s.rbegin())))
 {
     std::cout<< x << "\n";
     s.erase(x);
 }

Be aware that in way you defined while. Output of code above will be

Be aware that in way you defined while() loop you intended to stop if it finds 0 element. There is 0 left after the exit, set will not be empty. Traditionally used iteration looks like

for (auto it=s.rbegin(); it!=s.rend(); ++it)
{
}

Remy Lebeau · Answer 2 · 2017-01-28T04:43:25.247

0

Try this instead:

if (!s.empty())
{
    int x = *(s.rbegin());
    while (0 < x)
    {
        s.erase(x);
        if (s.empty()) break;
        x = *(s.rbegin());
    }
}

Alternatively, try erasing using iterators instead of values:

std::set<int>::reverse_iterator iter = s.rbegin();
while ((iter != s.rend()) && ((0 < *iter))
    s.erase((iter++).base());

edited Jan 28 '17 at 04:43

answered Jan 28 '17 at 03:24

Remy Lebeau

555,201
31
458
770

std::remove_if will not work with std::set, you'll get assignment of read-only location error. last compiler that could do that of which I know was vc6 (due to some unusual implementations of STL) – Swift - Friday Pie Jan 28 '17 at 04:20

score 0 · Answer 3 · answered Jan 28 '17 at 09:30

It seems that you are trying to erase all elements in the set which are positive. This can be done much simpler with this single line:

s.erase(s.upper_bound(0),s.end());

upper_bound gives you the iterator to the first number greater than 0. Since all elements from this one to the end are guaranteed to be greater than zero, you can erase them all.

score -1 · Answer 4 · answered Jan 28 '17 at 01:53

-1

You should check for the end of set before x=*(s.rbegin());

int x=*(s.rbegin());
while(!s.empty() && 0<x)
{
    s.erase(x);
    if(!s.empty())
        x=*(s.rbegin());
}

answered Jan 28 '17 at 01:53

cpatricio

457
1
5
11

"*check for the end of set*" - that applies to the first `rbegin()` as well before entering the loop, not just the `rbegin()` inside the loop. – Remy Lebeau Jan 28 '17 at 03:18

Bug in C++ Set?

4 Answers4