0

I know that erasing elements from an associative container inside a for loop invalidates it.

Is it the case when using a range based loop?

#include <iostream>
#include <unordered_map>
#include <set>
#include <algorithm>
#include <ranges>

struct A {
    int value;
    // other members...
};

int main() {
    std::unordered_map<std::string, std::set<A>> myMap;

    // Assuming you have populated the map and the set

    // Object of type A
    A a;

    for (auto& [key, valueSet] : myMap) {
        valueSet.erase(a);

        if (valueSet.empty()) {
            myMap.erase(key);
        }
    }

    return 0;
}

And how can the compiler detects it.

I run -fsanitize=address,undefined but gcc doesn't show any warning.

Amolgorithm
  • 697
  • 2
  • 20
Peter
  • 109
  • 7
  • 2
    When an operation invalidates iterators in a container, whether the operation was invoked from a loop, or not, or what kind of a loop it is, makes no difference whatsoever. Iterator invalidation does not depend on the calling context. The only issue is whether any iterators in the calling context are now invalidated. This is determined by understanding which, and all, iterators get invalidated, and whether the iterators in the calling context fit within the scope of the invalidated iterators. – Sam Varshavchik Aug 02 '23 at 18:14
  • Thank you for your quick reply, How to detect this with GCC ? – Peter Aug 02 '23 at 18:31
  • Some things are just too insidious for a robot to detect. – user4581301 Aug 02 '23 at 18:36
  • Unfortunately, to "detect this with GCC" will require defeating some fundamental laws of physics of our shared universe. Yesterday some code was compiled, which used a couple of iterators, called some function, and used those iterators a bit more afterwards. Today you compiled a different translation unit, where that function is defined, and the modified code now invalidates all the existing iterators to a container. Unfortunately, those iterators are still used by the code that was compiled yesterday, but the suffering compiler has completely forgot everything that was compiled yesterday. – Sam Varshavchik Aug 02 '23 at 18:41
  • 2
    You may loop by iterator and use `it = myMap.erase(key)` to get the next valid iterator. `++` an erased iterator is invalid. Besides, you may use `std::erase_if` in this case. – o_oTurtle Aug 03 '23 at 04:11

1 Answers1

1

Is it the case when using a range based loop?

Generally yes, a range-based for loop is just using iterators under the hood. It effectively expands to:

{   // for (init-statement; range-declaration : range-expression) { ... }
    /* init-statement */
    auto && __range = /* range-expression */;
    auto __begin = /* begin-expr */;
    auto __end = /* end-expr */;
    for ( ; __begin != __end; ++__begin) {
        /* range-declaration */ = *__begin;
        /* ... */
    }
}

Any operation which invalidates iterators might also interfere with a range-based for loop as a result.

std::unordered_map::erase invalidates the iterator to the erased element. In your case, erase(key) is unsafe, because it invalidates the __begin iterator which is used under the hood. ++__begin would then advance an invalidated iterator, which is undefined behavior.

-fsanitize=address,undefined does catch this type of mistake. (See live example at Compiler Explorer). It is a heap-use-after-free. Are you sure that -fsanitize is even supported on your platform? Maybe it doesn't do anything because you're using MinGW and an old version of GCC.

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96