generalize-pattern by pacak · Pull Request #161606 · rust-lang/rust

pacak · 2026-08-23T15:06:50Z

View all comments

Add a type parameter to Pattern so it can work with other haystack types (my goal is OsStr specifically)
Move the whole thing from core::str::Pattern to core::pattern::Pattern to reflect that

Not very useful on its own, but required to extend pattern splitting API to OsStr.

Contains changes from #161604, I'll rebase one that test lands.

Part of the #160971 cinematic universe.

This PR is part of a stack containing 15 PRs:

rustbot · 2026-08-23T15:06:53Z

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

cc @Amanieu, @folkertdev, @sayantn

rustbot · 2026-08-23T15:06:55Z

r? @JohnTitor

rustbot has assigned @JohnTitor.
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-23T15:15:30Z

stdarch is developed in its own repository

I'm not sure what's the right approach here. Sync it after this lands? A separate pull request?

folkertdev · 2026-08-23T15:18:32Z

This is fine, it's a tiny change, we'll handle the sync after it merges.

pacak · 2026-08-23T17:24:55Z

r? @nia-e

pacak · 2026-08-24T00:07:31Z

#161608 (review) - I'm going to try this

teor2345

This looks mostly OK, I just want to check some safety comments and stability risks with experienced reviewers.

View changes since this review

teor2345 · 2026-09-16T21:04:27Z

-        }
+    #[inline(always)]
+    unsafe fn get_unchecked(&self, range: Range<usize>) -> &Self {
+        // SAFETY: Caller promises position is a character boundary.


There are two positions here, start and end, and the str method has extra requirements:
https://doc.rust-lang.org/std/primitive.str.html#method.get_unchecked

Suggested change

// SAFETY: Caller promises position is a character boundary.

// SAFETY: Caller promises start and end positions are both on character boundaries,

// start is not after end, and both are within the slice.

teor2345 · 2026-09-16T21:12:18Z

 }

-unsafe impl<'a> Searcher<'a> for CharSearcher<'a> {
+unsafe impl<'a> Searcher<'a, str> for CharSearcher<'a> {


This is a pre-existing issue: there is no safety comment here. It would be easier to review any changes to the impl if the safety guarantees were documented explicitly. Maybe the safety comment should go on all the next*() methods, because that seems to be what the trait is concerned about?

Here there's no change except the type parameter (and anything that changed on the trait), so this seems more like a nitpick.

There are 14 of these in this file, so this might be best left to another PR. Let's check with an experienced libs reviewer?

teor2345 · 2026-09-16T21:14:35Z

+//!
+//! For more details, see the traits [`Pattern`], [`Haystack`], [`Searcher`],
+//! [`ReverseSearcher`] and [`DoubleEndedSearcher`].  Although this API is
+//! unstable, it is exposed via stable methods on corresponding haystack types.


Note this exposure in stable APIs, which implies some kind of stability guarantee…

teor2345 · 2026-09-16T21:18:12Z

