Skip to content

Conversation

Jules-Bertholet
Copy link
Contributor

@Jules-Bertholet Jules-Bertholet commented Aug 16, 2025

@rustbot label T-libs-api needs-fcp -T-libs

I’ve tried to only add impls that are unlikely to cause single-applicable-impl inference breakage.

@rustbot
Copy link
Collaborator

rustbot commented Aug 16, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 16, 2025
@rust-log-analyzer

This comment has been minimized.

@Jules-Bertholet Jules-Bertholet force-pushed the more_conversion_trait_impls branch from 7ef29d9 to 3a96604 Compare August 16, 2025 17:22
@rust-log-analyzer

This comment has been minimized.

@Jules-Bertholet Jules-Bertholet force-pushed the more_conversion_trait_impls branch 2 times, most recently from 676ff1f to bd960c9 Compare August 16, 2025 18:13
@jieyouxu
Copy link
Member

r? libs-api

@rustbot rustbot assigned m-ou-se and unassigned Mark-Simulacrum Aug 17, 2025
@tgross35
Copy link
Contributor

tgross35 commented Sep 5, 2025

Preparing for crater

@bors try

@rustbot label +I-libs-api-nominated
r? libs

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 5, 2025
@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Sep 5, 2025
@rustbot rustbot added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 5, 2025
Comment on lines 1181 to 1191
impl AsMut<str> for AsciiChar {
#[inline(always)]
fn as_mut(&mut self) -> &mut str {
let ascii_ptr: *mut [Self] = crate::slice::from_mut(self);
let str_ptr = ascii_ptr as *mut str;
// SAFETY: Each ASCII codepoint in UTF-8 is encoded as one single-byte
// code unit having the same value as the ASCII byte.
unsafe { &mut *str_ptr }
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This could probably be implemented in a new as_mut_str method, similar to as_str

Also, could everything here use #[inline] without the always?

@tgross35 tgross35 added S-waiting-on-team DEPRECATED: Use the team-based variants `S-waiting-on-t-lang`, `S-waiting-on-t-compiler`, ... and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2025
@rust-bors
Copy link

rust-bors bot commented Sep 5, 2025

☀️ Try build successful (CI)
Build commit: 0498c15 (0498c157964e0946754e29e57ee9ef4527dc2a42, parent: af00ff2ce62b6617ed19305ae39e135ac71d0b22)

@tgross35
Copy link
Contributor

tgross35 commented Sep 5, 2025

@craterbot check

@craterbot craterbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 10, 2025
@Jules-Bertholet
Copy link
Contributor Author

That 1 regression seems spurious.

@bors
Copy link
Collaborator

bors commented Sep 13, 2025

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

- `impl AsRef<str> for AsciiChar` (unstable)
- `impl AsMut<str> for AsciiChar` (unstable)
- `impl<T, const N: usize> From<[MaybeUninit<T>; N]> for MaybeUninit<[T; N]>`
- `impl<T, const N: usize> AsRef<[MaybeUninit<T>; N]> for MaybeUninit<[T; N]>`
- `impl<T, const N: usize> AsRef<[MaybeUninit<T>]> for MaybeUninit<[T; N]>`
- `impl<T, const N: usize> AsMut<[MaybeUninit<T>; N]> for MaybeUninit<[T; N]>`
- `impl<T, const N: usize> AsMut<[MaybeUninit<T>]> for MaybeUninit<[T; N]>`
- `impl<T, const N: usize> From<MaybeUninit<[T; N]>> for [MaybeUninit<T>; N]`
- `impl<T, const N: usize> AsRef<[Cell<T>; N]> for Cell<[T; N]>`
- `impl<T, const N: usize> AsRef<[Cell<T>]> for Cell<[T; N]>`
- `impl<T> AsRef<[Cell<T>]> for Cell<[T]>`
@Jules-Bertholet Jules-Bertholet force-pushed the more_conversion_trait_impls branch from 8804742 to 5785a90 Compare September 14, 2025 15:39
@rustbot
Copy link
Collaborator

rustbot commented Sep 14, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@joshtriplett
Copy link
Member

We talked about these in today's @rust-lang/libs-api meeting.

The ones that stuck out to us were the impl AsRef<str> for AsciiChar and impl AsMut<str> for AsciiChar. Those seem correct, but we didn't see those specifically proposed anywhere, and it seemed odd to us to have impls for AsciiChar but not [AsciiChar] or [AsciiChar; N].

Could you elaborate on these, and point to where they might have been discussed? Was there a discussion of those specifically, or are you just proposing them because Char::as_str exists? If so, given that [Char]::as_str also exists, could you please provide the corresponding traits consistently for arrays/slices of Char?

@joshtriplett
Copy link
Member

joshtriplett commented Sep 23, 2025

cc @the8472 for potential additional questions/comments.

@Jules-Bertholet
Copy link
Contributor Author

Could you elaborate on these, and point to where they might have been discussed? Was there a discussion of those specifically, or are you just proposing them because Char::as_str exists?

Yes.

If so, given that [Char]::as_str also exists, could you please provide the corresponding traits consistently for arrays/slices of Char?

Unfortunately, alloc fails to build (“type annotations needed”) when I do that.

@joshtriplett
Copy link
Member

Following up for clarity: other than AsRef<str> for AsciiChar and AsMut<str> for AsciiChar, we're fine approving all of these.

If we can't consistently add trait impls for [Char] as well due to inference failure, then please drop those two impls, and let's ship the rest.

@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 30, 2025
@Jules-Bertholet
Copy link
Contributor Author

please drop those two impls

Done.

@tgross35 tgross35 added S-waiting-on-t-libs-api Status: Awaiting decision from T-libs-api and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 20, 2025
@the8472
Copy link
Member

the8472 commented Oct 21, 2025

@rfcbot merge

@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Oct 21, 2025

Team member @the8472 has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 21, 2025
@the8472 the8472 removed the S-waiting-on-t-libs-api Status: Awaiting decision from T-libs-api label Oct 21, 2025
@tgross35
Copy link
Contributor

@the8472 should fcp be restarted without the T-libs label?

@the8472
Copy link
Member

the8472 commented Oct 21, 2025

@rfcbot cancel

@rust-rfcbot
Copy link
Collaborator

@the8472 proposal cancelled.

@rust-rfcbot rust-rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 21, 2025
@the8472 the8472 removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Oct 21, 2025
@the8472
Copy link
Member

the8472 commented Oct 21, 2025

@rfcbot merge

@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Oct 21, 2025

Team member @the8472 has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.