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

TrustedRandomAaccess specialization composes incorrectly for nested iter::Zips #80670

Merged

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Jan 4, 2021

I found this while working on improvements for TRA.

After partially consuming a Zip adapter and then wrapping it into another Zip where the adapters use their TrustedRandomAccess specializations leads to the outer adapter returning elements which should have already been consumed.

If the optimizer gets tripped up by the addition this might affect performance for chained zip() iterators even when the inner one is not partially advanced but it would require more extensive fixes to TrustedRandomAccess to communicate those offsets earlier.

Included test fails on nightly, playground link

After partially consuming a Zip adapter and then wrapping it into
another Zip where the adapters use their TrustedRandomAccess specializations
leads to the outer adapter returning elements which should have already been
consumed.
@rust-highfive
Copy link
Collaborator

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 4, 2021
@the8472 the8472 changed the title TrustedRandomAaccess spec composes incorrectly for nested iter::Zips TrustedRandomAaccess specialization composes incorrectly for nested iter::Zips Jan 4, 2021
@lcnr lcnr added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 5, 2021
@the8472
Copy link
Member Author

the8472 commented Jan 16, 2021

ping @m-ou-se

@m-ou-se
Copy link
Member

m-ou-se commented Jan 16, 2021

Thanks for fixing this!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 16, 2021

📌 Commit af2983a has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2021
@m-ou-se m-ou-se added A-iterators Area: Iterators C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 16, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Jan 16, 2021

Looks like this bug has been there for years. Interesting that it wasn't noticed before.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 16, 2021
Rollup of 17 pull requests

Successful merges:

 - rust-lang#78455 (Introduce {Ref, RefMut}::try_map for optional projections in RefCell)
 - rust-lang#80144 (Remove giant badge in README)
 - rust-lang#80614 (Explain why borrows can't be held across yield point in async blocks)
 - rust-lang#80670 (TrustedRandomAaccess specialization composes incorrectly for nested iter::Zips)
 - rust-lang#80681 (Clarify what the effects of a 'logic error' are)
 - rust-lang#80764 (Re-stabilize Weak::as_ptr and friends for unsized T)
 - rust-lang#80901 (Make `x.py --color always` apply to logging too)
 - rust-lang#80902 (Add a regression test for rust-lang#76281)
 - rust-lang#80941 (Do not suggest invalid code in pattern with loop)
 - rust-lang#80968 (Stabilize the poll_map feature)
 - rust-lang#80971 (Put all feature gate tests under `feature-gates/`)
 - rust-lang#81021 (Remove doctree::Import)
 - rust-lang#81040 (doctest: Reset errors before dropping the parse session)
 - rust-lang#81060 (Add a regression test for rust-lang#50041)
 - rust-lang#81065 (codegen_cranelift: Fix redundant semicolon warn)
 - rust-lang#81069 (Add sample code for Rc::new_cyclic)
 - rust-lang#81081 (Add test for rust-lang#34792)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d8843d9 into rust-lang:master Jan 16, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 16, 2021
@RalfJung

This comment has been minimized.

@the8472

This comment has been minimized.

@RalfJung

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-iterators Area: Iterators C-bug Category: This is a bug. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants