Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement PartialOrd and PartialEq for UTF-8 strings and FFI Strings #76189

Closed
wants to merge 1 commit into from
Closed

Implement PartialOrd and PartialEq for UTF-8 strings and FFI Strings #76189

wants to merge 1 commit into from

Conversation

lopopolo
Copy link
Contributor

@lopopolo lopopolo commented Sep 1, 2020

Implement PartialOrd and PartialEq for UTF-8 strings and FFI Strings

This PR adds many more PartialEq and PartialOrd implementations for
Path, PathBuf, OsStr, OsString, and Component.

The Rust Standard Library guarantees that these types are freely and
cheaply constructable from UTF-8 String and &str, but comparing
these types with &str literals is not ergonomic. One use case I'd like
to enable is filtering out paths containing a "test" directory while
walking a tree and inspecting the path with the components() iterator.

Example:

let is_test_source = relative_source
    .components()
    .any(|component| component.as_os_str() == OsStr::new("test"));

New core trait impls:

OsString:

  • impl PartialEq<String> for OsString and reflexive
  • impl PartialEq<Cow<'_, str>> for OsString and reflexive
  • impl PartialEq<Box<str>> for OsString and reflexive
  • impl PartialEq<Rc<str>> for OsString and reflexive
  • impl PartialEq<Arc<str>> for OsString and reflexive
  • impl PartialOrd<String> for OsString and reflexive
  • impl PartialOrd<Cow<'_, str>> for OsString and reflexive
  • impl PartialOrd<Box<str>> for OsString and reflexive
  • impl PartialOrd<Rc<str>> for OsString and reflexive
  • impl PartialOrd<Arc<str>> for OsString and reflexive

OsStr:

  • impl PartialEq<String> for OsStr and reflexive
  • impl PartialEq<Cow<'_, str>> for OsStr and reflexive
  • impl PartialEq<Box<str>> for OsStr and reflexive
  • impl PartialEq<Rc<str>> for OsStr and reflexive
  • impl PartialEq<Arc<str>> for OsStr and reflexive
  • impl PartialOrd<String> for OsStr and reflexive
  • impl PartialOrd<Cow<'_, str>> for OsStr and reflexive
  • impl PartialOrd<Box<str>> for OsStr and reflexive
  • impl PartialOrd<Rc<str>> for OsStr and reflexive
  • impl PartialOrd<Arc<str>> for OsStr and reflexive

Component:

  • impl PartialEq<OsStr> for Component and reflexive
  • impl PartialEq<&'_ OsStr> for Component and reflexive
  • impl PartialEq<OsString> for Component and reflexive
  • impl PartialEq<Cow<'_, OsStr>> for Component and reflexive
  • impl PartialEq<Box<OsStr>> for Component and reflexive
  • impl PartialEq<Path> for Component and reflexive
  • impl PartialEq<&'_ Path> for Component and reflexive
  • impl PartialEq<PathBuf> for Component and reflexive
  • impl PartialEq<Cow<'_, Path>> for Component and reflexive
  • impl PartialEq<Box<Path>> for Component and reflexive
  • impl PartialEq<str> for Component and reflexive
  • impl PartialEq<String> for Component and reflexive
  • impl PartialEq<Cow<'_, str>> for Component and reflexive
  • impl PartialEq<Box<str>> for Component and reflexive
  • impl PartialEq<Rc<str>> for Component and reflexive
  • impl PartialEq<Arc<str>> for Component and reflexive
  • impl PartialEq<str> for Component and reflexive
  • impl PartialEq<String> for Component and reflexive
  • impl PartialEq<Cow<'_, str>> for Component and reflexive
  • impl PartialEq<Box<str>> for Component and reflexive
  • impl PartialEq<Rc<str>> for Component and reflexive
  • impl PartialEq<Arc<str>> for Component and reflexive
  • impl PartialOrd<OsStr> for Component and reflexive
  • impl PartialOrd<&'_ OsStr> for Component and reflexive
  • impl PartialOrd<OsString> for Component and reflexive
  • impl PartialOrd<Cow<'_, OsStr>> for Component and reflexive
  • impl PartialOrd<Box<OsStr>> for Component and reflexive
  • impl PartialOrd<Path> for Component and reflexive
  • impl PartialOrd<&'_ Path> for Component and reflexive
  • impl PartialOrd<PathBuf> for Component and reflexive
  • impl PartialOrd<Cow<'_, Path>> for Component and reflexive
  • impl PartialOrd<Box<Path>> for Component and reflexive
  • impl PartialOrd<str> for Component and reflexive
  • impl PartialOrd<String> for Component and reflexive
  • impl PartialOrd<Cow<'_, str>> for Component and reflexive
  • impl PartialOrd<Box<str>> for Component and reflexive
  • impl PartialOrd<Rc<str>> for Component and reflexive
  • impl PartialOrd<Arc<str>> for Component and reflexive

