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

Refactor pick2_mut & pick3_mut to use get_disjoint_mut #138372

Merged
merged 2 commits into from
Mar 12, 2025
Merged

Refactor pick2_mut & pick3_mut to use get_disjoint_mut #138372

merged 2 commits into from
Mar 12, 2025
+20 −15

Conversation

Eclips4
Copy link
Contributor

@Eclips4 Eclips4 commented Mar 11, 2025

Closes #138196

@rustbot
Copy link
Collaborator

rustbot commented Mar 11, 2025

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 11, 2025
@Eclips4
Copy link
Contributor Author

Eclips4 commented Mar 11, 2025

r? @scottmcm

@rustbot rustbot assigned scottmcm and unassigned oli-obk Mar 11, 2025
scottmcm
scottmcm reviewed Mar 11, 2025
View reviewed changes
compiler/rustc_index/src/slice.rs Outdated
@@ -120,13 +120,12 @@ impl<I: Idx, T> IndexSlice<I, T> {
pub fn pick2_mut(&mut self, a: I, b: I) -> (&mut T, &mut T) {
let (ai, bi) = (a.index(), b.index());
assert!(ai != bi);
let len = self.raw.len();
assert!(ai < len && bi < len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, is it worth doing these checks when get_disjoint_mut is just going to do them again?

Maybe we could, instead, have the panic!s be more like

    Err(OverlappingIndices) => panic!("Indices {a:?} and {b:?} are not disjoint!"),

so that we can avoid writing out any of the assert!s in the code here.

Copy link
Contributor Author

@Eclips4 Eclips4 Mar 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I overlooked that when I looked at the get_disjoint_mut implementation. Sorry!

@scottmcm scottmcm 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 Mar 11, 2025
@Eclips4
Copy link
Contributor Author

Eclips4 commented Mar 11, 2025

@rustbot ready

@rustbot rustbot 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 Mar 11, 2025
@workingjubilee
Copy link
Member

I believe @scottmcm was simply incorrect when he said that "it's probably not worth refactoring the existing code to"... I may have missed something in my check but there's only 5 files that call pick2_mut and pick3_mut doesn't appear to be used at all.

@scottmcm
Copy link
Member

I don't mind an IndexSlice-specific version that always panics and shows the indexes, though. If we get an ICE that'll be handy.

Thanks for the update!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 12, 2025

📌 Commit 7398b39 has been approved by scottmcm

It is now in the queue for this repository.

@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 12, 2025
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 12, 2025
@jieyouxu
Rollup merge of rust-lang#138372 - Eclips4:issue-138196, r=scottmcm
c738bb0 
Refactor `pick2_mut` & `pick3_mut` to use `get_disjoint_mut`

Closes rust-lang#138196
@jieyouxu jieyouxu mentioned this pull request Mar 12, 2025
Closed
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Mar 12, 2025
@jieyouxu
Rollup merge of rust-lang#138372 - Eclips4:issue-138196, r=scottmcm
ae5ccbb 
Refactor `pick2_mut` & `pick3_mut` to use `get_disjoint_mut`

Closes rust-lang#138196
@jieyouxu jieyouxu mentioned this pull request Mar 12, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2025
@bors
Auto merge of rust-lang#138383 - jieyouxu:rollup-ntnvzmn, r=jieyouxu
74d92df 
Rollup of 25 pull requests

Successful merges:

 - rust-lang#134076 (Stabilize `std::io::ErrorKind::InvalidFilename`)
 - rust-lang#136842 (Add libstd support for Trusty targets)
 - rust-lang#137314 (change definitely unproductive cycles to error)
 - rust-lang#137504 (Move methods from Map to TyCtxt, part 4.)
 - rust-lang#137621 (Add std support to cygwin target)
 - rust-lang#137701 (Convert `ShardedHashMap` to use `hashbrown::HashTable`)
 - rust-lang#138109 (make precise capturing args in rustdoc Json typed)
 - rust-lang#138161 (Add PeekMut::refresh)
 - rust-lang#138162 (Update the standard library to Rust 2024)
 - rust-lang#138174 (Elaborate trait assumption in `receiver_is_dispatchable`)
 - rust-lang#138175 (Support rmeta inputs for --crate-type=bin --emit=obj)
 - rust-lang#138269 (uefi: fs: Implement FileType, FilePermissions and FileAttr)
 - rust-lang#138313 (Update books)
 - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js)
 - rust-lang#138331 (Use `RUSTC_LINT_FLAGS` more)
 - rust-lang#138333 (Rebuild llvm spuriously less frequently)
 - rust-lang#138343 (Enable `f16` tests for `powf`)
 - rust-lang#138345 (Some autodiff cleanups)
 - rust-lang#138346 (naked functions: on windows emit `.endef` without the symbol name)
 - rust-lang#138347 (Reduce `kw::Empty` usage, part 2)
 - rust-lang#138360 (Fix false-positive in `expr_or_init` and in the `invalid_from_utf8` lint)
 - rust-lang#138371 (Update compiletest's `has_asm_support` to match rustc)
 - rust-lang#138372 (Refactor `pick2_mut` & `pick3_mut` to use `get_disjoint_mut`)
 - rust-lang#138376 (Item-related cleanups)
 - rust-lang#138377 (Remove unnecessary lifetime from `PatInfo`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 12, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2025
@bors
Auto merge of rust-lang#138388 - matthiaskrgr:rollup-kbax8sz, r=matth…
3f155ac 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#138161 (Add PeekMut::refresh)
 - rust-lang#138174 (Elaborate trait assumption in `receiver_is_dispatchable`)
 - rust-lang#138313 (Update books)
 - rust-lang#138347 (Reduce `kw::Empty` usage, part 2)
 - rust-lang#138360 (Fix false-positive in `expr_or_init` and in the `invalid_from_utf8` lint)
 - rust-lang#138372 (Refactor `pick2_mut` & `pick3_mut` to use `get_disjoint_mut`)
 - rust-lang#138376 (Item-related cleanups)
 - rust-lang#138377 (Remove unnecessary lifetime from `PatInfo`.)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2025
@bors
Auto merge of rust-lang#138388 - matthiaskrgr:rollup-kbax8sz, r=matth…
57a4736 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#138161 (Add PeekMut::refresh)
 - rust-lang#138174 (Elaborate trait assumption in `receiver_is_dispatchable`)
 - rust-lang#138313 (Update books)
 - rust-lang#138347 (Reduce `kw::Empty` usage, part 2)
 - rust-lang#138360 (Fix false-positive in `expr_or_init` and in the `invalid_from_utf8` lint)
 - rust-lang#138372 (Refactor `pick2_mut` & `pick3_mut` to use `get_disjoint_mut`)
 - rust-lang#138376 (Item-related cleanups)
 - rust-lang#138377 (Remove unnecessary lifetime from `PatInfo`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2bdb10f into rust-lang:master Mar 12, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 12, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2025
@rust-timer
Unrolled build for rust-lang#138372
f98adf0 
Rollup merge of rust-lang#138372 - Eclips4:issue-138196, r=scottmcm

Refactor `pick2_mut` & `pick3_mut` to use `get_disjoint_mut`

Closes rust-lang#138196
@Eclips4 Eclips4 deleted the issue-138196 branch March 12, 2025 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottmcm scottmcm scottmcm left review comments

Assignees

@scottmcm scottmcm

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

Successfully merging this pull request may close these issues.

Implement pick[23]_mut via get_disjoint_mut [rustc cleanup]
6 participants
@Eclips4 @rustbot @workingjubilee @scottmcm @bors @oli-obk