Implement `TrustedRandomAccess` for `vec::Drain` by sdroege · Pull Request #81617 · rust-lang/rust

sdroege · 2021-02-01T08:17:19Z

No description provided.

rust-highfive · 2021-02-01T08:17:22Z

r? @Mark-Simulacrum

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

library/alloc/src/vec/drain.rs

Mark-Simulacrum · 2021-02-12T02:23:05Z

r? @m-ou-se for reassignment

Dylan-DPC-zz · 2021-03-20T13:31:54Z

r? @Amanieu

Amanieu · 2021-03-21T18:17:07Z

@bors r+

bors · 2021-03-21T18:17:09Z

📌 Commit 1d33db3 has been approved by Amanieu

bors · 2021-03-21T19:02:35Z

⌛ Testing commit 1d33db3 with merge e5642fd10b149d75caa992b4327123129d14f902...

bors · 2021-03-21T19:08:07Z

💔 Test failed - checks-actions

JohnTitor · 2021-07-27T18:59:58Z

Marking as S-blocked as per #81617 (comment).

the8472 · 2021-07-29T20:34:49Z

library/alloc/src/vec/drain.rs

Now that #85874 has landed this should become TrustedRandomAccessNoCoerce

Switched over to TrustedRandomAccessNoCoerce. Thanks and sorry for the delay!

crlf0710 · 2021-08-14T16:35:05Z

Ping from triage, any updates on this?

JohnCSimon · 2021-09-20T00:35:29Z

@sdroege
Ping again from triage, any updates on this?

…pping Otherwise this simply causes advancing of the iterator with no other effects, which is unnecessary work.

GuillaumeGomez · 2021-10-10T18:47:42Z

Ready for a new round of review! :)

the8472

Taking over review of this.

Overall it seems safe, just the safety comment could use some expansion

r? @the8472

the8472 · 2021-11-05T23:44:51Z

library/alloc/src/vec/drain.rs

+// requires each index to be accessed only once, this is safe to do here.
+//
+// TrustedRandomAccess (without NoCoerce) must not be implemented because
+// subtypes/supertypes of `T` might not be `NonDrop`


Drain is a bit more complicated than IntoIter due to its Drop impl doing actual work even for T: Copy. This comment should mention why this is still ok. Something along the lines that the tail move does not depend on how much has been drained.

JohnCSimon · 2021-12-05T01:37:04Z

Ping from triage:
@sdroege Can you please address the comment from the8472 and adjust the label with @rustbot ready when you're ready for review

bors · 2021-12-09T17:53:44Z

☔ The latest upstream changes (presumably #85157) made this pull request unmergeable. Please resolve the merge conflicts.

JohnCSimon · 2022-01-30T04:42:05Z

Ping from triage:
@sdroege
I'm closing this due to inactivity, Please reopen when you are ready to continue with this. Thank you.

@rustbot label: +S-inactive

rust-highfive assigned Mark-Simulacrum Feb 1, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 1, 2021

sdroege mentioned this pull request Feb 1, 2021

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

Merged

This comment has been minimized.

Sign in to view

sdroege force-pushed the vec-drain-trusted-random-access branch from 020dfb8 to 1d33db3 Compare February 1, 2021 08:36

oli-obk added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 1, 2021

the8472 reviewed Feb 1, 2021

View reviewed changes

library/alloc/src/vec/drain.rs Show resolved Hide resolved

rust-highfive assigned m-ou-se and unassigned Mark-Simulacrum Feb 12, 2021

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 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 20, 2021

rust-highfive assigned Amanieu and unassigned m-ou-se Mar 20, 2021

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

This comment has been minimized.

Sign in to view

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

JohnTitor added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jul 27, 2021

the8472 reviewed Jul 29, 2021

View reviewed changes

the8472 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Jul 29, 2021

camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 2, 2021

camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2021

sdroege added 3 commits October 10, 2021 21:15

Add SAFETY comment to the TrustedLen impl of vec::Drain

4a46acd

Implement TrustedRandomAccess for vec::Drain

50dbccc

Only exhaust the slice iterator in vec::Drain if the items need dro…

19480c1

…pping Otherwise this simply causes advancing of the iterator with no other effects, which is unnecessary work.

sdroege force-pushed the vec-drain-trusted-random-access branch from 9c70fcb to 19480c1 Compare October 10, 2021 18:21

GuillaumeGomez added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 10, 2021

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 Oct 31, 2021

the8472 requested changes Nov 5, 2021

View reviewed changes

the8472 assigned the8472 and unassigned Amanieu Nov 5, 2021

the8472 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2021

JohnCSimon closed this Jan 30, 2022

rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Jan 30, 2022

Uh oh!

Conversation

sdroege commented Feb 1, 2021

Uh oh!

rust-highfive commented Feb 1, 2021

Uh oh!

This comment has been minimized.

Uh oh!

Mark-Simulacrum commented Feb 12, 2021

Uh oh!

Dylan-DPC-zz commented Mar 20, 2021

Uh oh!

Amanieu commented Mar 21, 2021

Uh oh!

bors commented Mar 21, 2021

Uh oh!

bors commented Mar 21, 2021

Uh oh!

This comment has been minimized.

bors commented Mar 21, 2021

Uh oh!

JohnTitor commented Jul 27, 2021

Uh oh!

the8472 Jul 29, 2021

Choose a reason for hiding this comment

Uh oh!

sdroege Oct 10, 2021

Choose a reason for hiding this comment

Uh oh!

crlf0710 commented Aug 14, 2021

Uh oh!

JohnCSimon commented Sep 20, 2021

Uh oh!

GuillaumeGomez commented Oct 10, 2021

Uh oh!

the8472 left a comment

Choose a reason for hiding this comment

Uh oh!

the8472 Nov 5, 2021

Choose a reason for hiding this comment

Uh oh!

JohnCSimon commented Dec 5, 2021

Uh oh!

bors commented Dec 9, 2021

Uh oh!

JohnCSimon commented Jan 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

17 participants