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 conversions #89259

Closed
est31 opened this issue Sep 25, 2021 · 12 comments · Fixed by #126958
Closed

Tracking issue for const char conversions #89259

est31 opened this issue Sep 25, 2021 · 12 comments · Fixed by #126958
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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

@est31
Copy link
Member

est31 commented Sep 25, 2021

This is a tracking issue for const char conversions. List of functions belonging to !#[feature(const_char_from_u32_unchecked)]:

impl char {
    pub const unsafe fn from_u32_unchecked(i: u32) -> char;
}

// Available through core::char and std::char
mod char {
    pub const unsafe fn from_u32_unchecked(i: u32) -> char;
}

List of functions belonging to stable #![feature(const_char_convert)]:

impl char {
    pub const fn from_u32(self, i: u32) -> Option<char>;
    pub const fn from_digit(self, num: u32, radix: u32) -> Option<char>;
    pub const fn to_digit(self, radix: u32) -> Option<u32>;
}

// Available through core::char and std::char
mod char {
    pub const fn from_u32(i: u32) -> Option<char>;
    pub const fn from_digit(num: u32, radix: u32) -> Option<char>;
}

Steps:

@est31 est31 added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 25, 2021
@leonardo-m

This comment was marked as off-topic.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 18, 2021
Make char conversion functions unstably const

The char conversion functions like `char::from_u32` do trivial computations and can easily be converted into const fns. Only smaller tricks are needed to avoid non-const standard library functions like `Result::ok` or `bool::then_some`.

Tracking issue: rust-lang#89259
JohnTitor added a commit to JohnTitor/rust that referenced this issue Nov 19, 2021
Make char conversion functions unstably const

The char conversion functions like `char::from_u32` do trivial computations and can easily be converted into const fns. Only smaller tricks are needed to avoid non-const standard library functions like `Result::ok` or `bool::then_some`.

Tracking issue: rust-lang#89259
@Nugine
Copy link
Contributor

Nugine commented Sep 29, 2022

Hi! Is there any plan of stabilization?

@est31
Copy link
Member Author

est31 commented Sep 29, 2022

@Nugine I've made a stabilization PR, let's see where this goes: #102470

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 14, 2022
…, r=joshtriplett

Stabilize const char convert

Split out `const_char_from_u32_unchecked` from `const_char_convert` and stabilize the rest, i.e. stabilize the following functions:

```Rust
impl char {
    pub const fn from_u32(self, i: u32) -> Option<char>;
    pub const fn from_digit(self, num: u32, radix: u32) -> Option<char>;
    pub const fn to_digit(self, radix: u32) -> Option<u32>;
}

// Available through core::char and std::char
mod char {
    pub const fn from_u32(i: u32) -> Option<char>;
    pub const fn from_digit(num: u32, radix: u32) -> Option<char>;
}
```

And put the following under the `from_u32_unchecked` const stability gate as it needs `Option::unwrap` which isn't const-stable (yet):

```Rust
impl char {
    pub const unsafe fn from_u32_unchecked(i: u32) -> char;
}

// Available through core::char and std::char
mod char {
    pub const unsafe fn from_u32_unchecked(i: u32) -> char;
}
```

cc the tracking issue rust-lang#89259 (which I'd like to keep open for `const_char_from_u32_unchecked`).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 14, 2022
…, r=joshtriplett

Stabilize const char convert

Split out `const_char_from_u32_unchecked` from `const_char_convert` and stabilize the rest, i.e. stabilize the following functions:

```Rust
impl char {
    pub const fn from_u32(self, i: u32) -> Option<char>;
    pub const fn from_digit(self, num: u32, radix: u32) -> Option<char>;
    pub const fn to_digit(self, radix: u32) -> Option<u32>;
}

// Available through core::char and std::char
mod char {
    pub const fn from_u32(i: u32) -> Option<char>;
    pub const fn from_digit(num: u32, radix: u32) -> Option<char>;
}
```

And put the following under the `from_u32_unchecked` const stability gate as it needs `Option::unwrap` which isn't const-stable (yet):

```Rust
impl char {
    pub const unsafe fn from_u32_unchecked(i: u32) -> char;
}

// Available through core::char and std::char
mod char {
    pub const unsafe fn from_u32_unchecked(i: u32) -> char;
}
```

cc the tracking issue rust-lang#89259 (which I'd like to keep open for `const_char_from_u32_unchecked`).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 14, 2022
…, r=joshtriplett

Stabilize const char convert

Split out `const_char_from_u32_unchecked` from `const_char_convert` and stabilize the rest, i.e. stabilize the following functions:

```Rust
impl char {
    pub const fn from_u32(self, i: u32) -> Option<char>;
    pub const fn from_digit(self, num: u32, radix: u32) -> Option<char>;
    pub const fn to_digit(self, radix: u32) -> Option<u32>;
}

// Available through core::char and std::char
mod char {
    pub const fn from_u32(i: u32) -> Option<char>;
    pub const fn from_digit(num: u32, radix: u32) -> Option<char>;
}
```

And put the following under the `from_u32_unchecked` const stability gate as it needs `Option::unwrap` which isn't const-stable (yet):

```Rust
impl char {
    pub const unsafe fn from_u32_unchecked(i: u32) -> char;
}

// Available through core::char and std::char
mod char {
    pub const unsafe fn from_u32_unchecked(i: u32) -> char;
}
```

cc the tracking issue rust-lang#89259 (which I'd like to keep open for `const_char_from_u32_unchecked`).
@ChrisDenton
Copy link
Member

Stabilization is blocked by #67441 const stability, which is blocked by #73255.

Could we not manually write out a match statement instead of using unwrap?

@est31
Copy link
Member Author

est31 commented Dec 15, 2023

I think the idea was that there should be no extra hacks done just to get things compatible with const fn, but I don't remember it fully any more.

@ChrisDenton
Copy link
Member

I've opened #118979 to see what T-libs thinks about it.

@ChrisDenton
Copy link
Member

#118979 is now merged so I think there's no blocker for from_u32_unchecked being stabilized as const.

@dtolnay
Copy link
Member

dtolnay commented Jun 25, 2024

@rust-lang/libs-api:
@rfcbot fcp merge

I propose stabilizing const for the existing core::char::from_u32_unchecked function (stable since Rust 1.5) and <char>::from_u32_unchecked associated function (stable since Rust 1.52).

These functions were left out of the initial set of feature(const_char_convert) stabilizations in #102470, but have since been unblocked by #118979.

If unsafe { from_u32_unchecked(u) } is called in const with a value for which from_u32(u) returns None, we get the following compile error.

fn main() {
    let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
}
error[E0080]: it is undefined behavior to use this value
 --> src/main.rs:2:19
  |
2 |     let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
  |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0x0000d800, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
  |
  = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
  = note: the raw bytes of the constant (size: 4, align: 4) {
              00 d8 00 00                                     │ ....
          }

note: erroneous constant encountered
 --> src/main.rs:2:13
  |
2 |     let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@rfcbot
Copy link

rfcbot commented Jun 25, 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. 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 Jun 25, 2024
@rfcbot
Copy link

rfcbot commented Jul 2, 2024

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

@RalfJung
Copy link
Member

RalfJung commented Jul 4, 2024

If unsafe { from_u32_unchecked(u) } is called in const with a value for which from_u32(u) returns None, we get the following compile error.

Note that this is the error you get when the final value of the const is an invalid char.

If you ignore the return value of from_u32_unchecked, there is no error. It's still UB, we just don't catch the UB.

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

rfcbot commented Jul 12, 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.

@bors bors closed this as completed in fcaa6fd Jul 13, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jul 16, 2024
Stabilize const unchecked conversion from u32 to char

Closes rust-lang/rust#89259.

The functions in this PR were left out of the initial set of `feature(const_char_convert)` stabilizations in rust-lang/rust#102470, but have since been unblocked by rust-lang/rust#118979.

If `unsafe { from_u32_unchecked(u) }` is called in const with a value for which `from_u32(u)` returns None, we get the following compile error.

```rust
fn main() {
    let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
}
```

```console
error[E0080]: it is undefined behavior to use this value
 --> src/main.rs:2:19
  |
2 |     let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
  |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0x0000d800, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
  |
  = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
  = note: the raw bytes of the constant (size: 4, align: 4) {
              00 d8 00 00                                     │ ....
          }

note: erroneous constant encountered
 --> src/main.rs:2:13
  |
2 |     let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Jul 16, 2024
Stabilize const unchecked conversion from u32 to char

Closes rust-lang/rust#89259.

The functions in this PR were left out of the initial set of `feature(const_char_convert)` stabilizations in rust-lang/rust#102470, but have since been unblocked by rust-lang/rust#118979.

If `unsafe { from_u32_unchecked(u) }` is called in const with a value for which `from_u32(u)` returns None, we get the following compile error.

```rust
fn main() {
    let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
}
```

```console
error[E0080]: it is undefined behavior to use this value
 --> src/main.rs:2:19
  |
2 |     let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
  |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0x0000d800, but expected a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)
  |
  = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
  = note: the raw bytes of the constant (size: 4, align: 4) {
              00 d8 00 00                                     │ ....
          }

note: erroneous constant encountered
 --> src/main.rs:2:13
  |
2 |     let _ = const { unsafe { char::from_u32_unchecked(0xd800) } };
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. 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.

8 participants