Make slice->str conversion and related functions `const` #90607

WaffleLapkin · 2021-11-05T12:51:38Z

This PR marks the following APIs as const:

// core::str
pub const fn from_utf8(v: &[u8]) -> Result<&str, Utf8Error>;
pub const fn from_utf8_mut(v: &mut [u8]) -> Result<&mut str, Utf8Error>;
pub const unsafe fn from_utf8_unchecked_mut(v: &mut [u8]) -> &mut str;

impl Utf8Error {
    pub const fn valid_up_to(&self) -> usize;
    pub const fn error_len(&self) -> Option<usize>;
}

Everything but from_utf8_unchecked_mut uses const_str_from_utf8 feature gate, from_utf8_unchecked_mut uses const_str_from_utf8_unchecked_mut feature gate.

I'm not sure why from_utf8_unchecked_mut was left out being non-const, considering that from_utf8_unchecked is not only const, but const stable.

r? @oli-obk (performance-only const_eval_select use)

Mark-Simulacrum · 2021-11-05T12:56:48Z

library/core/src/str/validations.rs

+    // algorithm correctness can not depend on `align_offset` returning non-max values.
+    //
+    // As such the behaviour can't change after replacing `align_offset` with `usize::MAX`, only performance can.
+    let align = unsafe { const_eval_select((v, usize_bytes), ctfe_align, rt_align) };


It seems better to put this inside align_offset itself, since the justification should work for all calls to it, right?

While I think it would be great to put this inside align_offset itself, wouldn't this allow for user code to behave differently in const and non-const context?

const fn is_likely_called_in_ctfe() -> bool { let arr = [0u8; 2]; (&arr[0] as *const u8).align_offset(2) == usize::MAX && (&arr[1] as *const u8).align_offset(2) == usize::MAX }

(playground)

Actually even (&0u8 as *const u8).align_offset(1) == usize::MAX will do. Basically while const_eval_select in the caller of align_offset shouldn't change behaviour, const_eval_select in align_offset itself clearly changes it's return value depending on whatever it's called at runtime or compile time.

Sadly, it seems that we don't want to allow this, right? :(

const_eval_select in align_offset itself clearly changes it's return value depending on whatever it's called at runtime or compile time.

Sadly, it seems that we don't want to allow this, right? :(

Actually, the docs specifically state

If it is not possible to align the pointer, the implementation returns usize::MAX. It is permissible for the implementation to always return usize::MAX. Only your algorithm’s performance can depend on getting a usable offset here, not its correctness.
There are no guarantees whatsoever that offsetting the pointer will not overflow or go beyond the allocation that the pointer points into. It is up to the caller to ensure that the returned offset is correct in all terms other than alignment.

The only reason align_offset even exists is because I wanted to enable miri to do such trickery back when it wasn't able to do ptr2int conversions.

So, we can definitely make align_offset behave this way in const fn, as long as we have a tracking issue for the const stabilization that specifically raises this concern before we bake it into a stabilized function.

I can make a follow-up PR then

I have a slight preference for doing a PR for just align_offset first, since then this PR does less work, but if you prefer to first merge this one I'm ok with that.

I can do either way if you prefer align_offset first, then so be it :)

Upd: I've opened #90958

library/core/src/str/converts.rs

library/core/src/str/error.rs

library/core/src/str/validations.rs

…oli-obk Mark `<*const _>::align_offset` and `<*mut _>::align_offset` as `const fn` This PR marks the following APIs as `const`: ```rust impl<T> *const T { pub const fn align_offset(self, align: usize) -> usize; } impl<T> *mut T { pub const fn align_offset(self, align: usize) -> usize; } ``` `const` implementation simply returns `usize::MAX`. --- Previous discussion: rust-lang#90607 (comment) --- r? `@oli-obk`

This commit makes the following functions from `core::str` `const fn`: - `from_utf8[_mut]` (`feature(const_str_from_utf8)`) - `from_utf8_unchecked_mut` (`feature(const_str_from_utf8_unchecked_mut)`) - `Utf8Error::{valid_up_to,error_len}` (`feature(const_str_from_utf8)`)

WaffleLapkin · 2021-11-17T22:02:45Z

@oli-obk I've rebased to use #90958 & applied suggestions from your review

oli-obk · 2021-11-18T09:04:15Z

@bors r+ rollup

bors · 2021-11-18T09:04:16Z

📌 Commit cf6f64a has been approved by oli-obk

WaffleLapkin · 2021-11-18T09:52:36Z

wait, shouldn't we create a tracking issue first?

Upd: I've filled in the tracking issues, not sure how bors will work with this

…_utf8_unchecked_mut` features

oli-obk · 2021-11-18T11:24:11Z

Oops thanks!

@bors r+ rollup

bors · 2021-11-18T11:24:13Z

📌 Commit 573a00e has been approved by oli-obk

…=oli-obk Make slice->str conversion and related functions `const` This PR marks the following APIs as `const`: ```rust // core::str pub const fn from_utf8(v: &[u8]) -> Result<&str, Utf8Error>; pub const fn from_utf8_mut(v: &mut [u8]) -> Result<&mut str, Utf8Error>; pub const unsafe fn from_utf8_unchecked_mut(v: &mut [u8]) -> &mut str; impl Utf8Error { pub const fn valid_up_to(&self) -> usize; pub const fn error_len(&self) -> Option<usize>; } ``` Everything but `from_utf8_unchecked_mut` uses `const_str_from_utf8` feature gate, `from_utf8_unchecked_mut` uses `const_str_from_utf8_unchecked_mut` feature gate. --- I'm not sure why `from_utf8_unchecked_mut` was left out being non-`const`, considering that `from_utf8_unchecked` is not only `const`, but **`const` stable**. --- r? `@oli-obk` (performance-only `const_eval_select` use)

