-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
improve iota_view's iterator's binary operator+ (LWG-3580) #2417
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've already pushed a change to correct the pre-existing issues in my comments.
} | ||
_NODISCARD friend constexpr _Ioterator operator+(const difference_type _Off, _Ioterator _It) noexcept( | ||
noexcept(static_cast<_Wi>(_It._Current + _Off))) /* strengthened */ requires _Advanceable<_Wi> { | ||
is_nothrow_move_constructible_v<_Wi>&& noexcept( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CaseyCarter I observe that this one says is_nothrow_move_constructible_v<_Wi>
but the other two said is_nothrow_move_constructible_v<_Ioterator>
. You mentioned that they're equivalent, and there seems to be a rationale (as this one constructs an _Ioterator
prvalue), but I wanted to point this out in case it was unintentional and should be cleaned up later. No change requested here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other two functions return an lvalue denoting a variable with automatic storage duration, so they are implicitly moved, and that variable is a function argument, so they cannot benefit from NRVO or RVO. Consequently they move construct an _Ioterator
.
This function returns a prvalue, so no _Ioterator
s are moved or copied - C++17 delayed temporary materialization kicks in. It does need to account for potential throws from the _Ioterator
constructor from _Wi
, however. That constructor is annotated with noexcept(is_nothrow_move_constructible_v<_Wi>
. I'm not sure why I decided to "inline" that noexcept
-specifier instead of using the more direct is_nothrow_constructible_v<_Ioterator, _Wi>
. I really need to work at making my conditional noexcept
style more consistent.
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Thanks for implementing a seventh LWG issue resolution in today's batch! 🚀 🚀 😹 |
Fixes #2398