Make char and u8 methods const #79549

YenForYang · 2020-11-30T02:30:10Z

char methods escape_unicode, escape_default, len_utf8, len_utf16, to_ascii_lowercase, eq_ignore_ascii_case can be made const.

u8 methods to_ascii_lowercase, to_ascii_uppercase are required to be const as well.

u8::eq_ignore_ascii_case was additionally made const.

`escape_unicode`, `escape_default`, `len_utf8`, `len_utf16`, to_ascii_lowercase`, `eq_ignore_ascii_case` `u8` methods `to_ascii_lowercase`, `to_ascii_uppercase` also must be made const

rust-highfive · 2020-11-30T02:30:13Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

library/core/src/char/methods.rs

jyn514

LGTM but someone from T-libs should make sure the standard library wants to commit to these being const.

sfackler · 2020-12-02T01:23:57Z

library/core/src/char/methods.rs

    #[inline]
-    pub fn escape_unicode(self) -> EscapeUnicode {
+    pub const fn escape_unicode(self) -> EscapeUnicode {


When would this ever be called in a const context given that it returns an iterator?

Good point, I didn't really consider the practicality for making it const. I'm not sure about the downsides of making it callable in const context, but I'm totally fine with removing it. It'll probably be a good while before const iterators happen anyway.

I'm not sure about the downsides of making it callable in const context, but I'm totally fine with removing it.

Yeah, please remove the iterator functions.

oli-obk · 2020-12-02T11:29:08Z

cc @rust-lang/wg-const-eval

oli-obk · 2020-12-02T11:30:32Z

library/core/src/char/methods.rs

    #[inline]
-    pub fn escape_default(self) -> EscapeDefault {
+    pub const fn escape_default(self) -> EscapeDefault {


same here. I think we should not make functions const fn for now unless their result can actually be used for anything during const eval or there's a good reason to have a constant version of it. Unfortunately we are not yet at the "because we can" stage of making things const

This concern seems to still be outstanding.

lopopolo · 2020-12-30T17:10:24Z

In case you folks are looking for a use case for getting this PR over the line, focaccia is a crate on crates.io that implements several Unicode case folding strategies.

The main part of this implementation is a giant match statement that maps chars to byte sequences representing the case folded result. This function is almost able to be made const, but I'm missing support for char::to_ascii_lowercase: artichoke/focaccia#49.

This commit does everything except add the `const` annotation to the function signature. Adding `const` is blocked on rust-lang/rust#79549. See #49.

oli-obk · 2020-12-30T17:35:46Z

Nominating for stabilization (except for the iterator constructor functions). These functions can all be written in user code today by copy pasting the libcore implementation into user code and adding const to them. Thus they don't really need any deeper analysis for whether we'll always want to keep them as const, even if some internal feature does not end up getting stabilized.

RalfJung · 2020-12-30T17:51:38Z

FWIW, have we done insta-stabilizations before for const things?

m-ou-se · 2021-01-06T14:28:11Z

@YenForYang Can you remove the const from escape_default and escape_unicode, as suggested above? Then I'll kick off the stabilization process.

FWIW, have we done insta-stabilizations before for const things?

Not that I know. But having things unstable first is mostly useful to see if there is a better interface, better names, or there turn out to be some unforseen problems with it, or anything else that we could only learn from having some experience with it first. But I think none of that really applies for making simple functions like this const.

crlf0710 · 2021-01-22T11:36:42Z

@YenForYang Ping from triage! Would you mind address the comments above? Thanks!

lopopolo · 2021-02-13T18:48:42Z

Would it be permissible for me to submit a new PR based on this one with the concerns addressed so this can get merged?

m-ou-se · 2021-02-13T19:14:23Z

@lopopolo Sure. Note that we're at 1.52.0 now (for the since = ".." attribute).

lopopolo · 2021-02-13T23:25:28Z

I've rebased, squashed, and made the suggested changes in a new PR in the interests of getting this merged: #82078.

Make char and u8 methods const char methods `len_utf8`, `len_utf16`, `to_ascii_lowercase`, `eq_ignore_ascii_case` can be made const. `u8` methods `to_ascii_lowercase`, `to_ascii_uppercase` are required to be const as well. `u8::eq_ignore_ascii_case` was additionally made const. Rebase of rust-lang#79549 originally authored by `@YenForYang.` Changes from that PR: - Squashed all commits from rust-lang#79549. - rebased to latest upstream master. - Removed const attributes for `char::escape_unicode` and `char::escape_default`. - Updated `since` attributes for `const` stabilization to 1.52.0. cc `@m-ou-se.`

Make char and u8 methods const char methods `len_utf8`, `len_utf16`, `to_ascii_lowercase`, `eq_ignore_ascii_case` can be made const. `u8` methods `to_ascii_lowercase`, `to_ascii_uppercase` are required to be const as well. `u8::eq_ignore_ascii_case` was additionally made const. Rebase of rust-lang#79549 originally authored by ``@YenForYang.`` Changes from that PR: - Squashed all commits from rust-lang#79549. - rebased to latest upstream master. - Removed const attributes for `char::escape_unicode` and `char::escape_default`. - Updated `since` attributes for `const` stabilization to 1.52.0. cc ``@m-ou-se.``

YenForYang added 2 commits November 29, 2020 20:16

Make char methods const

e92a788

`escape_unicode`, `escape_default`, `len_utf8`, `len_utf16`, to_ascii_lowercase`, `eq_ignore_ascii_case` `u8` methods `to_ascii_lowercase`, `to_ascii_uppercase` also must be made const

