assignment operator: check if op can throw or not, and create a temp object if it can to keep strong exc guarantee #12
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
… it can to keep strong exc guarantee
… it can to keep strong exc guarantee - fix unit tests
david-grs
commented
Feb 23, 2018
| { | ||
| constexpr const bool canThrow = | ||
| (std::is_rvalue_reference<_T&&>::value && std::is_nothrow_move_constructible<_T>::value) | ||
| || (std::is_lvalue_reference<_T&&>::value && std::is_nothrow_copy_constructible<_T>::value); |
Owner
Author
There was a problem hiding this comment.
Now I review my own code I see the condition should be the other way around - I forgot the negation in fromt of the is_nothrow... I will change that.
| detail::static_any::throw_tag, | ||
| detail::static_any::nothrow_tag>::type; | ||
|
|
||
| assign(std::forward<_T>(t), ThrowTag{}); |
Contributor
There was a problem hiding this comment.
I can see how constexpr if could be useful here
| CallCounter& operator=(CallCounter&&) { ++move_constructions; return *this; } | ||
| ~CallCounter() { ++destructions; } | ||
|
|
||
| CallCounter(CallCounter&&) noexcept(NoExcept) { ++move_constructions; } |
| }; | ||
|
|
||
|
|
||
| /* |
Contributor
There was a problem hiding this comment.
So what's wrong with this guy?
Owner
Author
There was a problem hiding this comment.
it's a bad unit test - the name does not mean too much, but more importantly UnsafeCopy is not enough. I should add an UnsafeCopy with a noexcept move ctor and one without, to check both cases.
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.
@maciekgajewski could you have a look at the change? Don't spend too much time looking at the unit tests for now, I need to add more of them. the existing ones don't make too much sense as I should really check thoroughly the cases where copy ctor throw or not, same with move ctor.
Cheers!
David