Stabilize const_cstr_methods

[tgross35]

This PR seeks to stabilize const_cstr_methods. Fixes most of #101719

New const stable API

impl CStr {
    // depends: memchr
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, FromBytesWithNulError> {...}
    // depends: const_slice_index
    pub const fn to_bytes(&self) -> &[u8] {}
    // depends: pointer casts
    pub const fn to_bytes_with_nul(&self) -> &[u8] {}
    // depends: str::from_utf8
    pub const fn to_str(&self) -> Result<&str, str::Utf8Error> {}
}

I don't think any of these methods will have any issue when CStr becomes a thin pointer as long as memchr is const (which also allows for const strlen) .

Notes

• from_bytes_until_nul relies on const_slice_index, which relies on const_trait_impls, and generally this should be avoided. After talking with Oli, it should be OK in this case because we could replace the ranges with pointer tricks if needed (worst case being those feature gates disappear). Stabilize const_cstr_methods #107624 (comment)
• Making from_ptr const is deferred because it depends on const_eval_select. I have moved this under the new flag const_cstr_from_ptr Stabilize const_cstr_methods #107624 (comment)

cc @oli-obk I think you're the const expert

@rustbot modify labels: +T-libs-api +needs-fcp

[rustbot]

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

The Miri subtree was changed

cc @rust-lang/miri

[tgross35]

r? libs-api

@rustbot label -T-libs

[rustbot]

Failed to set assignee to T-libs-api: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

[tgross35]

#107429 merged, so this is no longer blocked. It needs the wg-const-eval signoff, but should otherwise be good

[tgross35]

I'm not sure whether const stabilization needs an fcp or not, but with the const_slice_index and const_eval_select fixes I think this should be ready for the next step @dtolnay

(Unless you have anything else @oli-obk)

[bors]

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

[dtolnay]

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

This PR stabilizes the const on the following already stable functions, all of which have been stable since Rust 1.10 or older:

impl CStr {
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, FromBytesWithNulError>
    pub const fn to_bytes(&self) -> &[u8]
    pub const fn to_bytes_with_nul(&self) -> &[u8]
    pub const fn to_str(&self) -> Result<&str, str::Utf8Error>
}

Not all of these have a current implementation that is 100% on track for stabilization; for example the following, which uses https://doc.rust-lang.org/1.67.0/std/primitive.slice.html#method.get_unchecked, which doesn't even have a tracking issue but relies on being able to call trait methods inside const:

https://github.com/rust-lang/rust/blob/6c4269301e885c3407da8401f4ad43ee66f01352/library/core/src/ffi/c_str.rs#L569

However, I am confident that all of the functions for which const is being stabilized here could be reimplemented in a lower-level way using raw pointers, if necessary, to not call any trait methods.

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

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.

[m-ou-se]

I believe the plan is still to make &CStr a thin pointer at some point. (So, strlen() to get the slice length.) Would that be harder if the to_ methods were const?

cc @rust-lang/wg-const-eval

[tgross35]

const_slice_index was removed so I updated to_bytes to just use from_raw_parts instead.

FCP needs one more check still @rust-lang/libs-api

[rfcbot]

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

[BurntSushi]

SGTM. One concern I have about "assuming memchr is const" is that it's plausible that the "const version of memchr" is not as fast as what one could do with non-const features (such as dynamically querying the features of the CPU at runtime). My understanding though is that there is an unstable way to say "run this code in a const context, and run this other code otherwise." That's enough to alleviate my concern, although, it would be nice if the underlying mechanism to make that work were also available to users of Rust and not just std.

[rfcbot]

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]

I believe const_eval_select is what you're referring to, which was discussed earlier #107624 (comment) (the reason CStr::from_ptr isn't in this PR). Based on that discussion, it seems like this will likely be intrinsic-stable by the time CStr becomes a thin pointer.

The rust implementation of memchr doesn't use that feature (link) and is what is used for CStr::from_bytes_x.

To summarize the possibilities:

Pointer Configuration	from_ptr	from_bytes_x	len/to_bytes_x
Current (fat)	O(1) best-strlen	O(1) current-memchr	Constant Time
Thin + const_eval_select	CT	O(1) best-memchr	O(1) best-strlen
Thin, no const_eval_select	CT	O(1) current-memchr	O(1) current-memchr (strlen)

So worst case, to_bytes_x will be as fast as our current implementation of from_bytes_x. If we add a const fn len it will have the same worst case.

[tgross35]

@dtolnay would you be able to merge this before the beta branch next week?

[dtolnay]

@bors r+

[bors]

📌 Commit 5cb701f has been approved by dtolnay

It is now in the queue for this repository.