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

[iterator.operations, range.iter.op.distance] Reword "get to". #4715

Merged
merged 1 commit into from Jun 23, 2021

Conversation

tkoeppe
Copy link
Contributor

@tkoeppe tkoeppe commented Jun 22, 2021

It was previously not explicitly stated that input iterators would
actually be incremented (which invalidates copies). The new wording is
more explicit about how the distance is measured, and in doing so
calls out more explicitly that input iterators are indeed incremented.

Fixes #2807.
Fixes #2813.

This is an alternative approach to #2813, as suggested by @jensmaurer in the comments.

@jwakely, @CaseyCarter, I'd welcome your review on this!

It was previously not explicitly stated that input iterators would
actually be incremented (which invalidates copies). The new wording is
more explicit about how the distance is measured, and in doing so
calls out more explicitly that input iterators are indeed incremented.
@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 22, 2021

@CaseyCarter: I don't think either version of advance is using similar wording, so this should make things neither more nor less consistent. The range version of advance does say "increments ", though, so this seems like a reasonable way to say that an iterator really gets incremented.

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

Looks good for this case, but the neighboring advance and ranges::advance still talk about "increments i by n" for input iterators, which should get a similar treatment; maybe just "increments i n times" except that something should separate "i" and "n".

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 22, 2021

I see -- I thought we had blanket wording for the "by n" bit, but I was thinking of [algorithms.requirements]p12. I was mainly after making sure we are saying "increment" explicitly.

I'm not sure if the "by n" part itself is controversial or in need of clarification. Thoughts?

@jensmaurer
Copy link
Member

Well, I think that some of the ranges::advance variants avoid the "by n" and clearly say what they mean, in terms of repeatedly incrementing the iterator. Given that advance is such a basic building block, I feel we should not gloss over the expressiveness differences of iterators here, and be explicit instead.

I'm ok with forking off "advance" into a separate issue.

@CaseyCarter
Copy link
Contributor

LWG originally chose the "increments i by n" phrasing to avoid "increments i n times." (Ditto "decrements i by -n".) I've never been enthusiastic about it, but folks felt it was "clear enough". Feel free to replace it if you have a better phrasing.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 22, 2021

I tend to agree that "increments i by n" is clear enough; I have yet to see any confusion arising from it. Just for contrast, the linked issue shows that the lack of clarity about incrementing was indeed something that could make you wonder about what exactly is specified, but nobody has complained about "by n" yet. If anyone really wants to spend time on that, let me know, but I'd just leave it as is.

@jensmaurer
Copy link
Member

I'd just leave it as is.

Fine with me.

@tkoeppe tkoeppe merged commit 5474388 into cplusplus:main Jun 23, 2021
@umanwizard
Copy link

Thanks for addressing this!

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 23, 2021

@umanwizard: My pleasure, thanks for reporting!

@tkoeppe tkoeppe deleted the iter_get branch June 23, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants