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] clarify preconditions on std::distance #5244

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RedBeard0531
Copy link
Contributor

The prior wording implied that the two sides of the "or" were independent, so as long as last was reachable from first, first didn't need to be reachable from last even if passing a RandomAccessIterator. It seems clear that it is really trying to express two different possible preconditions depending on whether a RandomAccessIterator is passed or not. I used the same phrasing as in the "Effects:" clause to separate the cases.

Alternatively, this precondition seems redundant with http://eel.is/c++draft/iterators#iterator.requirements.general-12, so maybe it should just be removed? This seems to be the only function in [iterators], [ranges], and [algorithms] that has an explicit precondition that iterators passed to it are reachable.

The prior wording implied that the two sides of the "or" were independent, so as long as `last` was reachable from `first`, `first` didn't need to be reachable from `last` even if passing a `RandomAccessIterator`. It seems clear that it is really trying to express two different possible preconditions depending on whether a `RandomAccessIterator` is passed or not. I used the same phrasing as in the "Effects:" clause to separate the cases.
@jwakely
Copy link
Member

jwakely commented Feb 1, 2022

Reachable is defined in terms of ++, so it's wrong to require that first is reachable from last for all random access iterators.

@jwakely
Copy link
Member

jwakely commented Feb 1, 2022

The prior wording implied that the two sides of the "or" were independent, so as long as last was reachable from first, first didn't need to be reachable from last even if passing a RandomAccessIterator.

Which is correct. Unless they are equal (in which case they are both reachable from the other by zero applications of ++) then it's impossible for them both to be reachable from the other (except in the special case of a circular buffer with no end iterator?)

@RedBeard0531
Copy link
Contributor Author

Ahh, I think I see now. It is trying to make distance(vector.end(), vector.begin()) well defined, without making distance(list.end(), list.begin()) well defined. Maybe something like this would make that more clear?

Either [first, last) is a valid range, or [last, first) is a valid range and InputIterator meets the Cpp17RandomAccessIterator requirements

@CaseyCarter
Copy link
Contributor

Maybe something like this would make that more clear?

Either [first, last) is a valid range, or [last, first) is a valid range and InputIterator meets the Cpp17RandomAccessIterator requirements

"Either" suggests these two cases are exclusive, which suggests to me that distance(x, x) might be invalid since both [first, last) and [last, first) are valid ranges. I do think that replacing "woof is reachable from meow" with "[meow, woof) is a valid range" is an improvement. Could you live with this sentence without the "Either" at the beginning?

@tkoeppe
Copy link
Contributor

tkoeppe commented Feb 24, 2022

@RedBeard0531: could you make the changes as Jon and Casey suggest?

@jensmaurer jensmaurer added the changes requested Changes to the wording or approach have been requested and not yet applied. label Feb 24, 2022
@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 22, 2022

@RedBeard0531 Ping :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes requested Changes to the wording or approach have been requested and not yet applied.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants