5

OK, I have some code that seems to work but I'm not sure it will always work. I'm moving a unique_ptr into a stl map using one of the members of the class as the map key, but I'm not sure whether the move might invalidate the pointer in some situations.

The code is along these lines:

struct a
{
    std::string s;
};
std::map<std::string, std::unique_ptr<a>> m;
std::unique_ptr<a> p = std::make_unique<a>();

// some code that does stuff

m[p->s] = std::move(p);

So this currently seems works but it seems to me it might be possible for p to become invalid before the string is used as the map key, and that would lead to a memory exception. Obviously I could create a temporary string before the move, or I could assign via an iterator, but I'd prefer not to if it isn't necessary.

Bill Sellers
  • 251
  • 2
  • 8

2 Answers2

13

This code has well-defined behaviour.

In C++17, std::move(p) will be evaluated before m[p->s]. Before C++17, std::move(p) could be evaluated either before or after m[p->s]. However, this doesn't matter because std::move(p) does not modify p. It is only the assignment that actually causes p to be moved-from.

The assignment operator that is called has the signature

unique_ptr& operator=(unique_ptr&& other);

and is called as if by

m[p->s].operator=(std::move(p));

This means that the modification of p is guaranteed to not take place until the body of operator= is entered (the initialization of the other parameter is merely a reference binding). And certainly the body of operator= cannot be entered until the object expression m[p->s] is evaluated.

So your code is well-defined in all versions of C++.

Brian Bi
  • 91,815
  • 8
  • 136
  • 249
5

The code is fine. In C++ 17, we were given strong guarantees on the sequencing, which makes this code 100% OK.

Prior to C++17 the standard has

In all cases, the assignment is sequenced after the value computation of the right and left operands, and before the value computation of the assignment expression.

But that still means the code is okay. We don't know which of m[p->s] and std::move(p) happens first, but since move does't actually do anything to p, p->s will be valid and be resolved before p is moved into m[p->s]

NathanOliver
  • 150,499
  • 26
  • 240
  • 331
  • This quotation applies to built-in assignment operators, does it not? – chris Apr 03 '20 at 16:15
  • @chris That specific paragraph applies to all types. – NathanOliver Apr 03 '20 at 16:17
  • @idclev463035818 Right, `std::move` is just a cast, so it has no effect on what happens to `p->s`. What the quote does have in it though, is it guarantees that `std::move(p)` and `m[p->s]` will both be evaluated **before** the actual assignment happens (the actual move) – NathanOliver Apr 03 '20 at 16:21
  • I see the context in the standard now. It appears that cppreference has taken the text to mean otherwise, which intrigues me. – chris Apr 03 '20 at 16:24