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

Stabilize get_many_mut as get_disjoint_mut #134633

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Stabilize get_many_mut as get_disjoint_mut #134633

wants to merge 1 commit into from
+98 −103

Conversation

GrigorenkoPV
Copy link
Contributor

@GrigorenkoPV GrigorenkoPV commented Dec 21, 2024 •
edited
Loading

Tracking issue: #104642

Closes #104642

FCP completed in #104642 (comment)

@rustbot
Copy link
Collaborator

rustbot commented Dec 21, 2024

r? @cuviper

rustbot has assigned @cuviper.
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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 21, 2024
@GrigorenkoPV
Copy link
Contributor Author

r? @joshtriplett

@rustbot rustbot assigned joshtriplett and unassigned cuviper Jan 14, 2025
@Mark-Simulacrum Mark-Simulacrum added the relnotes Marks issues that should be documented in the release notes of the next release. label Jan 20, 2025
@rustbot rustbot mentioned this pull request Jan 20, 2025
Open
3 tasks
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Jan 20, 2025
Closed
3 tasks
@GrigorenkoPV
Copy link
Contributor Author

Should I add #[rustc_confusables("get_many_mut")] to help nightly users switch to the new name?

@cuviper
Copy link
Member

cuviper commented Jan 22, 2025

I'm not sure about using rustc_confusables for this, but it would help to keep the feature name the same even though the method is changing, so anyone using #![feature(get_many_mut)] will trigger the stable_feature lint.

@GrigorenkoPV
Copy link
Contributor Author

Reverted the feature flag rename.

cuviper
cuviper reviewed Jan 23, 2025
View reviewed changes
library/core/src/error.rs Outdated Show resolved Hide resolved
library/core/src/slice/mod.rs
#[inline]
pub unsafe fn get_many_unchecked_mut<I, const N: usize>(
pub unsafe fn get_disjoint_unchecked_mut<I, const N: usize>(
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this might be better as get_unchecked_disjoint_mut? But I'm not on the API team and wasn't present to know if they discussed this.

Copy link
Contributor Author

@GrigorenkoPV GrigorenkoPV Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

#136152 did get_disjoint_unchecked_mut for HashMap, so I think we'll have to mirror that here.

@bors
Copy link
Contributor

bors commented Jan 27, 2025

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

@GrigorenkoPV
Stabilize get_many_mut as get_disjoint_mut
1abc853 
* Renames the methods:
	* `get_many_mut` -> `get_disjoint_mut`
	* `get_many_unchecked_mut` -> `get_disjoint_unchecked_mut`
* Does not rename the feature flag: `get_many_mut`
* Marks the feature as stable
* Renames some helper stuff:
	* `GetManyMutError` -> `GetDisjointMutError`
	* `GetManyMutIndex` -> `GetDisjointMutIndex`
	* `get_many_mut_helpers` -> `get_disjoint_mut_helpers`
	* `get_many_check_valid` -> `get_disjoint_check_valid`

This only touches slice methods.
HashMap's methods and feature gates are not renamed here
(nor are they stabilized).
@GrigorenkoPV
Copy link
Contributor Author

GrigorenkoPV commented Jan 29, 2025 •
edited
Loading

r? t-libs

@rustbot

This comment was marked as outdated.

Sign in to view
@rustbot rustbot assigned ibraheemdev and unassigned joshtriplett Jan 29, 2025
@ibraheemdev
Copy link
Member

r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 31, 2025
@rustbot rustbot assigned Amanieu and unassigned ibraheemdev Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cuviper cuviper cuviper left review comments

Assignees

@Amanieu Amanieu

Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tracking Issue for get_many_mut
8 participants
@GrigorenkoPV @rustbot @cuviper @bors @ibraheemdev @joshtriplett @Amanieu @Mark-Simulacrum