Mark constructors that do not throw exceptions as noexcept#3278
Mark constructors that do not throw exceptions as noexcept#3278SiliconA-Z wants to merge 1 commit intomicrosoft:mainfrom
Conversation
45d780f to
a6923ce
Compare
While we can continue considering |
|
I think you should split this PR into two PRs.
For containers, wrapping types, and defaulted functions, I think it's wrong to add |
frederick-vs-ja
left a comment
There was a problem hiding this comment.
I've looked into stream types. Unfortunately, perhaps we shouldn't add noexcept to any of their move constructors.
It may be OK to make most (but not all) move assignment operators noexcept. And we should also consider swap functions and other functions called by them.
|
|
||
| basic_spanbuf(const basic_spanbuf&) = delete; | ||
| basic_spanbuf(basic_spanbuf&& _Right) | ||
| basic_spanbuf(basic_spanbuf&& _Right) noexcept |
There was a problem hiding this comment.
For stream classes with their own buffers, unfortunately, we always copy constructs the basic_streambuf base class subobjects, and thus the move constructors can't be noexcept. Ditto for basic_meowbuf and basic_(i|o)meowstream classes.
| basic_spanbuf& operator=(const basic_spanbuf&) = delete; | ||
|
|
||
| basic_spanbuf& operator=(basic_spanbuf&& _Right) { | ||
| basic_spanbuf& operator=(basic_spanbuf&& _Right) noexcept { |
There was a problem hiding this comment.
Perhaps we should refactor this move assignment operator to avoid move construction.
There was a problem hiding this comment.
I'm working on this in my (another) PR.
| basic_ispanstream& operator=(const basic_ispanstream&) = delete; | ||
|
|
||
| basic_ispanstream& operator=(basic_ispanstream&& _Right) { | ||
| basic_ispanstream& operator=(basic_ispanstream&& _Right) noexcept { |
There was a problem hiding this comment.
I think it's reasonable to strengthen exception specifications for move assignment operators.
However, you seemly forgot to do similar strengthening for swap functions (and other functions called by swap).
| } | ||
|
|
||
| basic_syncbuf& operator=(basic_syncbuf&& _Right) { | ||
| basic_syncbuf& operator=(basic_syncbuf&& _Right) noexcept { |
There was a problem hiding this comment.
noexcept shouldn't be added here because emit may throw. This is LWG-3498.
|
|
||
| protected: | ||
| __CLR_OR_THIS_CALL basic_istream(basic_istream&& _Right) : _Chcount(_Right._Chcount) { | ||
| __CLR_OR_THIS_CALL basic_istream(basic_istream&& _Right) noexcept : _Chcount(_Right._Chcount) { |
There was a problem hiding this comment.
It seems unsuitable to add noexcept (ditto for basic_iostream and basic_ostream), since
- this constructor calls
basic_ios<...>::init, initcallswiden,widencallsuse_facet, which may throwbad_cast.
However, it seems OK to make move functions noexcept.
|
We talked about this at the weekly maintainer meeting and have decided to close this PR without merging, for the reasons stated in #3278 (comment) . We remain open to considering small, targeted PRs that have been carefully thought through to be correct, such as @frederick-vs-ja's #3314. |
No description provided.