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.

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

Tracking Issue for const_char_encode_utf8 #130512

Closed
4 tasks done
bjoernager opened this issue Sep 18, 2024 · 14 comments · Fixed by #131463
Closed
4 tasks done

Tracking Issue for const_char_encode_utf8 #130512

bjoernager opened this issue Sep 18, 2024 · 14 comments · Fixed by #131463
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@bjoernager
Copy link
Contributor

bjoernager commented Sep 18, 2024

Feature gate: #![feature(const_char_encode_utf8)]

This is a tracking issue for supporting char::encode_utf8 in const scenarios.

Public API

impl char {
    pub const fn encode_utf8(self, dst: &mut [u8]) -> &mut str;
}

Steps / History

Unresolved Questions

  • None yet.
@bjoernager bjoernager added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 18, 2024
@dtolnay
Copy link
Member

dtolnay commented Sep 19, 2024

@rfcbot
Copy link

rfcbot commented Sep 19, 2024

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

@rfcbot 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 Sep 19, 2024
@BurntSushi
Copy link
Member

I'll note that in the implementation PR, making this const seems to require making the panic error message worse (even in the case where encode_utf8 isn't used in a const context) and also introducing more uses of unsafe. I'm not especially happy about that.

@bjoernager
Copy link
Contributor Author

The use of unsafe code cannot be avoided as long as slice indexes aren't const-friendly (AFAIK). But I would argue that this use is properly documented and (relatively) invulnerable to UB, although I am open to other views on this.

@RalfJung has also noted that core::intrinsics::const_eval_select could be used to regain full diagnostics in non-const calls – a feature which I would be more than happy trying to implement.

@RalfJung
Copy link
Member

RalfJung commented Sep 19, 2024 via email

@bjoernager
Copy link
Contributor Author

#130611 resolves the diagnostics question. Can this be added without conflicting with the FCP?

@BurntSushi
Copy link
Member

@bjoernager Yeah that should be fine. Thank you! I appreciate that. :-)

@RalfJung
Copy link
Member

RalfJung commented Sep 20, 2024 via email

@bjoernager
Copy link
Contributor Author

Done. :)

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 20, 2024
… r=dtolnay

Address diagnostics regression for `const_char_encode_utf8`.

Relevant tracking issue: rust-lang#130512

This PR regains full diagnostics for non-const calls to `char::encode_utf8`.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 20, 2024
Rollup merge of rust-lang#130611 - bjoernager:const-char-encode-utf8, r=dtolnay

Address diagnostics regression for `const_char_encode_utf8`.

Relevant tracking issue: rust-lang#130512

This PR regains full diagnostics for non-const calls to `char::encode_utf8`.
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 1, 2024
@rfcbot
Copy link

rfcbot commented Oct 1, 2024

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

@bjoernager
Copy link
Contributor Author

Am I supposed to draft a stabilisation PR or is that outside my domain?

@RalfJung
Copy link
Member

RalfJung commented Oct 9, 2024

If you want to then yes you can draft a stabilization PR. :) Please link it to this issue and mention that it must only land once FCP has completed.

@bjoernager
Copy link
Contributor Author

#131463 stabilises this.

@rfcbot 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 11, 2024
@rfcbot
Copy link

rfcbot commented Oct 11, 2024

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.

tgross35 added a commit to tgross35/rust that referenced this issue Oct 11, 2024
… r=RalfJung

Stabilise `const_char_encode_utf8`.

Closes: rust-lang#130512

This PR stabilises the `const_char_encode_utf8` feature gate (i.e. support for `char::encode_utf8` in const scenarios).

Note that the linked tracking issue is currently awaiting FCP.
@bors bors closed this as completed in 8ea41b9 Oct 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 12, 2024
Rollup merge of rust-lang#131463 - bjoernager:const-char-encode-utf8, r=RalfJung

Stabilise `const_char_encode_utf8`.

Closes: rust-lang#130512

This PR stabilises the `const_char_encode_utf8` feature gate (i.e. support for `char::encode_utf8` in const scenarios).

Note that the linked tracking issue is currently awaiting FCP.
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants