2388. Handling self-assignment in the proposed library function std::exchange

Section: 22.2.3 [utility.exchange] Status: NAD Submitter: Nick Calus Opened: 2014-05-09 Last modified: 2015-05-05

Priority: 2

View all other issues in [utility.exchange].

View all issues with NAD status.

Discussion:

In paper N3668, the addition of a template function std::exchange had been proposed. In the rationale provided by the paper, we find the following:

I chose the name for symmetry with atomic_exchange, since they behave the same except for this function not being atomic.

and:

Atomic objects provide an atomic_exchange function ([atomics.types.operations.req]p18) that assigns a new value to the object and returns the old value. This operation is also useful on non-atomic objects, and this paper proposes adding it to the library.

But the specified semantics of std::exchange is defined as follows:

template <class T, class U=T> T exchange(T& obj, U&& new_val);

Effects: Equivalent to:

T old_val = std::move(obj);
obj = std::forward<U>(new_val);
return old_val;

When looking at the post-condition of the std::exchange function, one would expect the return value to be the old value of obj and also that obj now contains the value of new_value. This post-condition is violated when obj is a reference to the same object as new_value and type T has move semantics.

Given it's specification, it is clear that std::exchange is meant to be used with types that have move semantics. Therefore, the post-condition is violated for self-assignments.

Suppose the following situation:

You have a vector of objects. The objects implement move semantics and are emptied when moved from. You provide a function that allows you to replace an object at a specific index by a new object (provided by reference as an argument to your function). When replacing an object, your function calls a member-function do_something_fancy on the old object.

void your_function(int i, X& new_val) {
  std::exchange(vec[i], new_val).do_something_fancy();
}

Your function gets called with a given index and the corresponding element of said vector. (by coincidence or by purpose, it doesn't really matter)

your_function(5, vec[5]);

This will cause the object at vec[5] to be in an empty state. If this object would not implement move semantics, assignment performance is potentially worse, but at least it is not in an empty (to my business logic, invalid) state.

So to me, the current reference implementation of std::exchange does not have the behavior it is expected to have.

[2014-12-18 Telecon]

MC: does this resolution solve the problem?

JW: and is the cost of the extra construction and move acceptable?

AM: not all moves are cheap

VV: seems like a design change

JW: maybe this should be rolled into my unwritten paper on self-swap, so we deal with them consistently

VV: we should update the issue saying something like that and maybe NAD Future

MC: instead, add Requires clause saying the arguments are not the same.

JW: interesting, that can even be checked in a debug mode assertion

MC: ACTION: send alternative P/R that we can consider

Previous resolution [SUPERSEDED]:

This wording is relative to N3936.

1. Change 22.2.3 [utility.exchange] as indicated:

   template <class T, class U=T> T exchange(T& obj, U&& new_val);
   

   -1- Effects: Equivalent to:

   T tmp = std::forward<U>(new_val);
   T old_val = std::move(obj);
   obj = std::forward<U>(new_val)std::move(tmp);
   return old_val;
   

Previous resolution from Marshall [SUPERSEDED]:

This wording is relative to N4296.

1. Change 22.2.3 [utility.exchange] as indicated:

   template <class T, class U=T> T exchange(T& obj, U&& new_val);
   

   -?- Requires: obj and new_val shall not refer to the same object.

   -1- Effects: Equivalent to:

   T old_val = std::move(obj);
   obj = std::forward<U>(new_val);
   return old_val;
   

[2015-03-30, Marshall provides alternative wording]

[2015-05, Lenexa]

MC: self exchange does not work
MC: PR is just to add requires
STL: what if the new thing is a subobject, isn't that just as bad, any aliasing
STL: don't know that we need to do anything here if we aren't changing the implementation
NM: should remove the requires
MC: so NAD
STL: could add note
NM: remove requires
DK: requires isn't already there
RL: no note?
STL: no note, NAD, burden for next person that asks about aliasing
DK: what do we do for swap?
STL: self swap has always been noop, exchange could do something bad because it clears out
MC: alright, NAD it is

Proposed resolution: