Stabilize slice::strip_prefix and slice::strip_suffix

[ijackson]

These two methods are useful. The corresponding methods on str are already stable.

I believe that stablising these now would not get in the way of, in the future, extending these to take a richer pattern API a la str's patterns.

Tracking PR: #73413. I also have an outstanding PR to improve the docs for these two functions and the corresponding ones on str: #75078

I have tried to follow the instructions in the dev guide. The part to do with compiler/rustc_feature did not seem applicable. I assume that's because these are just library features, so there is no corresponding machinery in rustc.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (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.

[Mark-Simulacrum]

r? @Amanieu for t-libs approval

[Amanieu]

@rfcbot fcp merge

This API seems fine for now, but rust-lang/rfcs#2500 plans to generalize them to take a Pattern as an argument instead.

[rfcbot]

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

• @Amanieu
• @BurntSushi
• @KodrAus
• @dtolnay
• @sfackler
• @withoutboats

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.

[Mark-Simulacrum]

That seems like it will be at least an inference break - do we want to try to future proof these somehow?

[KodrAus]

The stabilized str variants expose the unstable Pattern trait. Is there anything stopping us from doing that here too?

[tesuji]

@KodrAus std::str::pattern::Pattern is str oriented, .i.e. it only supports string type.
There are plans to generalize Pattern to other types. But those are not implemented.

[ijackson]

Mark Rousskov writes ("Re: [rust-lang/rust] Stabilize slice::strip_prefix and slice::strip_suffix (#77853)"):

That seems like it will be at least an inference break

Good point. Thanks for pointing this out.

- do we want to try to future proof these somehow?

Yes, surely. What if we provided a slice::Pattern trait only implemented for &[T] ? Ian.

…

-- Ian Jackson <[email protected]> These opinions are my own. Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk, that is a private address which bypasses my fierce spamfilter.

[KodrAus]

We discussed this a bit in the recent Libs meeting and felt that if we wanted to bring this forward and stabilize it ahead of reworking the pattern API to be non-str specific then we could reduce some churn by making the signatures generic on a trivial unstable trait that we replace with Pattern when we can.

Something like:

#[unstable] // We never stabilize this trait
pub trait SlicePattern {
    type Item;
    fn as_slice(&self) -> &[Self::Item];
}

impl<T> SlicePattern for [T] {
    type Item = T;

    fn as_slice(&self) -> &[Self::Item] {
        self
    }
}

impl<T> [T] {
    #[stable]
    pub fn strip_prefix<P: SlicePattern<Item = T>>(&self, prefix: P) -> Option<&[T]>;

    #[stable]
    pub fn strip_suffix<P: SlicePattern<Item = T>>(&self, suffix: P) -> Option<&[T]>;
}

We may still get breakage from inference failures, but bring the API closer to what we actually want in the long term, and only need to change the unstable backing trait.

What do you think?

[ijackson]

Ashley Mannix writes ("Re: [rust-lang/rust] Stabilize slice::strip_prefix and slice::strip_suffix (#77853)"):

We discussed this a bit in the recent Libs meeting and felt that if we wanted to bring this forward and stabilize it ahead of reworking the pattern API to be non-str specific then we could reduce some churn by making the signatures generic on a trivial unstable trait that we replace with Pattern when we can.

I thought this might be a possibility and was meaning to suggest it.

We may still get breakage from inference failures, but bring the API closer to what we actually want in the long term, and only need to change the unstable backing trait.

If the inference break is a small enough problem that this approach would be suitable for stabilisation, then great. I will put this item back on my list for "unblocked, next thing is to write code". Ian.

…

-- Ian Jackson <[email protected]> These opinions are my own. Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk, that is a private address which bypasses my fierce spamfilter.

[ijackson]

Hi. I have updated this now. I wrote a bunch of stuff in the commit message which github likes to hide so here it is for your err delectation and delight?

Stablize slice::strip_prefix and strip_suffix, with SlicePattern

We hope later to extend `core::str::Pattern` to slices too, perhaps as
part of stabilising that.  We want to minimise the amount of type
inference breakage when we do that, so we don't want to stabilise
strip_prefix and strip_suffix taking a simple `&[T]`.

@KodrAus suggested the approach of introducing a new perma-unstable
trait, which reduces this future inference break risk.

I found it necessary to make two impls of this trait, as the unsize
coercion don't apply when hunting for trait implementations.

Since SlicePattern's only method returns a reference, and the whole
trait is just a wrapper for slices, I made the trait type be the
non-reference type [T] or [T;N] rather than the reference.  Otherwise
the trait would have a lifetime parameter.

I marked both the no-op conversion functions `#[inline]`.  I'm not
sure if that is necessary but it seemed at the very least harmless.

[KodrAus]

Thanks @ijackson! This looks like a good approach to me.

[Amanieu]

@rust-lang/libs could you fill in checkboxes in #77853 (comment) considering the updated proposal with a trait bound?

[tesuji]

Should we seal this trait?

[Amanieu]

I don't think it is necessary right now since the trait is still unstable.

[Amanieu]

Stability attributes don't work on trait impls, I think you can just remove them.

[jplatte]

They are being rendered by rustdoc, e.g. https://doc.rust-lang.org/core/num/struct.NonZeroU128.html#impl-TryFrom%3CNonZeroU128%3E-1

[Amanieu]

@bors r+

[bors]

📌 Commit 8b2e79d has been approved by Amanieu

[bors]

⌛ Testing commit 8b2e79d with merge 8f0b945...

[bors]

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 8f0b945 to master...

[ijackson]

Yay :-). Thanks all

[tesuji]

This should be 1.51 right ?

[ijackson]

Err, perhaps. I have just tried looking for git tags or something to tell me which Rust versions contain 8f0b945, the relevant commit, but there don't seem to be any tags for beta versions. I guess that makes sense. I found that origin/beta does not contain that commit, and the forge tells me beta is 1.50. So should I send a MR to fix this ?

[tesuji]

This PR is on 1.51.0 milestone according to rustbot.
Yes, someone could send another PR to update the stabilization version.

[ijackson]

This PR is on 1.51.0 milestone according to rustbot. Yes, someone could send another PR to update the stabilization version.

Willdo then, thanks.

…

-- Ian Jackson <[email protected]> These opinions are my own. Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk, that is a private address which bypasses my fierce spamfilter.