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

Better documentation for Iter::zip behavior #52279

Closed
Lucretiel opened this issue Jul 12, 2018 · 4 comments
Closed

Better documentation for Iter::zip behavior #52279

Lucretiel opened this issue Jul 12, 2018 · 4 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@Lucretiel
Copy link
Contributor

In particular, we should specify and document what exactly zip does when its first child terminates. Currently, as soon as self.a.next() returns None, it short-circuits execution without calling self.b.next(). This could lead to some potential surprising behavior:

self.a  1  2  3     5  6
self.b  1  2  3  4  5  6
------
self    11 22 33    54 65

To be clear, I'm not saying this behavior is bad; just that it should be more explicitly documented. This will allow developers to more confidently build abstractions on top of Zip, knowing exactly what it does with the underlying iterator.

@kennytm kennytm added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Jul 12, 2018
@rustyseris
Copy link

In this case, I would even day that the documentation is misleading.
When either iterator returns None, all further calls to next will return None.

@frewsxcv
Copy link
Member

@Lucretiel
Copy link
Contributor Author

That's close, but I'd propose a further change, to emphasize that zip always does a.next(); b.next(), and that if a.next() returns None, it short circuits and returns None immediately, without calling b.next()

@frewsxcv
Copy link
Member

Opened a pull request for this: #52477

kennytm added a commit to kennytm/rust that referenced this issue Jul 18, 2018
…xcrichton

Clarify short-circuiting behvaior of Iterator::zip.

Fixes rust-lang#52279.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 27, 2021
…ntee, r=dtolnay

Weaken guarantee around advancing underlying iterators in zip

The current guarantee (introduced in rust-lang#52279) is too strong as it prevents adapters from exploiting knowledge about the iterator length and using counted loops for example because they would stop calling `next()` before it ever returned `None`. Additionally several nested zip iterators already fail to uphold this.

This does not yet remove any of the specialization code that tries (and sometimes fails) to uphold the guarantee for `next()`
because removing it would also affect `next_back()` in more surprising ways.

The intent is to be able to remove for example this branch

https://github.com/rust-lang/rust/blob/36bcf4069717b9dff90270d13b53a3b130329960/library/core/src/iter/adapters/zip.rs#L234-L243

or this test

https://github.com/rust-lang/rust/blob/36bcf4069717b9dff90270d13b53a3b130329960/library/core/tests/iter/adapters/zip.rs#L177-L188

Solves rust-lang#82303 by declaring it a non-issue.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 27, 2021
…ntee, r=dtolnay

Weaken guarantee around advancing underlying iterators in zip

The current guarantee (introduced in rust-lang#52279) is too strong as it prevents adapters from exploiting knowledge about the iterator length and using counted loops for example because they would stop calling `next()` before it ever returned `None`. Additionally several nested zip iterators already fail to uphold this.

This does not yet remove any of the specialization code that tries (and sometimes fails) to uphold the guarantee for `next()`
because removing it would also affect `next_back()` in more surprising ways.

The intent is to be able to remove for example this branch

https://github.com/rust-lang/rust/blob/36bcf4069717b9dff90270d13b53a3b130329960/library/core/src/iter/adapters/zip.rs#L234-L243

or this test

https://github.com/rust-lang/rust/blob/36bcf4069717b9dff90270d13b53a3b130329960/library/core/tests/iter/adapters/zip.rs#L177-L188

Solves rust-lang#82303 by declaring it a non-issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

4 participants