…=oli-obk Make slice->str conversion and related functions `const` This PR marks the following APIs as `const`: ```rust // core::str pub const fn from_utf8(v: &[u8]) -> Result<&str, Utf8Error>; pub const fn from_utf8_mut(v: &mut [u8]) -> Result<&mut str, Utf8Error>; pub const unsafe fn from_utf8_unchecked_mut(v: &mut [u8]) -> &mut str; impl Utf8Error { pub const fn valid_up_to(&self) -> usize; pub const fn error_len(&self) -> Option<usize>; } ``` Everything but `from_utf8_unchecked_mut` uses `const_str_from_utf8` feature gate, `from_utf8_unchecked_mut` uses `const_str_from_utf8_unchecked_mut` feature gate. --- I'm not sure why `from_utf8_unchecked_mut` was left out being non-`const`, considering that `from_utf8_unchecked` is not only `const`, but **`const` stable**. --- r? ``@oli-obk`` (performance-only `const_eval_select` use)

Rollup of 8 pull requests Successful merges: - rust-lang#90386 (Add `-Zassert-incr-state` to assert state of incremental cache) - rust-lang#90438 (Clean up mess for --show-coverage documentation) - rust-lang#90480 (Mention `Vec::remove` in `Vec::swap_remove`'s docs) - rust-lang#90607 (Make slice->str conversion and related functions `const`) - rust-lang#90750 (rustdoc: Replace where-bounded Clean impl with simple function) - rust-lang#90895 (require full validity when determining the discriminant of a value) - rust-lang#90989 (Avoid suggesting literal formatting that turns into member access) - rust-lang#91002 (rustc: Remove `#[rustc_synthetic]`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned oli-obk Nov 5, 2021

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

Mark-Simulacrum reviewed Nov 5, 2021

View reviewed changes

apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Nov 11, 2021

oli-obk reviewed Nov 16, 2021

View reviewed changes

library/core/src/str/converts.rs Show resolved Hide resolved

oli-obk reviewed Nov 16, 2021

View reviewed changes

library/core/src/str/error.rs Show resolved Hide resolved

oli-obk reviewed Nov 16, 2021

View reviewed changes

library/core/src/str/validations.rs Show resolved Hide resolved

This was referenced Nov 16, 2021

Mark <*const _>::align_offset and <*mut _>::align_offset as const fn #90958

Merged

Tracking Issue for const_align_offset #90962

Open

WaffleLapkin force-pushed the const_str_from_utf8 branch from f27d3c6 to cf6f64a Compare November 17, 2021 21:58

oli-obk added the const-hack label Nov 18, 2021

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 18, 2021

This was referenced Nov 18, 2021

Tracking Issue for str::from_utf8_unchecked_mut as a const fn #91005

Open

Tracking Issue for const checked slice to str conversions #91006

Open

Fill in tracking issues for const_str_from_utf8 and `const_str_from…

573a00e

…_utf8_unchecked_mut` features

JohnTitor mentioned this pull request Nov 18, 2021

Rollup of 6 pull requests #91013

Closed

JohnTitor mentioned this pull request Nov 18, 2021

Rollup of 9 pull requests #91016

Closed

JohnTitor mentioned this pull request Nov 18, 2021

Rollup of 8 pull requests #91019

Merged

est31 mentioned this pull request Nov 18, 2021

Make char conversion functions unstably const #89258

Merged

bors merged commit 77c985f into rust-lang:master Nov 18, 2021

rustbot added this to the 1.58.0 milestone Nov 18, 2021

WaffleLapkin deleted the const_str_from_utf8 branch November 19, 2021 19:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make slice->str conversion and related functions `const` #90607

Make slice->str conversion and related functions `const` #90607

WaffleLapkin commented Nov 5, 2021

Mark-Simulacrum Nov 5, 2021

WaffleLapkin Nov 13, 2021

WaffleLapkin Nov 13, 2021

oli-obk Nov 16, 2021

WaffleLapkin Nov 16, 2021

oli-obk Nov 16, 2021

WaffleLapkin Nov 16, 2021 •

edited

Loading

WaffleLapkin commented Nov 17, 2021

oli-obk commented Nov 18, 2021

bors commented Nov 18, 2021

WaffleLapkin commented Nov 18, 2021 •

edited

Loading

oli-obk commented Nov 18, 2021

bors commented Nov 18, 2021

Make slice->str conversion and related functions const #90607

Make slice->str conversion and related functions const #90607

Conversation

WaffleLapkin commented Nov 5, 2021

Mark-Simulacrum Nov 5, 2021

Choose a reason for hiding this comment

WaffleLapkin Nov 13, 2021

Choose a reason for hiding this comment

WaffleLapkin Nov 13, 2021

Choose a reason for hiding this comment

oli-obk Nov 16, 2021

Choose a reason for hiding this comment

WaffleLapkin Nov 16, 2021

Choose a reason for hiding this comment

oli-obk Nov 16, 2021

Choose a reason for hiding this comment

WaffleLapkin Nov 16, 2021 • edited Loading

Choose a reason for hiding this comment

WaffleLapkin commented Nov 17, 2021

oli-obk commented Nov 18, 2021

bors commented Nov 18, 2021

WaffleLapkin commented Nov 18, 2021 • edited Loading

oli-obk commented Nov 18, 2021

bors commented Nov 18, 2021

Make slice->str conversion and related functions `const` #90607

Make slice->str conversion and related functions `const` #90607

WaffleLapkin Nov 16, 2021 •

edited

Loading

WaffleLapkin commented Nov 18, 2021 •

edited

Loading