add noexcept on assignment operators, and a test check those #91
Conversation
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
- they are no better than what the compiler will do to perform such assigments without explicit operators - construct a temporary variant using conversion constructor, and move-assign from the temporary
|
Note, that in C++11 and later destructors marked by |
|
Shall I rebase this on master? I sooo much hate merge conflicts 🍝 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
You can check how the test ran on master here: https://travis-ci.org/lightmare/variant/builds/106630852
With this patch Travis/AppVeyor will still fail, that's intended.
Except for Xcode6 having some issue with
is_nothrow_move_constructible, all other builds on travis will have only 2 failedis_nothrow_destructiblechecks. Those will go away once~variant()is declarednoexcept(unconditionally), but I didn't wan't to touch that line so that it doesn't conflict with reverting previous commits or any other fix to the destructor issue.AppVeyor has more than 2 failing
is_nothrow_destructible, that might mean it's indeed broken on MSVC and so the checks will need to be skipped/removed.Note: I removed private functions
copy_assignandmove_assign-- the names were misleading, they did destroy&construct, not assign -- and copied their bodies to respective operators.