tracking issue for strip_prefix, strip_suffix for str #67302
Comments
jonas-schievink
added
B-unstable
|
The code generated by |
|
Oof that does indeed look pretty bad. |
|
I whipped up that fix here: #69784 Not sure if it's actually the ideal fix, but figured it would be a good point to launch the discussion from. |
…=kennytm Optimize strip_prefix and strip_suffix with str patterns As mentioned in rust-lang#67302 (comment). I'm not sure whether adding these methods to `Pattern` is desirable—but they have default implementations so the change is backwards compatible. Plus it seems like they're slated for wholesale replacement soon anyway? rust-lang#56345 ---- Constructing a Searcher in strip_prefix and strip_suffix is unnecessarily slow when the pattern is a fixed-length string. Add strip_prefix and strip_suffix methods to the Pattern trait, and add optimized implementations of these methods in the str implementation. The old implementation is retained as the default for these methods.
…=kennytm Optimize strip_prefix and strip_suffix with str patterns As mentioned in rust-lang#67302 (comment). I'm not sure whether adding these methods to `Pattern` is desirable—but they have default implementations so the change is backwards compatible. Plus it seems like they're slated for wholesale replacement soon anyway? rust-lang#56345 ---- Constructing a Searcher in strip_prefix and strip_suffix is unnecessarily slow when the pattern is a fixed-length string. Add strip_prefix and strip_suffix methods to the Pattern trait, and add optimized implementations of these methods in the str implementation. The old implementation is retained as the default for these methods.
…=kennytm Optimize strip_prefix and strip_suffix with str patterns As mentioned in rust-lang#67302 (comment). I'm not sure whether adding these methods to `Pattern` is desirable—but they have default implementations so the change is backwards compatible. Plus it seems like they're slated for wholesale replacement soon anyway? rust-lang#56345 ---- Constructing a Searcher in strip_prefix and strip_suffix is unnecessarily slow when the pattern is a fixed-length string. Add strip_prefix and strip_suffix methods to the Pattern trait, and add optimized implementations of these methods in the str implementation. The old implementation is retained as the default for these methods.
|
Ok, the code should be far more optimized now. What remains to be done? The main issue description has yet to be filled in. 😄 |
|
On some criterion benchmarks that I did, I'm seeing that fn strip_suffix(input: &str) -> &str {
input.strip_suffix(char::is_whitespace).unwrap_or(input)
}is much faster than |
|
Ah, yeah, looks the optimizations I made to strip_prefix/strip_suffix in #69784 are optimizations that don't exist for Lines 4137 to 4148 in 20fc02f I wonder if By the way, since I'm here, @Dylan-DPC (or anyone else on the Rust team), what remains to be done for this issue? |
If you want to change the implementation this is a good time. Else, the thing left is stabilisation with the libs team approving it (if needed) but i'd wait for some time to check if there are any other issues. |
|
The implementation is now faster than the existing similar stable functions |
|
@benesch you can submit a stabilisation PR by following the instructions here: https://forge.rust-lang.org/libs/maintaining-std.html?highlight=stabi#when-a-feature-is-being-stabilized. |
Stabilize str_strip feature
This PR stabilizes these APIs:
```rust
impl str {
/// Returns a string slice with the prefix removed.
///
/// If the string starts with the pattern `prefix`, `Some` is returned with the substring where
/// the prefix is removed. Unlike `trim_start_matches`, this method removes the prefix exactly
/// once.
pub fn strip_prefix<'a, P: Pattern<'a>>(&'a self, prefix: P) -> Option<&'a str>;
/// Returns a string slice with the suffix removed.
///
/// If the string ends with the pattern `suffix`, `Some` is returned with the substring where
/// the suffix is removed. Unlike `trim_end_matches`, this method removes the suffix exactly
/// once.
pub fn strip_suffix<'a, P>(&'a self, suffix: P) -> Option<&'a str>
where
P: Pattern<'a>,
<P as Pattern<'a>>::Searcher: ReverseSearcher<'a>;
}
```
Closes rust-lang#67302
Stabilize str_strip feature
This PR stabilizes these APIs:
```rust
impl str {
/// Returns a string slice with the prefix removed.
///
/// If the string starts with the pattern `prefix`, `Some` is returned with the substring where
/// the prefix is removed. Unlike `trim_start_matches`, this method removes the prefix exactly
/// once.
pub fn strip_prefix<'a, P: Pattern<'a>>(&'a self, prefix: P) -> Option<&'a str>;
/// Returns a string slice with the suffix removed.
///
/// If the string ends with the pattern `suffix`, `Some` is returned with the substring where
/// the suffix is removed. Unlike `trim_end_matches`, this method removes the suffix exactly
/// once.
pub fn strip_suffix<'a, P>(&'a self, suffix: P) -> Option<&'a str>
where
P: Pattern<'a>,
<P as Pattern<'a>>::Searcher: ReverseSearcher<'a>;
}
```
Closes rust-lang#67302
Error was:
error[E0658]: use of unstable library feature 'str_strip': newly added
--> /usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/impl-serde-0.3.2/src/serialize.rs:97:24
|
97 | let (v, stripped) = v.strip_prefix("0x").map_or((v, false), |v| (v, true));
| ^^^^^^^^^^^^
|
= note: see issue #67302 <rust-lang/rust#67302> for more information
Cc @KodrAus
Will fill the content later. Creating this so that #66735 can be merged and tracked.
The text was updated successfully, but these errors were encountered: