liballoc: introduce String, Vec const-slicing #128399

mammothbane · 2024-07-30T14:45:32Z

This change const-qualifies many methods on Vec and String, notably as_slice, as_str, len. These changes are made behind the unstable feature flag const_vec_string_slice.

Motivation

This is to support simultaneous variance over ownership and constness. I have an enum type that may contain either String or &str, and I want to produce a &str from it in a possibly-const context.

enum StrOrString<'s> {
    Str(&'s str),
    String(String),
}

impl<'s> StrOrString<'s> {
    const fn as_str(&self) -> &str {
        match self {
             // In a const-context, I really only expect to see this variant, but I can't switch the implementation
             // in some mode like #[cfg(const)] -- there has to be a single body
             Self::Str(s) => s,

             // so this is a problem, since it's not `const`
             Self::String(s) => s.as_str(), 
        }
    }
}

Currently String and Vec don't support this, but can without functional changes. Similar logic applies for len, capacity, is_empty.

Changes

The essential thing enabling this change is that Unique::as_ptr is const. This lets us convert RawVec::ptr -> Vec::as_ptr -> Vec::as_slice -> String::as_str.

I had to move the Deref implementations into as_{str,slice} because Deref isn't #[const_trait], but I would expect this change to be invisible up to inlining. I moved the DerefMut implementations as well for uniformity.

