Skip to content
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

C.21 Exception for copy-and-swap idiom #2164

Open
Eisenwave opened this issue Dec 22, 2023 · 4 comments
Open

C.21 Exception for copy-and-swap idiom #2164

Eisenwave opened this issue Dec 22, 2023 · 4 comments

Comments

@Eisenwave
Copy link
Contributor

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:

@sergio-nsk
Copy link

sergio-nsk commented Dec 30, 2023

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.

@Eisenwave
Copy link
Contributor Author

Eisenwave commented Dec 31, 2023

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.

@sergio-nsk
Copy link

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.

@Eisenwave
Copy link
Contributor Author

Eisenwave commented Jan 1, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants