CppCoreGuidelines icon indicating copy to clipboard operation
CppCoreGuidelines copied to clipboard

C.21 Exception for copy-and-swap idiom

Open Eisenwave opened this issue 1 year ago • 4 comments

C.21 is missing an exception for the copy-and-swap idiom. It's perfectly safe (and considered idiomatic by many) for resource management to write:

T& operator=(T) noexcept { /* ... */ }

This would violate C.21 since technically, the user has only defined the copy assignment operator, and the move assignment operator is implicitly not declared.

The lack of an exception to this rule also manifests itself in clang-tidy false positives. See:

  • https://github.com/llvm/llvm-project/issues/76064
  • https://stackoverflow.com/questions/77692186/clang-tidy-does-not-recognize-unifying-assignment-operator-as-move-assigment

Eisenwave avatar Dec 22 '23 22:12 Eisenwave

I don't think it is worth to be an exception in C.21. Such the copy-and-swap idiom should be avoided: it performs copy and swap for lvalues and move and swap for rvalues, that is not optimal. And additional calls to clear() and shrink_to_fit() after swap() may be required.

sergio-nsk avatar Dec 30 '23 17:12 sergio-nsk

And additional calls to clear() and shrink_to_fit() after swap() may be required.

No, they are not. The other object gets copied or moved-from just to pass the T parameter. this then gets swapped with that parameter, whose lifetime is at most the surrounding full-expression. One only has to clear() afterwards if this gets swapped with a T&& parameter, but that is not the case. The following implementation is sufficient:

T& T::operator=(T other) noexcept {
    swap(other);
    return *this;
}

Also, while I'm not a huge fan of this kind of assignment operator myself, if we're not going to add an exception to C.21 for this, then we should add a rule which advises against such assignment operators in the first place. The current wording just says nothing on this, making it unclear how the corresponding clang-tidy rule should be implemented.

Eisenwave avatar Dec 31 '23 09:12 Eisenwave

If T::T(T&&) is missing, and std::move(t) is passed, then T other is copy constructed: possible 2x memory consumption and unexpected non empty state of t observed after the assignment expression.

sergio-nsk avatar Jan 01 '24 01:01 sergio-nsk

If T::T(T&&) is missing, and std::move(t) is passed, then T other is copy constructed

Yes, but why would the move constructor be missing? That would still be a violation of C.21 and I don't seek to change that, so I don't see how this point is relevant.

This reminds me that an exception to C.60: Make copy assignment non-virtual, take the parameter by const&, and return by non-const& would also be necessary though, and perhaps a note on C.62 that operator=(T) makes self-assignment safe trivially.

Eisenwave avatar Jan 01 '24 07:01 Eisenwave