Skip to content

Add pattern matching API to OsStr, second attempt - #160971

Draft
pacak wants to merge 25 commits into
rust-lang:mainfrom
pacak:osstr-pattern
Draft

Add pattern matching API to OsStr, second attempt#160971
pacak wants to merge 25 commits into
rust-lang:mainfrom
pacak:osstr-pattern

Conversation

@pacak

@pacak pacak commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

View all comments

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.

rust-lang/libs-team#311

@rustbot

rustbot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

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

rustbot commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

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
pacak marked this pull request as draft August 12, 2026 08:45
@rustbot 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

pacak commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

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...

@pacak
pacak marked this pull request as ready for review August 12, 2026 08:49
@rustbot 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
@rust-log-analyzer

This comment has been minimized.

@pacak
pacak marked this pull request as draft August 12, 2026 09:03
@rustbot 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
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@pacak
pacak marked this pull request as ready for review August 13, 2026 10:07
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 13, 2026
@Mark-Simulacrum

Copy link
Copy Markdown
Member

r? @nia-e -- I probably won't have time to review this large patch set coherently soon. It may be best to start a thread on #t-libs on Zulip and try to find a dedicated reviewer there, and then split this into multiple PRs rather than trying to review this all at the same time.

@rustbot rustbot assigned nia-e and unassigned Mark-Simulacrum Aug 22, 2026
@pacak

pacak commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Each commit mostly makes sense by itself (compiles, tests are passing, etc). I can probably pull out most of the test changes and merge them independently. The rest is at least logically coupled together. Let me poke at #t-libs.

@pacak

pacak commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

zulip

@epage

epage commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Not seeing a link to rust-lang/libs-team#311 which would be relevant for this.

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 23, 2026
std: reduce visibility of some internal OsStr related types

The std::sys::os_str::{Buf, Slice} types are only used within the std crate and not actually exported. Whole `sys` module is private. 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.

This commit is extracted from rust-lang#160971
@pacak
pacak marked this pull request as draft August 23, 2026 17:20
@rustbot 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 23, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 23, 2026
core: refactor tests/pattern.rs tests

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.

I also added a variant to `test_stress_indices` that matches stuff with `|_| true` which wasn't covered.

This commit is extracted from rust-lang#160971
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 23, 2026
std: reduce visibility of some internal OsStr related types

The std::sys::os_str::{Buf, Slice} types are only used within the std crate and not actually exported. Whole `sys` module is private. 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.

This commit is extracted from rust-lang#160971
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 23, 2026
coretests: Add a few tests for backward multibyte predicate

Surprisingly enough there's no rfind tests for multibyte needles, at least it is possible to break this test without breaking anything other test.

This commit is extracted from rust-lang#160971
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 24, 2026
core: refactor tests/pattern.rs tests

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.

I also added a variant to `test_stress_indices` that matches stuff with `|_| true` which wasn't covered.

This commit is extracted from rust-lang#160971
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 24, 2026
std: reduce visibility of some internal OsStr related types

The std::sys::os_str::{Buf, Slice} types are only used within the std crate and not actually exported. Whole `sys` module is private. 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.

This commit is extracted from rust-lang#160971
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 24, 2026
coretests: Add a few tests for backward multibyte predicate

Surprisingly enough there's no rfind tests for multibyte needles, at least it is possible to break this test without breaking anything other test.

This commit is extracted from rust-lang#160971
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 24, 2026
core: refactor tests/pattern.rs tests

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.

I also added a variant to `test_stress_indices` that matches stuff with `|_| true` which wasn't covered.

This commit is extracted from rust-lang#160971
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 24, 2026
std: reduce visibility of some internal OsStr related types

The std::sys::os_str::{Buf, Slice} types are only used within the std crate and not actually exported. Whole `sys` module is private. 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.

This commit is extracted from rust-lang#160971
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 24, 2026
coretests: Add a few tests for backward multibyte predicate

Surprisingly enough there's no rfind tests for multibyte needles, at least it is possible to break this test without breaking anything other test.

This commit is extracted from rust-lang#160971
rust-bors Bot pushed a commit that referenced this pull request Aug 24, 2026
Rollup merge of #161589 - pacak:private-osstr, r=clarfonthey

std: reduce visibility of some internal OsStr related types

The std::sys::os_str::{Buf, Slice} types are only used within the std crate and not actually exported. Whole `sys` module is private. 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.

This commit is extracted from #160971
rust-bors Bot pushed a commit that referenced this pull request Aug 24, 2026
Rollup merge of #161592 - pacak:change-pattern-tests, r=nia-e

core: refactor tests/pattern.rs tests

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.

I also added a variant to `test_stress_indices` that matches stuff with `|_| true` which wasn't covered.

This commit is extracted from #160971
@rust-bors

rust-bors Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

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

rust-bors Bot pushed a commit that referenced this pull request Aug 24, 2026
Rollup merge of #161604 - pacak:test-multibyte, r=nia-e

coretests: Add a few tests for backward multibyte predicate

Surprisingly enough there's no rfind tests for multibyte needles, at least it is possible to break this test without breaking anything other test.

This commit is extracted from #160971
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants