unordered_map::find() and two iterators

Question

Having a class with a private member

std::unordered_map<std::string, size_t> myMap;

and the corresponding getter

std::unordered_map<std::string, size_t> getMyMap() const {return myMap;}

I observe strange behaviour by applying std::unordered_map::find() two times, each time saving the returned iterator, e.g.

auto pos1 = test.getMyMap().find("a");
auto pos2 = test.getMyMap().find("a");

Altough I look for the same key "a" the iterator points to different elements. The following sample code illustrates the problem:

#include <iostream>
#include <unordered_map>
#include <vector>
#include <string>

class MyMap{
 public:
  MyMap(){
    myMap= {
      {"a", 1},
      {"b", 2}
    };
  }

  std::unordered_map<std::string, size_t> getMyMap() const {return myMap;}

private:
  std::unordered_map<std::string, size_t> myMap;
};

int main(){

  MyMap test;

  auto pos1 = test.getMyMap().find("a");
  auto pos2 = test.getMyMap().find("a");
  std::cout << pos1->first << "\t" << pos1->second << std::endl;
  std::cout << pos2->first << "\t" << pos2->second << std::endl;
}

Compiling with g++ -std=c++11 and running gives

b   2
a   1

where the first line is unexpected. It should be "a 1".

Changing the code to

  auto pos3 = test.getMyMap().find("a");
  std::cout << pos3->first << "\t" << pos3->second << std::endl;
  auto pos4 = test.getMyMap().find("a");
  std::cout << pos4->first << "\t" << pos4->second << std::endl;

results in the correct output

a   1
a   1

Furthermore just creating an unordered_map in the main file and applying find() works. It seems the problem is connected to the getter-method, maybe to return-value-optimisation. Do you have any explanations for this phenomena?

Answer 1

It's because you have undefined behavior in your code. The getMyMap returns a copy of the map, a copy that is destructed once the expression test.getMyMap().find("a") is done.

This means you have two iterators that are pointing to no longer existing maps.

The solution is very simple: Make getMyMap return a constant reference instead:

std::unordered_map<std::string, size_t> const& getMyMap() const;

---

That it seems to work in the latter case, is because that's a pitfall of undefined behavior, it might sometime seem like it works, when in reality it doesn't.

Answer 2

test.getMyMap().find("a"); does find on a copy of original myMap which is destructed after the expression is complete, making the iterators pos1 and pos2 to non-existing map, invoking an undefined behaviour

Instead you can play around like following :

  auto mymap = test.getMyMap() ;   // Store a copy

  auto pos1 = mymap.find("a");    // Then do stuff on copy
  auto pos2 = mymap.find("a");