Is it well-defined to modify an unordered_set inside a foreach loop if one breaks out immediately after?

Question

Consider the following program. The loop in the middle attempts to replace exactly one item with one other item, and then breaks out of the loop.

#include <unordered_set>
#include <stdio.h>

int main(){
    std::unordered_set<int> foo{1,2,3};
    printf("Set Before:\n");
    for (int x : foo)
        printf("%d\n", x);
    for (int x : foo) {
        if (x == 1) {
            foo.erase(1);
            foo.insert(4);
            break;
        }
    }
    printf("Set After:\n");
    for (int x : foo)
        printf("%d\n", x);
}

Is the code above well-defined?

Answer 1

Is the code above well-defined?

Yes. The erase would invalidate the iterator you're on right now, which would make the subsequent increment of it undefined behavior - but there is no subsequent increment, since you're breaking unconditionally.

---

Although instead of looping over each element until you find 1, you can just try to erase it and see if that did anything:

if (foo.erase(1)) {
    foo.insert(4);
}

Answer 2

for(type var:target) loops are defined as being equivalent to:

{
  auto&& __target = target;
  auto&& __start = __magic_begin(__target);
  auto&& __finish = __magic_end(__target);
  for (; __start != __finish; ++__start) {
    type var = *__start;
    __body_of_loop_here__
  }
}

where __magic_begin is a magic function that does some stuff to find the begin iterator, whose details do not matter here. (also, __ prefixed names are just for illustration purposes).

As your code is defined with the above transformation, it is defined without it.

Most things in the standard are not this clear cut; this one really is.

There are even bugs caused by the above transformation. For example, if target has temporaries that aren't the result of the expression, their lifetime ends before the __start iterator is created.