Skip to content

Stabilize char_max_len #145610

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Stabilize char_max_len #145610

wants to merge 1 commit into from
+17 −26

Conversation

@GrigorenkoPV
Copy link
Contributor

@GrigorenkoPV GrigorenkoPV commented Aug 19, 2025 •
edited
Loading

Tracking issue: #121714

r? t-libs-api

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

Closes #121714

@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 19, 2025
@GrigorenkoPV GrigorenkoPV mentioned this pull request Aug 19, 2025
Open
3 tasks
@Amanieu Amanieu added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Aug 20, 2025
@Amanieu
Copy link
Member

Amanieu commented Aug 20, 2025

Nominating for libs-api discussion since I'm not sure we want to stabilize both the associated constants and the free constants in the char module.

@HTGAzureX1212
Copy link
Contributor

HTGAzureX1212 commented Aug 20, 2025 •
edited
Loading

Nominating for libs-api discussion since I'm not sure we want to stabilize both the associated constants and the free constants in the char module.

Would removing the freestanding constants be feasible? All constants in primitive type "modules" seem to be deprecated themselves anyway, we shouldn't add a constant just for it to be deprecated.

@joshtriplett
Copy link
Member

joshtriplett commented Aug 20, 2025

The main reason to keep the freestanding constants is that we don't currently support importing associated constants. You would always have to reference them with the char:: prefix.

@Amanieu
Copy link
Member

Amanieu commented Aug 26, 2025

Given that we've deprecated a lot of the non-associated constants, I think we should only be stabilizing the associated constants.

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Aug 26, 2025 •
edited by joshtriplett
Loading

Team member @Amanieu 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.

@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Aug 26, 2025
@bors
Copy link
Collaborator

bors commented Aug 27, 2025

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

@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cyrgani cyrgani added the S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. label Sep 10, 2025
@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.

@rustbot

This comment has been minimized.

Sign in to view
@rust-rfcbot rust-rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Sep 30, 2025
@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Sep 30, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@rust-rfcbot rust-rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Oct 10, 2025
@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Oct 10, 2025

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bjoernager
Copy link
Contributor

bjoernager commented Oct 10, 2025

@rustbot label -S-waiting-on-fcp -S-waiting-on-review -needs-fcp

@rustbot rustbot removed S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Oct 10, 2025
@krobelus krobelus mentioned this pull request Oct 19, 2025
Closed
@bors
Copy link
Collaborator

bors commented Nov 6, 2025

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

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Nov 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 6, 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.

@GrigorenkoPV
Copy link
Contributor Author

GrigorenkoPV commented Nov 6, 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 Nov 6, 2025
Amanieu
Amanieu reviewed Nov 6, 2025
View reviewed changes
library/core/src/char/mod.rs Outdated
/// The maximum number of bytes required to [encode](char::encode_utf8) a `char` to
/// UTF-8 encoding.
#[unstable(feature = "char_max_len", issue = "121714")]
#[stable(feature = "char_max_len", since = "CURRENT_RUSTC_VERSION")]
Copy link
Member

@Amanieu Amanieu Nov 6, 2025

Choose a reason for hiding this comment

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

In the FCP we decided to only stabilize the associated constants, not the constants in the core::char module.

Copy link
Contributor Author

@GrigorenkoPV GrigorenkoPV Nov 6, 2025

Choose a reason for hiding this comment

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

Fair enough.

Should I just delete them or keep them as perma-unstable?

Copy link
Contributor

@bjoernager bjoernager Nov 6, 2025

Choose a reason for hiding this comment

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

I'd say just keep them as-is as IIRC there hasn't been a formal decision to remove them yet.

Copy link
Contributor Author

@GrigorenkoPV GrigorenkoPV Nov 6, 2025

Choose a reason for hiding this comment

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

Done. They will remain under char_max_len. The newly stabilized constants in impl char are now gated behind the stable char_max_len_assoc feature. This is similar to what I did during the partial stabilization of io_error_more.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Amanieu Amanieu Amanieu left review comments

+1 more reviewer

@bjoernager bjoernager bjoernager left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Amanieu Amanieu

Labels

finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tracking Issue for char_max_len