+/// assert_eq!("abcdef_z".find(|ch| ch > 'd' && ch < 'y'), Some(4));
+/// assert_eq!("abcddd_z".find(|ch| ch > 'd' && ch < 'y'), None);
+/// ```
+pub trait Pattern<H: Haystack + ?Sized>: Sized {


So adding this generic could impact stable type inference, even though Pattern itself is unstable.

Do we need a crater run here? Or do we want to rely on the beta crater run later?
(This is a question for an experienced libs reviewer.)

teor2345 · 2026-09-16T21:23:17Z

+    ///   [`cursor_at_front()`][Self::cursor_at_front]),
+    /// - the back of the haystack (as returned by
+    ///   [`cursor_at_back()`][Self::cursor_at_back]), or
+    /// - any cursor returned by a [`Searcher`] or [`ReverseSearcher`].


This might help explain why those positions are always safe.

Suggested change

/// - any cursor returned by a [`Searcher`] or [`ReverseSearcher`].

/// - any cursor returned by the [`Searcher`] or [`ReverseSearcher`] unsafe traits,

/// which are guaranteed to return valid split positions from their `next()` methods.

teor2345 · 2026-09-16T21:31:15Z

+    ///
+    /// # Safety
+    ///
+    /// The range's start and end must be valid haystack split positions,


See the second safety requirement of https://doc.rust-lang.org/std/primitive.str.html#method.get_unchecked

This was implied, but it's better to make safety comments explicit.

Suggested change

/// The range's start and end must be valid haystack split positions,

/// The range's start and end must be valid split positions inside the haystack,

pacak · 2026-09-17T11:19:28Z

I guess I'll rebase one more time once tests are merged.

Not sure what to do with stability guarantees.

I can look into describing safety requirements for searchers, probably a separate commit/PR.

teor2345 · 2026-09-17T13:50:20Z

Not sure what to do with stability guarantees.

Rust has a tool called "crater" to check ecosystem impact, I'm waiting on an experienced libs/types reviewer (which I am not) to see if it's worth doing for this PR specifically.

I can look into describing safety requirements for searchers, probably a separate commit/PR.

Let's also wait for a more experienced reviewer for this decision.

@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: Original code had pattern parametrized by a reference (`Pattern<&str>`), I changed it to be `Pattern<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.

rustbot · 2026-09-17T18:56:00Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

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

rustbot assigned JohnTitor Aug 23, 2026

This was referenced Aug 23, 2026

core: skip emitting rejects if result doesn't cary it #161608

Draft

std: add OsStr pattern benchmarks #161610

Draft

rustbot assigned nia-e and unassigned JohnTitor Aug 23, 2026

pacak force-pushed the generalize-pattern branch from 9471adf to 0a4f4c4 Compare August 24, 2026 09:41

This comment has been minimized.

Sign in to view

pacak marked this pull request as draft August 24, 2026 13:12

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

pacak force-pushed the generalize-pattern branch from 0a4f4c4 to ae668d2 Compare August 25, 2026 13:46

pacak changed the title ~~Generalize std::str::Pattern to be generic over the haystack and rehome it~~ generalize-pattern Aug 25, 2026

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

This comment has been minimized.

Sign in to view

rustbot added the has-merge-commits PR has merge commits, merge with caution. label Aug 25, 2026

pacak marked this pull request as draft August 26, 2026 07:29

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

pacak force-pushed the generalize-pattern branch from ae668d2 to ebc1513 Compare August 26, 2026 07:35

This comment has been minimized.

Sign in to view

pacak force-pushed the generalize-pattern branch from ebc1513 to 7f1c215 Compare August 26, 2026 19:56

pacak force-pushed the generalize-pattern branch from 7f1c215 to ef9cc1d Compare September 16, 2026 16:39

pacak marked this pull request as ready for review September 16, 2026 18:16

teor2345 reviewed Sep 16, 2026

View reviewed changes

pacak force-pushed the generalize-pattern branch 2 times, most recently from 60b9872 to e2acde3 Compare September 17, 2026 11:14

This comment has been minimized.

Sign in to view

pacak added 2 commits September 17, 2026 14:52

core: move Pattern et al to core::pattern module

5ad3f42

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 force-pushed the generalize-pattern branch from e2acde3 to 5ad3f42 Compare September 17, 2026 18:55

	// SAFETY: Caller promises position is a character boundary.
	// SAFETY: Caller promises start and end positions are both on character boundaries,
	// start is not after end, and both are within the slice.

	/// - any cursor returned by a [`Searcher`] or [`ReverseSearcher`].
	/// - any cursor returned by the [`Searcher`] or [`ReverseSearcher`] unsafe traits,
	/// which are guaranteed to return valid split positions from their `next()` methods.

	/// The range's start and end must be valid haystack split positions,
	/// The range's start and end must be valid split positions inside the haystack,

Uh oh!

Conversation

pacak commented Aug 23, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 23, 2026

Uh oh!

rustbot commented Aug 23, 2026

Uh oh!

pacak commented Aug 23, 2026

Uh oh!

folkertdev commented Aug 23, 2026

Uh oh!

pacak commented Aug 23, 2026

Uh oh!

pacak commented Aug 24, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

teor2345 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teor2345 Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

pacak Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teor2345 Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teor2345 Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

teor2345 Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teor2345 Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

pacak Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

teor2345 Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

pacak Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

pacak commented Sep 17, 2026

Uh oh!

teor2345 commented Sep 17, 2026

Uh oh!

This comment has been minimized.

rustbot commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pacak commented Aug 23, 2026 •

edited by rustbot

Loading

teor2345 left a comment •

edited by rustbot

Loading

teor2345 Sep 16, 2026 •

edited

Loading

teor2345 Sep 16, 2026 •

edited

Loading