PathBuf:

  • impl PartialEq<str> for PathBuf and reflexive
  • impl PartialEq<&'_ str> for PathBuf and reflexive
  • impl PartialEq<String> for PathBuf and reflexive
  • impl PartialEq<Cow<'_, str>> for PathBuf and reflexive
  • impl PartialEq<Box<str>> for PathBuf and reflexive
  • impl PartialEq<Rc<str>> for PathBuf and reflexive
  • impl PartialEq<Arc<str>> for PathBuf and reflexive
  • impl PartialOrd<str> for PathBuf and reflexive
  • impl PartialOrd<&'_ str> for PathBuf and reflexive
  • impl PartialOrd<String> for PathBuf and reflexive
  • impl PartialOrd<Cow<'_, str>> for PathBuf and reflexive
  • impl PartialOrd<Box<str>> for PathBuf and reflexive
  • impl PartialOrd<Rc<str>> for PathBuf and reflexive
  • impl PartialOrd<Arc<str>> for PathBuf and reflexive

Path:

  • impl PartialEq<str> for Path and reflexive
  • impl PartialEq<&'_ str> for Path and reflexive
  • impl PartialEq<String> for Path and reflexive
  • impl PartialEq<Cow<'_, str>> for Path and reflexive
  • impl PartialEq<Box<str>> for Path and reflexive
  • impl PartialEq<Rc<str>> for Path and reflexive
  • impl PartialEq<Arc<str>> for Path and reflexive
  • impl PartialOrd<str> for Path and reflexive
  • impl PartialOrd<&'_ str> for Path and reflexive
  • impl PartialOrd<String> for Path and reflexive
  • impl PartialOrd<Cow<'_, str>> for Path and reflexive
  • impl PartialOrd<Box<str>> for Path and reflexive
  • impl PartialOrd<Rc<str>> for Path and reflexive
  • impl PartialOrd<Arc<str>> for Path and reflexive

Fixes #71700.

@lopopolo
Copy link
Contributor Author

lopopolo commented Sep 1, 2020

CI failed on awscli installation

This PR adds many more `PartialEq` and `PartialOrd` implementations for
`Path`, `PathBuf`, `OsStr`, `OsString`, and `Component`.

The Rust Standard Library guarantees that these types are freely and
cheaply constructable from UTF-8 `String` and `&str`, but comparing
these types with `&str` literals is not ergonomic. One use case I'd like
to enable is filtering out paths containing a "test" directory while
walking a tree and inspecting the path with the `components()` iterator.

Example:

```rust
let is_test_source = relative_source
    .components()
    .any(|component| component.as_os_str() == OsStr::new("test"));
```

New core trait impls:

`OsString`:

