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.zip, ranges.cartesian.iterator] Simplify maybe-const<Const, Views> into const Views #5778

Merged
merged 3 commits into from Aug 23, 2022

Conversation

hewillk
Copy link
Contributor

@hewillk hewillk commented Aug 23, 2022

Elsewhere it is simple enough since there is no fold expression and Base is defined, and I think this improves readability.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 23, 2022

Is this editorial?

@hewillk
Copy link
Contributor Author

hewillk commented Aug 23, 2022

Not sure, if this was LWG then I would close it.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 23, 2022

Well, to be editorial the change cannot change the normative meaning of the text, first and foremost. I don't know whether the proposed new text is equivalent to the existing one, or if you're proposing a (simplifying) change of the observable specification. Can you elaborate?

@cpplearner
Copy link
Contributor

IIUC the idea is: these maybe-const<Const, Views> appear in the right operand of requires Const && stuff, so they are only used when Const is true, so they are equivalent to maybe-const<true, Views>, which is the same as const Views.

@jwakely
Copy link
Member

jwakely commented Aug 23, 2022

I believe this has no normative impact. The requires clause will short circuit so that if Const is false, we never try to check the convertible_to constraint, and so it doesn't matter whether we use iterator_t<X> or iterator<const X> as it will not even be instantiated. So the constraint using iterator_t<const X> is only checked when Const is true, in which case maybe-const would have added the const anyway.

So I think this is a valid editorial simplification.

Edit: i.e. what cpplearner said at the same time.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 23, 2022

OK, thanks!

In that case, can you please review if any of the linebreaks can now go away?

source/ranges.tex Outdated Show resolved Hide resolved
source/ranges.tex Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

None yet

5 participants