Add pattern matching API to OsStr, second attempt by pacak · Pull Request #160971 · rust-lang/rust

pacak · 2026-08-12T08:45:49Z

This is an attempt at reviving #109350

Description

As much as possible I left the original code and commit structure intact. There are a few places where rust code changed - that I had to fix.

Original attempt had several bugs in the implementation - I added tests before the commit that would break them and fixed the problem in the commit that would break them.

~~New instances are breaking diagnostics in an unexpected way: #160710 #160717, after poking at it I have a rough idea what's wrong. I guess I'll have to look into fixing that myself.~~ Ended up replacing them with associated functions. Breaks too much unrelated code.

Original description

This is a sizeable patchset so when reviewing looking at individual commits (rather than the whole changeset) is advisable.

The motivation for this PR is parsing command line arguments. It adds {starts,ends}_with, strip_{prefix,suffix}, {,r}split_once and split methods to OsStr supporting char, &str and FnMut(char) -> bool patterns. (Other methods can be easily added once general consensus for this PR is reached).

Note that this PR doesn’t implement #49802 and doesn’t allow OsStr to be a pattern. This is done because:

in vast majority of cases this is not necessary,
OSStr indexing proposed in the RFC is complex to implement and error-prone to use and
this PR is forwards compatible with the RFC so if someone really needs &OsStr as a pattern it can be added at later time.

This PR also sort of implements the new Pattern API. As I understand it’s no longer a thing, but I’ve decided to keep the change in because it does allow common interface and code sharing. (Though I have some doubts about the actual interface; for example I question existence of Searcher::next method). Keep in mind this is just a means to an end so if messing about with core::str::pattern would be a blocker I can undo those changes.

The core idea with this PR is introduction of core::str_bytes::Byte type which handles byte slices which are possibly invalid UTF-8. str and OsStr are kind of Bytes. With that, pattern matching has to be implemented only once for Bytes type so that the same matching code doesn’t have to be duplicated for str and OsStr. Bytes can have Flavours (UTF-8, WTF-8 or unstructured) which allow implementing optimization based on str being valid UTF-8 or OsStr on Windows being valid WTF-8.

rustbot · 2026-08-12T08:45:53Z

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

Any special-casing of Miri in the standard library requires review.

cc @rust-lang/miri

rustbot · 2026-08-12T08:45:56Z

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: libs
libs expanded to 12 candidates
Random selection from JohnTitor, Mark-Simulacrum, clarfonthey, nia-e

pacak · 2026-08-12T08:49:16Z

pacak marked this pull request as draft

Pull request is feature complete and ready for review. I wanted to create it as a draft to get a CI run (original had problems with windows) without disturbing anyone, but github UI was not cooperative. So much for not disturbing...

The sys::os_str::{Buf, Slice} types are only used within the std crate and not actually reexported. They don't need to be public. This might result in a better generated code, but more importantly it avoids some compile errors down the line.

@pacak