- `impl PartialEq<String> for OsString` and reflexive
- `impl PartialEq<Cow<'_, str>> for OsString` and reflexive
- `impl PartialEq<Box<str>> for OsString` and reflexive
- `impl PartialEq<Rc<str>> for OsString` and reflexive
- `impl PartialEq<Arc<str>> for OsString` and reflexive
- `impl PartialOrd<String> for OsString` and reflexive
- `impl PartialOrd<Cow<'_, str>> for OsString` and reflexive
- `impl PartialOrd<Box<str>> for OsString` and reflexive
- `impl PartialOrd<Rc<str>> for OsString` and reflexive
- `impl PartialOrd<Arc<str>> for OsString` and reflexive

`OsStr`:

- `impl PartialEq<String> for OsStr` and reflexive
- `impl PartialEq<Cow<'_, str>> for OsStr` and reflexive
- `impl PartialEq<Box<str>> for OsStr` and reflexive
- `impl PartialEq<Rc<str>> for OsStr` and reflexive
- `impl PartialEq<Arc<str>> for OsStr` and reflexive
- `impl PartialOrd<String> for OsStr` and reflexive
- `impl PartialOrd<Cow<'_, str>> for OsStr` and reflexive
- `impl PartialOrd<Box<str>> for OsStr` and reflexive
- `impl PartialOrd<Rc<str>> for OsStr` and reflexive
- `impl PartialOrd<Arc<str>> for OsStr` and reflexive

`Component`:

- `impl PartialEq<OsStr> for Component` and reflexive
- `impl PartialEq<&'_ OsStr> for Component` and reflexive
- `impl PartialEq<OsString> for Component` and reflexive
- `impl PartialEq<Cow<'_, OsStr>> for Component` and reflexive
- `impl PartialEq<Box<OsStr>> for Component` and reflexive
- `impl PartialEq<Path> for Component` and reflexive
- `impl PartialEq<&'_ Path> for Component` and reflexive
- `impl PartialEq<PathBuf> for Component` and reflexive
- `impl PartialEq<Cow<'_, Path>> for Component` and reflexive
- `impl PartialEq<Box<Path>> for Component` and reflexive
- `impl PartialEq<str> for Component` and reflexive
- `impl PartialEq<String> for Component` and reflexive
- `impl PartialEq<Cow<'_, str>> for Component` and reflexive
- `impl PartialEq<Box<str>> for Component` and reflexive
- `impl PartialEq<Rc<str>> for Component` and reflexive
- `impl PartialEq<Arc<str>> for Component` and reflexive
- `impl PartialEq<str> for Component` and reflexive
- `impl PartialEq<String> for Component` and reflexive
- `impl PartialEq<Cow<'_, str>> for Component` and reflexive
- `impl PartialEq<Box<str>> for Component` and reflexive
- `impl PartialEq<Rc<str>> for Component` and reflexive
- `impl PartialEq<Arc<str>> for Component` and reflexive
- `impl PartialOrd<OsStr> for Component` and reflexive
- `impl PartialOrd<&'_ OsStr> for Component` and reflexive
- `impl PartialOrd<OsString> for Component` and reflexive
- `impl PartialOrd<Cow<'_, OsStr>> for Component` and reflexive
- `impl PartialOrd<Box<OsStr>> for Component` and reflexive
- `impl PartialOrd<Path> for Component` and reflexive
- `impl PartialOrd<&'_ Path> for Component` and reflexive
- `impl PartialOrd<PathBuf> for Component` and reflexive
- `impl PartialOrd<Cow<'_, Path>> for Component` and reflexive
- `impl PartialOrd<Box<Path>> for Component` and reflexive
- `impl PartialOrd<str> for Component` and reflexive
- `impl PartialOrd<String> for Component` and reflexive
- `impl PartialOrd<Cow<'_, str>> for Component` and reflexive
- `impl PartialOrd<Box<str>> for Component` and reflexive
- `impl PartialOrd<Rc<str>> for Component` and reflexive
- `impl PartialOrd<Arc<str>> for Component` and reflexive

