Skip to content

[beta] Remove unsound TrustedRandomAccess implementations #87136

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 20, 2021
Merged

[beta] Remove unsound TrustedRandomAccess implementations #87136

merged 1 commit into from
Jul 20, 2021

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jul 14, 2021

This re-applies #86222 for 1.54, as the larger fixes in #85874 are still in progress.

@steffahn @cuviper
Remove unsound TrustedRandomAccess implementations
b1e90f9 
Removes the implementations that depend on the user-definable trait `Copy`.

Beta backport: Does not modify `vec::IntoIter`.

(cherry picked from commit 28ea358)
@rust-highfive
Copy link
Contributor

r? @dtolnay

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

@rust-highfive
Copy link
Contributor

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 14, 2021
@cuviper
Copy link
Member Author

cuviper commented Jul 14, 2021

We discussed this in the @rust-lang/libs meeting last week and reaffirmed that plan today.

@cuviper
Copy link
Member Author

cuviper commented Jul 20, 2021

r? @Mark-Simulacrum

@Mark-Simulacrum
Copy link
Member

@bors r+ p=1

Could you file this PR (or a similar one) against 1.55 (master) as well? I would like to stop backporting fixes to this bug and instead have the "true" fix revert or otherwise work atop these commits.

@bors
Copy link
Collaborator

bors commented Jul 20, 2021

📌 Commit b1e90f9 has been approved by Mark-Simulacrum

@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 Jul 20, 2021
@Mark-Simulacrum
Copy link
Member

@bors rollup=never

@cuviper
Copy link
Member Author

cuviper commented Jul 20, 2021

Could you file this PR (or a similar one) against 1.55 (master) as well? I would like to stop backporting fixes to this bug and instead have the "true" fix revert or otherwise work atop these commits.

@steffahn as the true author from #85874, would you be able to do that?
Or is it too much in conflict with the current state of that PR?

@bors
Copy link
Collaborator

bors commented Jul 20, 2021

⌛ Testing commit b1e90f9 with merge 3a5d335...

@steffahn
Copy link
Member

@cuviper Sure, I'll make a PR for master and resolve merge conflicts after it's merged. I suggested that myself, too, in order to avoid the repeated need for beta backport. Seems like we may indeed not make it into 1.55 with the full #85874 given the approaching deadline and review being slow.

There's no conflict with #85874, that one removes the impls in question, too (and leaves adding them back as future work). I'll create the PR tomorrow.

@cuviper
Copy link
Member Author

cuviper commented Jul 20, 2021

Thanks! Feel free to tag me as the reviewer on that.

@bors bors mentioned this pull request Jul 20, 2021
Merged
@bors
Copy link
Collaborator

bors commented Jul 20, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 3a5d335 to beta...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 20, 2021
@bors bors merged commit 3a5d335 into rust-lang:beta Jul 20, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jul 20, 2021
@steffahn steffahn mentioned this pull request Jul 21, 2021
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 22, 2021
@GuillaumeGomez
Rollup merge of rust-lang#87343 - steffahn:fix_unsound_zip_optimizati…
aece3df 
…on_only_regression_fix, r=cuviper

Regression fix to avoid further beta backports: Remove unsound TrustedRandomAccess implementations

Removes the implementations that depend on the user-definable trait `Copy`.

Only fix regressions to ensure merge in 1.55: Does not modify `vec::IntoIter`.

<hr>

This PR applies the beta-`1.53` backport rust-lang#86222 (merged as part of rust-lang#86225), a reduced version of rust-lang#85874 that only fixes regressions, to `master` in order to avoid the need for further backports from `1.55` onwards. Beta-`1.54` backport already happened with rust-lang#87136. In case that rust-lang#85874 gets merged quickly (within a week), this PR would be unnecessary.

r? `@cuviper`
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
7 participants
@cuviper @rust-highfive @Mark-Simulacrum @bors @steffahn @dtolnay @rustbot