Skip to content
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

[range.slide.view] Replace conditional with call to max #5250

Open
JohelEGP opened this issue Feb 5, 2022 · 6 comments
Open

[range.slide.view] Replace conditional with call to max #5250

JohelEGP opened this issue Feb 5, 2022 · 6 comments

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented Feb 5, 2022

[range.slide.view] adds (via #5249)

auto sz = ranges::distance(base_) - n_ + 1;
if (sz < 0) sz = 0;
return to-unsigned-like(sz);

Which can be simplified to

auto sz = max({}, ranges::distance(base_) - n_ + 1);
return to-unsigned-like(sz);
@timsong-cpp
Copy link
Contributor

I don't think requiring mental overload resolution among the multiple overloads of ranges::max to figure out the meaning of the the untyped braces is an improvement.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Feb 7, 2022

That's std::max.

@timsong-cpp
Copy link
Contributor

That's std::max.

It's not. We are in namespace ranges.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Feb 7, 2022

It can be simpler:

return to-unsigned-like(max({}, ranges::distance(base_) - n_ + 1));

ranges::max accepts (https://godbolt.org/z/a49G3Y84b):

return to-unsigned-like(ranges::max({0}, ranges::distance(base_) - n_ + 1));

which should alleviate the concerns of #5250 (comment).

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Feb 7, 2022

That's std::max.

It's not. We are in namespace ranges.

IIRC, when inside a nested namespace, the convention is to qualify "overloaded" identifiers, like distance above. I don't see this documented at https://github.com/cplusplus/draft/wiki/Specification-Style-Guidelines, so going purely by [contents], unqualified max would indeed be ranges::max.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Feb 7, 2022

which should alleviate the concerns of #5250 (comment).

Actually, that only alleviates my original, left-unsaid readability concern by replacing 0 with {}. Suggested "simplifications" certainly still need overload resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants