Skip to content

C.65 Self-move - require only valid but unspecified #1938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 16 commits into from
57 changes: 26 additions & 31 deletions CppCoreGuidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -6215,50 +6215,45 @@ Unless there is an exceptionally strong reason not to, make `x = std::move(y); y

##### Reason

If `x = x` changes the value of `x`, people will be surprised and bad errors can occur. However, people don't usually directly write a self-assignment that turn into a move, but it can occur. However, `std::swap` is implemented using move operations so if you accidentally do `swap(a, b)` where `a` and `b` refer to the same object, failing to handle self-move could be a serious and subtle error.
People don't usually directly write a self-move assignment, but it can occur. `std::swap` is implemented using
move operations so if you accidentally do `swap(a, b)` where `a` and `b` refer to the same object, failing to
handle self-move could be a serious and subtle error. At a minimum, a self-move should put the object into a
valid but unspecified state which is the same guarantee provided by the standard library containers. It is
not required to leave the object unchanged.

##### Example

class Foo {
string s;
int i;
If all of the members of a type are safe for move-assignment, the default generated move-assignment
operator will be safe too.

X& operator=(X&& a) = default;

Otherwise, the manually written move-assignment operator must be made safe for self-assignment

class X {
public:
Foo& operator=(Foo&& a);
X();
X& operator=(X&& a) noexcept;
// ...
~X() { delete m_owning; }
private:
T* m_owning; // bad (See R.20) but used in the example because
// it requires a manual move-assignment
};

Foo& Foo::operator=(Foo&& a) noexcept // OK, but there is a cost
X& X::operator=(X&& a) noexcept
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used 'X' instead of 'Foo' to match C.64

{
if (this == &a) return *this; // this line is redundant
s = std::move(a.s);
i = a.i;
auto* tmp = a.m_owning;
a.m_owning = nullptr;
delete m_owning; // null in the case of a self move
m_owning = tmp;
return *this;
}

The one-in-a-million argument against `if (this == &a) return *this;` tests from the discussion of [self-assignment](#Rc-copy-self) is even more relevant for self-move.

##### Note

There is no known general way of avoiding an `if (this == &a) return *this;` test for a move assignment and still get a correct answer (i.e., after `x = x` the value of `x` is unchanged).

##### Note

The ISO standard guarantees only a "valid but unspecified" state for the standard-library containers. Apparently this has not been a problem in about 10 years of experimental and production use. Please contact the editors if you find a counter example. The rule here is more caution and insists on complete safety.

##### Example

Here is a way to move a pointer without a test (imagine it as code in the implementation a move assignment):

// move from other.ptr to this->ptr
T* temp = other.ptr;
other.ptr = nullptr;
delete ptr;
ptr = temp;

##### Enforcement

* (Moderate) In the case of self-assignment, a move assignment operator should not leave the object holding pointer members that have been `delete`d or set to `nullptr`.
* (Not enforceable) Look at the use of standard-library container types (incl. `string`) and consider them safe for ordinary (not life-critical) uses.
* (Moderate) In the case of self-assignment, a move assignment operator should not leave the object holding pointer members that have been deleted.
* (Not enforceable) Look at the use of standard-library container types (including `string`) and consider them safe for ordinary (not life-critical) uses.

### <a name="Rc-move-noexcept"></a>C.66: Make move operations `noexcept`

Expand Down