C++ undefined behavior on unordered_map as an rvalue in a range-based for loop

I hate to add to the myriad of questions about undefined behavior on stack overflow, but this problem has mystified me.

When using a range-based for loop on an unordered_map that is created inline, I get an unexpected result. When using the same loop on an unordered_map that was first assigned to a variable, I get the expected result.

I would expect both loops to print 1, but that is not what I observe.

Any help understanding what is happening would be appreciated. Thanks!

I'm running on Debian 10 with g++ 8.3.0

#include <algorithm>
#include <unordered_map>
#include <iostream>
#include <vector>

int main() {

        for (const int i : std::unordered_map<int, std::vector<int>> {{0, std::vector<int> {1}}}.at(0)) {
                std::cout << i << std::endl; //prints 0
        }

        std::unordered_map<int, std::vector<int>> map {
                {0, std::vector<int> {1}}
        };

        for (const int i : map.at(0)) {
                std::cout << i << std::endl; //prints 1
        }

}

Solution 1:

The problem lies in the fact that you create a temporary std::unordered_map and return a reference to one of its contents. Let's inspect two behaviours that occur here:

1. Range-based for expansion:

From the question you linked in the comments we can see that the following syntax:

for ( for-range-declaration : expression ) statement

translates directly to the following:

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

The important part is to understand that when you either create a temporary or provide an lvalue, a reference to it will be created (auto&& __range). If we're dealing with lvalues, we encouter the results we expect. However, when range-init returns a temporary object, things get a little more interesting. We encounter lifetime extension.

2. Lifetime extension:

This is way simpler than it may seem. If you return a temporary object to initialize (bind) a reference to it, the lifetime of said object is extended to match the lifetime of said reference. That means that if range-init returns a temporary, saving a reference to it (__range) extends the lifetime of that temporary up to the last curly bracket of the code I copy-pasted above. That's the reason for those most-outer brackets.

Your case:

In your case, we have quite a tricky situation. Inspecting your loop:

for (const int i : std::unordered_map<int, std::vector<int>> {{0, std::vector<int> {1}}}.at(0)) {
    std::cout << i << std::endl;
}

We must acknowledge two things:

1. You create a temporary std::unordered_map.
2. range-init is not referring to your std::unordered_map. It refers to what .at(0) retuns - a value from that map.

This results in the following consequence - the lifetime of your map is not extended. That means that its destructor will be called at the end of the full expression (at the ; from the auto && __range = range-init;). When std::unordered_map::~unordered_map is called, it calls destructors of everything it manages - e.g. its keys and values. In other words, that destructor call will call the destructor of the vector you obtained a reference to via the at(0) call. Your __range is now a dangling reference.