u8 methods made const

5ac8379

rust-highfive assigned sfackler Nov 30, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 30, 2020

YenForYang added 3 commits November 29, 2020 20:52

Update methods.rs

0091322

Update mod.rs

a7de9c1

Update methods.rs

0a4416c

jyn514 reviewed Nov 30, 2020

View reviewed changes

library/core/src/char/methods.rs Outdated Show resolved Hide resolved

jyn514 added A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 30, 2020

YenForYang added 2 commits December 1, 2020 14:30

Fix since in rustc_const_stable to next stable

9060936

Fix since in rustc_const_stable to next stable

4337b2a

jyn514 reviewed Dec 1, 2020

View reviewed changes

library/core/src/char/methods.rs Outdated Show resolved Hide resolved

YenForYang marked this pull request as ready for review December 1, 2020 21:28

YenForYang added 2 commits December 1, 2020 15:32

Update methods.rs

b5cf6ad

Update mod.rs

8b303a6

jyn514 approved these changes Dec 1, 2020

View reviewed changes

sfackler reviewed Dec 2, 2020

View reviewed changes

oli-obk reviewed Dec 2, 2020

View reviewed changes

ThijsRay mentioned this pull request Dec 30, 2020

Remove const for escape_default and escape_unicode YenForYang/rust#1

Closed

lopopolo mentioned this pull request Dec 30, 2020

Update mapping::lookup so it is a const function artichoke/focaccia#49

Merged

lopopolo mentioned this pull request Dec 30, 2020

Prepare to make mapping::lookup a const function artichoke/focaccia#50

Merged

oli-obk added the I-nominated label Dec 30, 2020

m-ou-se removed the I-nominated label Jan 6, 2021

m-ou-se assigned m-ou-se and unassigned sfackler Jan 6, 2021

m-ou-se 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 Jan 6, 2021

lopopolo mentioned this pull request Feb 13, 2021

Make char and u8 methods const #82078

Merged

m-ou-se closed this Feb 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make char and u8 methods const #79549

Make char and u8 methods const #79549

YenForYang commented Nov 30, 2020 •

edited by m-ou-se

Loading

rust-highfive commented Nov 30, 2020

jyn514 left a comment

sfackler Dec 2, 2020

YenForYang Dec 3, 2020

RalfJung Dec 30, 2020

oli-obk commented Dec 2, 2020

oli-obk Dec 2, 2020 •

edited

Loading

RalfJung Dec 30, 2020

lopopolo commented Dec 30, 2020

oli-obk commented Dec 30, 2020

RalfJung commented Dec 30, 2020 •

edited

Loading

m-ou-se commented Jan 6, 2021

crlf0710 commented Jan 22, 2021

lopopolo commented Feb 13, 2021

m-ou-se commented Feb 13, 2021

lopopolo commented Feb 13, 2021

Make char and u8 methods const #79549

Make char and u8 methods const #79549

Conversation

YenForYang commented Nov 30, 2020 • edited by m-ou-se Loading

rust-highfive commented Nov 30, 2020

jyn514 left a comment

Choose a reason for hiding this comment

sfackler Dec 2, 2020

Choose a reason for hiding this comment

YenForYang Dec 3, 2020

Choose a reason for hiding this comment

RalfJung Dec 30, 2020

Choose a reason for hiding this comment

oli-obk commented Dec 2, 2020

oli-obk Dec 2, 2020 • edited Loading

Choose a reason for hiding this comment

RalfJung Dec 30, 2020

Choose a reason for hiding this comment

lopopolo commented Dec 30, 2020

oli-obk commented Dec 30, 2020

RalfJung commented Dec 30, 2020 • edited Loading

m-ou-se commented Jan 6, 2021

crlf0710 commented Jan 22, 2021

lopopolo commented Feb 13, 2021

m-ou-se commented Feb 13, 2021

lopopolo commented Feb 13, 2021

YenForYang commented Nov 30, 2020 •

edited by m-ou-se

Loading

oli-obk Dec 2, 2020 •

edited

Loading

RalfJung commented Dec 30, 2020 •

edited

Loading