-
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
Implement ranges::iota_view #1693
Conversation
Rework the implementation of P1739R4 to propose as a resolution for LWG-3407; implement the resolution of LWG-3523.
#endif // TRANSITION, DevCom-1347136 | ||
|
||
constexpr _Ioterator& operator+=(const difference_type _Off) | ||
#if defined(__clang__) || defined(__EDG__) // TRANSITION, DevCom-1347136 |
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.
DevCom-1347136 is fun: the compiler warns about possible loss of data narrowing _Off
in noexcept(_Current += _Off)
(Yes, loss of data in an unevaluated operand.)
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.
Looks good, all I found were nitpicks!
static_assert(same_as<ranges::range_difference_t<R>, long long>); | ||
} | ||
|
||
{ |
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.
Is there some condition missing here?
I do not really see the benefit of that scope, as there are no other variables defined 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.
Mostly the braces are here (as on other blocks) to wrap this up in a nicely self-contained chunk. It's probably my lizard brain telling me to factor this overlarge function out into separate small and simple functions. That said I acknowledge that removing this pair of braces reduces indentation but has no semantic effect. I can strike them if you think it's an improvement.
1, 2, 3, 4, thank you for adding more, 5, 6, 7, 8, views in |
Also rework the implementation of P1739R4 to propose as a resolution for LWG-3407, and implement the resolution of LWG-3523.