`PathBuf`:

- `impl PartialEq<str> for PathBuf` and reflexive
- `impl PartialEq<&'_ str> for PathBuf` and reflexive
- `impl PartialEq<String> for PathBuf` and reflexive
- `impl PartialEq<Cow<'_, str>> for PathBuf` and reflexive
- `impl PartialEq<Box<str>> for PathBuf` and reflexive
- `impl PartialEq<Rc<str>> for PathBuf` and reflexive
- `impl PartialEq<Arc<str>> for PathBuf` and reflexive
- `impl PartialOrd<str> for PathBuf` and reflexive
- `impl PartialOrd<&'_ str> for PathBuf` and reflexive
- `impl PartialOrd<String> for PathBuf` and reflexive
- `impl PartialOrd<Cow<'_, str>> for PathBuf` and reflexive
- `impl PartialOrd<Box<str>> for PathBuf` and reflexive
- `impl PartialOrd<Rc<str>> for PathBuf` and reflexive
- `impl PartialOrd<Arc<str>> for PathBuf` and reflexive

`Path`:

- `impl PartialEq<str> for Path` and reflexive
- `impl PartialEq<&'_ str> for Path` and reflexive
- `impl PartialEq<String> for Path` and reflexive
- `impl PartialEq<Cow<'_, str>> for Path` and reflexive
- `impl PartialEq<Box<str>> for Path` and reflexive
- `impl PartialEq<Rc<str>> for Path` and reflexive
- `impl PartialEq<Arc<str>> for Path` and reflexive
- `impl PartialOrd<str> for Path` and reflexive
- `impl PartialOrd<&'_ str> for Path` and reflexive
- `impl PartialOrd<String> for Path` and reflexive
- `impl PartialOrd<Cow<'_, str>> for Path` and reflexive
- `impl PartialOrd<Box<str>> for Path` and reflexive
- `impl PartialOrd<Rc<str>> for Path` and reflexive
- `impl PartialOrd<Arc<str>> for Path` and reflexive

Fixes #71700.
@lopopolo
Copy link
Contributor Author

lopopolo commented Sep 1, 2020

Added more PartialOrd impls.

@lopopolo
Copy link
Contributor Author

lopopolo commented Sep 1, 2020

It looks like this PR changes type inference. I'm not sure what to do next.

error[E0277]: can't compare `str` with `std::string::String`
   --> compiler/rustc_macros/src/symbols.rs:101:20
    |
101 |             if str < prev_str {
    |                    ^ no implementation for `str < std::string::String` and `str > std::string::String`
    |
    = help: the trait `std::cmp::PartialOrd<std::string::String>` is not implemented for `str`
    = note: required because of the requirements on the impl of `std::cmp::PartialOrd<&std::string::String>` for `&str`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
error: could not compile `rustc_macros`.

@LeSeulArtichaut LeSeulArtichaut added needs-rfc This change is large or controversial enough that it should have an RFC accepted before doing it. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 5, 2020
@LeSeulArtichaut
Copy link
Contributor

I believe this will require a libs team FCP?
r? @sfackler

@sfackler
Copy link
Member

Those type inference errors are the problems that adding more impls like these cause. If it's breaking something in the compiler, there's a good chance it's going to break quite a bit of the ecosystem as well.

@Dylan-DPC-zz
Copy link

I don't think this can be accepted as is, because it can cause breakage in the ecosystem. I'm going to close this for now to prevent it from rotting further, if you have a better way of achieving this, you are free to submit a new PR. Thanks for contributing :)

@Dylan-DPC-zz Dylan-DPC-zz added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. needs-rfc This change is large or controversial enough that it should have an RFC accepted before doing it. labels Sep 30, 2020
@lopopolo lopopolo deleted the partialeq-utf-8-string-to-ffi-string branch May 15, 2021 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing PartialEq<&str> impls on Path and PathBuf
5 participants