rustbot · 2024-07-30T14:45:44Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @tgross35 (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

tgross35 · 2024-07-30T15:29:02Z

This shouldn't be implemented as new functions, the existing as_ptr, len etc just get a const keyword and a #[rustc_const_unstable(...)] attribute.

I am not sure that the motivation is that strong but it seems harmless to unstably add const to most of the non-&mut methods. This is an API change though so

r? libs-api

tgross35 · 2024-07-30T21:15:41Z

You may as well change the implementation before libs-api takes a look since it's pretty easy, and much of this PR as-is won't be needed. Please do the updates to rustc_const_unstable as mentioned above. Including len, is_empty, as_bytes, and as_slice seems reasonable too.

@rustbot author

mammothbane · 2024-07-31T17:59:38Z

Thanks for the review! Made those changes and added capacity on the same logic.

@rustbot review

bors · 2024-08-12T04:59:03Z

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

Amanieu · 2024-08-11T13:52:45Z

library/alloc/src/string.rs

@@ -1006,7 +1006,8 @@ impl String {
 #[inline]
 #[must_use = "`self` will be dropped if the result is not used"]
 #[stable(feature = "rust1", since = "1.0.0")]
- pub fn into_bytes(self) -> Vec<u8> {
+ #[rustc_const_unstable(feature = "const_vec_string_slice", issue = "none")]


This needs a tracking issue so that we can eventually const-stabilize it.

I've created #129041

Can you update all the rustc_const_unstable to point to that tracking issue? Then it should be ready to merge.

I am not the author of the PR, nor am I a member of the org, so I do not have permissions to do so. Ping @mammothbane, as this PR needs a rebase anyways.

GrigorenkoPV · 2024-08-24T22:26:17Z

@rustbot author

mammothbane · 2024-09-04T17:44:47Z

Sorry for the delays here — my dev env broke and it's taken me a while to get around to fixing it and verifying my rebase. Working on it now.

tgross35 · 2024-09-05T17:51:16Z

I know you're probably not done yet, but please make sure you squash at some point

mammothbane · 2024-09-05T18:39:03Z

Squashed -- all changes made afaik.

@rustbot review

bors · 2024-09-14T16:52:56Z

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

Amanieu · 2024-09-15T11:05:02Z

@rust-lang/wg-const-eval I don't think this introduces any const-eval issues, but just double-checking with you.

r=me once conflicts are resolved.

RalfJung · 2024-09-15T11:08:58Z

library/alloc/src/string.rs

@@ -1042,7 +1044,7 @@ impl String {
 #[must_use]
 #[stable(feature = "string_as_str", since = "1.7.0")]
 pub fn as_mut_str(&mut self) -> &mut str {


FWIW it should be fine to also do this with the mut methods

oli-obk · 2024-09-15T11:32:46Z

library/alloc/src/string.rs

- self
+ #[rustc_const_unstable(feature = "const_vec_string_slice", issue = "129041")]
+ pub const fn as_str(&self) -> &str {
+ unsafe { str::from_utf8_unchecked(self.vec.as_slice()) }


Why doesn't tidy require a SAFETY comment here?

This change `const`-qualifies many methods on Vec and String, notably `as_slice`, `as_str`, `len`. These changes are made behind the unstable feature flag `const_vec_string_slice` with the following tracking issue: rust-lang#129041

bors · 2024-09-19T21:41:33Z

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

rustbot assigned tgross35 Jul 30, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 30, 2024

rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 30, 2024

rustbot assigned Amanieu and unassigned tgross35 Jul 30, 2024

tgross35 removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 30, 2024

rustbot 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 Jul 30, 2024

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 31, 2024

Amanieu approved these changes Aug 13, 2024

View reviewed changes

GrigorenkoPV mentioned this pull request Aug 13, 2024

Tracking Issue for const_vec_string_slice #129041

Open

3 tasks

rustbot 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 Aug 24, 2024

mammothbane force-pushed the master branch from 06d03a0 to b45fec3 Compare September 5, 2024 17:06

mammothbane force-pushed the master branch from e6ee5d4 to 52f9d5d Compare September 5, 2024 18:37

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 5, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 5, 2024

RalfJung reviewed Sep 15, 2024

View reviewed changes

oli-obk reviewed Sep 15, 2024

View reviewed changes

mammothbane force-pushed the master branch from 52f9d5d to 9ac23f5 Compare September 19, 2024 10:53

This comment has been minimized.

Sign in to view

liballoc: introduce String, Vec const-slicing

fba2983

This change `const`-qualifies many methods on Vec and String, notably `as_slice`, `as_str`, `len`. These changes are made behind the unstable feature flag `const_vec_string_slice` with the following tracking issue: rust-lang#129041

mammothbane force-pushed the master branch from 8233cf9 to fba2983 Compare September 19, 2024 11:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

liballoc: introduce String, Vec const-slicing #128399

liballoc: introduce String, Vec const-slicing #128399

mammothbane commented Jul 30, 2024 •

edited

Loading

rustbot commented Jul 30, 2024

tgross35 commented Jul 30, 2024

tgross35 commented Jul 30, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mammothbane commented Jul 31, 2024

bors commented Aug 12, 2024

Amanieu Aug 11, 2024

GrigorenkoPV Aug 13, 2024

Amanieu Aug 13, 2024

GrigorenkoPV Aug 13, 2024

GrigorenkoPV commented Aug 24, 2024

mammothbane commented Sep 4, 2024

tgross35 commented Sep 5, 2024

mammothbane commented Sep 5, 2024

bors commented Sep 14, 2024

Amanieu commented Sep 15, 2024

RalfJung Sep 15, 2024

oli-obk Sep 15, 2024 •

edited

Loading

This comment has been minimized.

bors commented Sep 19, 2024

liballoc: introduce String, Vec const-slicing #128399

Are you sure you want to change the base?

liballoc: introduce String, Vec const-slicing #128399

Conversation

mammothbane commented Jul 30, 2024 • edited Loading

Motivation

Changes

rustbot commented Jul 30, 2024

tgross35 commented Jul 30, 2024

tgross35 commented Jul 30, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mammothbane commented Jul 31, 2024

bors commented Aug 12, 2024

Amanieu Aug 11, 2024

Choose a reason for hiding this comment

GrigorenkoPV Aug 13, 2024

Choose a reason for hiding this comment

Amanieu Aug 13, 2024

Choose a reason for hiding this comment

GrigorenkoPV Aug 13, 2024

Choose a reason for hiding this comment

GrigorenkoPV commented Aug 24, 2024

mammothbane commented Sep 4, 2024

tgross35 commented Sep 5, 2024

mammothbane commented Sep 5, 2024

bors commented Sep 14, 2024

Amanieu commented Sep 15, 2024

RalfJung Sep 15, 2024

Choose a reason for hiding this comment

oli-obk Sep 15, 2024 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

bors commented Sep 19, 2024

mammothbane commented Jul 30, 2024 •

edited

Loading

tgross35 commented Jul 30, 2024 •

edited

Loading

oli-obk Sep 15, 2024 •

edited

Loading