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.

Sign up for GitHub

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

Jump to bottom

Implement TrustedLen and TrustedRandomAccess for Range<integer>, array::IntoIter, VecDequeue's iterators #81607

Merged
merged 4 commits into from
Mar 22, 2021
Merged

Implement TrustedLen and TrustedRandomAccess for Range<integer>, array::IntoIter, VecDequeue's iterators #81607

merged 4 commits into from
Mar 22, 2021

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Jan 31, 2021

This should make some FromIterator and .zip() specializations applicable in a few more cases.

@rustbot label libs-impl

@rustbot
Copy link
Collaborator

rustbot commented Jan 31, 2021

Error: Label libs-impl can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(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 31, 2021
@sdroege
Copy link
Contributor

sdroege commented Jan 31, 2021

Ah I also had some of those locally already but wanted to finish the remaining ones first :) vec::Drain should also be possible to cover here, if I'm not mistaken.

@the8472 the8472 force-pushed the trustedrandomaccess-all-the-things branch from 0897250 to e8908a0 Compare January 31, 2021 21:56
@sdroege
Copy link
Contributor

sdroege commented Feb 1, 2021

vec::Drain should also be possible to cover here, if I'm not mistaken.

That's in #81617 now. On top of your PR here you could do the same for VecDeque

@the8472 the8472 force-pushed the trustedrandomaccess-all-the-things branch from e8908a0 to f0313ee Compare February 2, 2021 00:39
@the8472
Copy link
Member Author

the8472 commented Feb 5, 2021

Yeah, still worth mentioning in case someone wants to lift the Copy bound later

That doesn't seem to be practically feasible for TrustedRandomAccess. Doing so would require somehow getting the source into shape again after iteration to make it droppable without leaks or double-frees. This would be easy enough for internal iteration such as try_fold. But for external iteration this would either require Zip to implement Drop to do the cleanup iteration, which is a breaking change or it would require cleanup after each call to next() and similar single-step methods which would negate the benefits of TrustedRandomAccess.

I think implementing more LLVM-friendly iteration for sources with T: Drop would require yet another unsafe specialization trait similar to TrustedRandomAcces but with the additional restriction that the caller must call a cleanup method when they're done with the unchecked accesses.

That or llvm needs to be fixed in several ways. Then we could do away with most of this mess.

@Mark-Simulacrum
Copy link
Member

r? @m-ou-se (possibly for further reassignment), I don't have a lot of knowledge about iterators right now and probably not the time to do a deep dive

@the8472 the8472 mentioned this pull request Feb 19, 2021
Closed
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2021
@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Mar 21, 2021

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 21, 2021

📌 Commit f0313ee03f750adde3897c26ab208228da8ddc02 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 Mar 21, 2021
@bors
Copy link
Contributor

bors commented Mar 21, 2021

⌛ Testing commit f0313ee03f750adde3897c26ab208228da8ddc02 with merge e4bce39493aa9786c563d386e1bbde6e26c2580e...

@bors
Copy link
Contributor

bors commented Mar 21, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 21, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@the8472 the8472 force-pushed the trustedrandomaccess-all-the-things branch from f0313ee to 54c73a3 Compare March 21, 2021 19:41
@the8472 the8472 force-pushed the trustedrandomaccess-all-the-things branch from 54c73a3 to 08a1dd2 Compare March 21, 2021 19:44
@m-ou-se
Copy link
Member

m-ou-se commented Mar 21, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Mar 21, 2021

📌 Commit 08a1dd2 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 Mar 21, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 21, 2021
@Dylan-DPC
Rollup merge of rust-lang#81607 - the8472:trustedrandomaccess-all-the…
231b7c5 
…-things, r=m-ou-se

Implement TrustedLen and TrustedRandomAccess for Range<integer>, array::IntoIter, VecDequeue's iterators

This should make some `FromIterator` and `.zip()` specializations applicable in a few more cases.

`@rustbot` label libs-impl
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 21, 2021
Closed
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 22, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2021
@bors
Auto merge of rust-lang#83360 - Dylan-DPC:rollup-17xulpv, r=Dylan-DPC
142c831 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#80193 (stabilize `feature(osstring_ascii)`)
 - rust-lang#80771 (Make NonNull::as_ref (and friends) return refs with unbound lifetimes)
 - rust-lang#81607 (Implement TrustedLen and TrustedRandomAccess for Range<integer>, array::IntoIter, VecDequeue's iterators)
 - rust-lang#82554 (Fix invalid slice access in String::retain)
 - rust-lang#82686 (Move `std::sys::unix::platform` to `std::sys::unix::ext`)
 - rust-lang#82771 (slice: Stabilize IterMut::as_slice.)
 - rust-lang#83329 (Cleanup LLVM debuginfo module docs)
 - rust-lang#83336 (Fix ICE with `use clippy::a::b;`)
 - rust-lang#83350 (Download a more recent LLVM version if `src/version` is modified)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 29a53e6 into rust-lang:master Mar 22, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdroege sdroege sdroege left review comments

Assignees

@m-ou-se m-ou-se

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.53.0
Development

Successfully merging this pull request may close these issues.
10 participants
@the8472 @rustbot @rust-highfive @sdroege @Mark-Simulacrum @m-ou-se @bors @rust-log-analyzer @crlf0710 @JohnCSimon