Support assignment and swap for a facade that forbids relocation - #77
Open
mingxwa wants to merge 2 commits into
Open
Support assignment and swap for a facade that forbids relocation#77mingxwa wants to merge 2 commits into
mingxwa wants to merge 2 commits into
Conversation
proxy::operator=(const proxy&) commits a throwing copy through a temporary, which needs a move assignment. A facade that forbids relocation has none, so the expression re-selected the copy assignment itself and recursed until the stack overflowed. proxy::operator=(P&&) reaches the same expression and inherited the crash, and failed to compile outright for a facade that is not copyable either. Stage the temporary only where an assignment exists to commit it with, and destroy the contained value before constructing the new one otherwise. Assignment then works for every facade, at the cost of leaving *this without a value when the construction throws, which is the best a facade that can neither relocate nor copy without throwing can offer. Take the staged form whenever the commit cannot throw, which is copyability >= nothrow as much as relocatability >= nontrivial. A facade with nothrow copyability keeps its old value when the construction throws, as one with a trivial copy assignment already did.
proxy::swap relocates, so a facade that forbids relocation had no swap at all unless it was trivially copyable, even though std::swap already exchanged such proxies through the copy constructor and the copy assignment, both of which bind an rvalue. The member was missing for types the standard library already reports as swappable. Add an overload for exactly those facades, constrained on relocatability == none, that exchanges the values by copying. Splitting it from the relocating overload rather than branching inside one keeps both noexcept specifications honest: the relocating overload is untouched, and the new one is noexcept when the copies and the destructions are. An empty operand is exchanged with a single copy rather than through a temporary. A facade that can relocate keeps relocating, even where the relocation can throw and a copy could not. Exchanging by copy is what a facade with no other way gets, not a path taken from one that has one. Documented the constraint the hidden friend has always carried.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
proxy::operator=(const proxy&)recursing until the stack overflows for a facade that forbids relocation. Staging a throwing copy in a temporary needs a move assignment, which such a facade does not have.proxy::operator=(P&&)inheriting that recursion, and failing to compile when the facade is not copyable either. It now stages only where an assignment can commit the temporary, and does so whenever that commit cannot throw, which keeps the old value for a nothrow copyable facade.proxy::swapoverload for facades that forbid relocation, exchanging by copy.std::swapalready did this through the copy constructor and copy assignment, so the member was missing for typesstd::is_swappable_vreports as swappable. The relocating overload is untouched.swaphas always carried, and the states a throwing exchange can leave.