Skip to content

improve iota_view's iterator's binary operator+ (LWG-3580) - #2417

Merged
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:mainfrom
fsb4000:fix2398
Dec 17, 2021
Merged

improve iota_view's iterator's binary operator+ (LWG-3580)#2417
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:mainfrom
fsb4000:fix2398

Conversation

@fsb4000

Copy link
Copy Markdown
Contributor

Fixes #2398

@fsb4000
Igor Zhukov (fsb4000) requested a review from a team as a code owner December 14, 2021 02:26
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added the LWG Library Working Group issue label Dec 14, 2021
Comment thread stl/inc/ranges

Copy link
Copy Markdown
Contributor

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.

Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges Outdated
Comment thread stl/inc/ranges
}
_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(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casey Carter (@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.

Copy link
Copy Markdown
Contributor

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 _Ioterators 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.

@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 8578156 into microsoft:main Dec 17, 2021
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for implementing a seventh LWG issue resolution in today's batch! 🚀 🚀 😹

@fsb4000
Igor Zhukov (fsb4000) deleted the fix2398 branch December 17, 2021 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LWG Library Working Group issue ranges C++20/23 ranges

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LWG-3580 iota_view's iterator's binary operator+ should be improved

3 participants