Add a Haystack trait describing something that can be searched in and make core::str::Pattern (and related types) generic on that trait. This will allow Pattern to be used for types other than str (most notably OsStr). This somewhat follows the Pattern API 2.0 design. While that design is apparently abandoned (?), it is somewhat helpful when going for patterns on OsStr, so I’m going with it unless someone tells me otherwise. ;) For now leave Pattern, Haystack et al in core::str::pattern. Since they are no longer str-specific, I’ll move them to core::pattern in future commit. This one leaves them in place to make the diff smaller. @pacak: I moved some (or all new) of the `P: Pattern<&'a str> constraints into where clause to keep things narrower: ``` pub fn foo<'a, P: Pattern<&'a str>>(&'a self, pat: P, ...) ... ``` to ``` pub fn replacen<'a, P>(&'a self, pat: P, ...) ... where P: Pattern<&'a str>, ``` Original code had indices in Haystack abstracted as an associated type Cursor. Replaced with usize - Cursor adds noise with not much value. Changed wording in 2-3 places - for example Searcher is generic over a few types so it makes more sense to talk about split points in general with utf8 split points as an example for `&str`.

Pattern is no longer str-specific, so move it from core::str::pattern module to a new core::pattern module. This introduces no changes in behaviour or implementation. Just moves stuff around and adjusts documentation.

@pacak

Introduce core::pattern::Split and core::pattern::SplitN internal types which can be used to implement iterators splitting haystack into parts. Convert str’s Split-family of iterators to use them. In the future, more haystacks will use those internal types. Co-authored-by: Peter Jaszkowiak <p.jaszkow@gmail.com> @pacak: Fixed some typos, added a few `#[inline]`. Since there's no `H::Cursor` - I had to add `ctx: PhantomData<H>`.

@pacak

Introduce core::pattern::EmptyNeedleSearcher internal type which implements logic for matching an empty pattern against a haystack. Convert core::str::pattern::StrSearcher to use it. In future more implementations will take advantage of it. Also adapt and rework TwoWayStrategy into an internal SearchResult trait which abstracts differences between Searcher’s next, next_match and next_rejects methods. It makes it simpler to write a single generic method implementing optimised versions of all those calls. @pacak: - Fixed a few typos. - There's no H::Cursor parameter so code gets a bit simplified. - Added a test to assert how TwoWaySearcher runs with EmptyNeedleSearcher

@pacak

@pacak: - made more things const fn - there was a (copy-paste?) error in try_finish_byte_sequence so I added a test that checks try_next_code_point(_reverse) with some values, including invalid ones. - reworded a few comments (passive voice, etc) Also different comments: since former is public and later is private due to historical reasons. > This is different than [`next_code_point`] in that it doesn't assume > This is different than `next_code_point_reverse` in that it doesn't assume

@pacak

Firstly, combine functions and results lists into a single list with 'function => result' pairs. This makes it easier to match function with its result. Secondly, eliminate InRange step so that it's easier to notice series of matches or rejects. @pacak: I added a variant to test_stress_indices that matches stuff

@pacak

@pacak: The problem is that current two way matchers are a bit underspecified/undertested. Next commit would break some functionality without breaking any tests for matchers or any tests for the functionality it breaks.

@pacak

Introduce a new core::str_bytes module with types and functions which handle string-like bytes slices. String-like means that they code treats UTF-8 byte sequences as characters within such slices but doesn't assume that the slices are well-formed. A `str` is trivially a bytes sequence that the module can handle but so is OsStr (which is WTF-8 on Windows and unstructured bytes on Unix). Move bunch of code (most notably implementation of the two-way string-matching algorithm) from core::str to core::str_bytes. Note that this likely introduces regression in some of the str function performance (since the new code cannot assume well-formed UTF-8). This is going to be rectified by following commit which will make it again possible for the code to assume bytes format. This is not done in this commit to keep it smaller. @pacak: - Added a few comments - tried to hide internal types from the diagnostic And then there's two different bugs where it would report matched areas as rejected. This broke str::trim_end_matches and who knows what else. Caught it thanks to tests in the previous commit. And one underflow bug on invalid input.

It works right now, but original implementation of the next commit breaks them with none of existing tests catching this regression.

@pacak

Since core::str_bytes module cannot assume byte slices it deals with are well-formed UTF-8 (or even WTF-8), the code must be defensive and accept invalid sequences. This eliminates optimisations which would be otherwise possible. Introduce a `Flavour` trait which tags `Bytes` type with information about the byte sequence. For example, if a `Bytes` object is created from `&str` it’s tagged with `Utf8` flavour which gives the code freedom to assume data is well-formed UTF-8. This brings back all the optimisations removed in previous commit. @pacak: - removed IS_WTF8 associated constant - unused - fixed a bug related to multibyte reverse matching: `next_code_point_reverse` reads the input via Iterator::next_back, passing `bytes.iter().rev()` reverses it a second time. Not good.

Implement Haystack for &OsStr and Pattern<&OsStr> for &str, char and Predicate. Furthermore, add prefix/suffix matching/stripping and splitting methods to OsStr type to make use of those patterns. Using OsStr as a pattern is *not* implemented. Neither is indexing into OsStr. All matching and indexing has to be done via provided functions.

To work around orphan rules, introduce a wrapper type for predicate functions to be used as pattern. Specefically, if we want to add predicat pattern implementation for OsStr type, doing it with a naked `FnMut` results in compile-time errors: error[E0210]: type parameter `F` must be covered by another type when it appears before the first local type (`OsStr`) impl<'hs, F: FnMut(char) -> bool> core::pattern::Pattern<&'hs OsStr> for F { ^ type parameter `F` must be covered by another type when it appears before the first local type (`OsStr`)

Due to technical limitations adding support for predicate as patterns on OsStr slices must be done via core::pattern::Predicate wrapper type. This isn’t ideal but for the time being it’s the best option I've came up with. The core of the issue (as I understand it) is that FnMut is a foreign type in std crate where OsStr is defined. Using predicate as a pattern on OsStr is the final piece which now allows parsing command line arguments.

pacak · 2026-08-13T10:14:35Z

I think it's ready. CI passes, each commit compiles and tests are passing. I ended up removing From instances to convert between Bytes and &str/&[u8] - they would break a bunch of code, they are methods on Bytes instead. Instances to convert between Bytes and &OsStr are still there. Can't be methods on Bytes (Bytes lives in core, OsStr - in std). Can be methods on OsStr, but I think it would be more confusing.

This PR also touches stdarch since type signature for Pattern is a bit different now. I imagine the same change will have to be merged there as well (?).

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

rustbot assigned Mark-Simulacrum Aug 12, 2026

pacak marked this pull request as draft August 12, 2026 08:45

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 12, 2026

pacak marked this pull request as ready for review August 12, 2026 08:49

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 Aug 12, 2026