Skip to content

Conversation

kornelski
Copy link
Contributor

@kornelski kornelski commented Sep 1, 2024

Box<str> lacks PartialEq implementations for comparisons with other string types. It can't even be compared to a string literal:

error[E0277]: can't compare `&str` with `Box<str>`
 --> src/lib.rs:6:21
  |
6 |     assert!("boxed" == boxed);
  |                     ^^ no implementation for `&str == Box<str>`
  |
  = help: the trait `PartialEq<Box<str>>` is not implemented for `&str`
help: consider dereferencing both sides of the expression
  |
6 |     assert!(*"boxed" == *boxed);

This PR adds the same PartialEq implementations to Box<str> as Cow<str> has, making it comparable with str and String in many more cases.

This is a risky change, as it may affect type inference. AFAIK PartialEq implementations can't be marked as unstable, so this is insta-stable.

Forum discussion.

This makes Box<str> comparable with str and String in more cases, on par with Cow<str>
@rustbot
Copy link
Collaborator

rustbot commented Sep 1, 2024

r? @Amanieu

rustbot has assigned @Amanieu.
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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 1, 2024
@jieyouxu jieyouxu added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Sep 1, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Sep 1, 2024

This will definitely need a crater run

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
------
 > importing cache manifest from ghcr.io/rust-lang/rust-ci-cache:20d3b4d4a2629cbf7865cdbf92fe47512a7c96658c24253a045ff38e8075cd7fb37ca6fcadfa6e6d093333943ad24f6fc4f163ec5b74fd940de9d5bb03eb4d3b:
------
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---

---- [ui] tests/ui/inference/issue-72616.rs stdout ----
diff of stderr:

9    = note: multiple `impl`s satisfying `String: PartialEq<_>` found in the `alloc` crate:
10            - impl PartialEq for String;
11            - impl<'a, 'b> PartialEq<&'a str> for String;
+            - impl<'a, 'b> PartialEq<Box<str>> for String;
12            - impl<'a, 'b> PartialEq<Cow<'a, str>> for String;
13            - impl<'a, 'b> PartialEq<str> for String;


The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/inference/issue-72616/issue-72616.stderr
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/inference/issue-72616/issue-72616.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args inference/issue-72616.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/inference/issue-72616.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/inference/issue-72616" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/inference/issue-72616/auxiliary"
--- stderr -------------------------------
error[E0283]: type annotations needed
##[error]  --> /checkout/tests/ui/inference/issue-72616.rs:22:37
   |
   |
LL |         if String::from("a") == "a".try_into().unwrap() {}
   |                              |
   |                              type must be known at this point
   |
   |
   = note: multiple `impl`s satisfying `String: PartialEq<_>` found in the `alloc` crate:
           - impl PartialEq for String;
           - impl<'a, 'b> PartialEq<&'a str> for String;
           - impl<'a, 'b> PartialEq<Box<str>> for String;
           - impl<'a, 'b> PartialEq<Cow<'a, str>> for String;
           - impl<'a, 'b> PartialEq<str> for String;
   |
   |
LL |         if String::from("a") == <&str as TryInto<T>>::try_into("a").unwrap() {}
   |                                 +++++++++++++++++++++++++++++++   ~
error[E0283]: type annotations needed
##[error]  --> /checkout/tests/ui/inference/issue-72616.rs:22:37
   |
   |
LL |         if String::from("a") == "a".try_into().unwrap() {}
   |
   |
   = note: multiple `impl`s satisfying `_: TryFrom<&str>` found in the following crates: `core`, `std`:
           - impl TryFrom<&str> for std::sys_common::net::LookupHost;
           - impl<T, U> TryFrom<U> for T
             where U: Into<T>;
   = note: required for `&str` to implement `TryInto<_>`
   |
   |
LL |         if String::from("a") == <&str as TryInto<T>>::try_into("a").unwrap() {}
   |                                 +++++++++++++++++++++++++++++++   ~
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0283`.
------------------------------------------

@kornelski
Copy link
Contributor Author

kornelski commented Sep 1, 2024

if String::from("a") == "a".try_into().unwrap()
note: multiple impls satisfying String: PartialEq<_> found in the alloc crate:

I think the useless_conversions lint needs to be adopted first to make crates less sensitive to type inference: #129249

@kornelski kornelski closed this Sep 1, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 22, 2024
…,dtolnay

Add str.as_str() for easy Deref to string slices

Working with `Box<str>` is cumbersome, because in places like `iter.filter()` it can end up being `&Box<str>` or even `&&Box<str>`, and such type doesn't always get auto-dereferenced as expected.

Dereferencing such box to `&str` requires ugly syntax like `&**boxed_str` or `&***boxed_str`, with the exact amount of `*`s.

`Box<str>` is [not easily comparable with other string types](rust-lang#129852) via `PartialEq`. `Box<str>` won't work for lookups in types like `HashSet<String>`, because `Borrow<String>` won't take types like `&Box<str>`. OTOH `set.contains(s.as_str())` works nicely regardless of levels of indirection.

`String` has a simple solution for this: the `as_str()` method, and `Box<str>` should too.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 23, 2024
…,dtolnay

Add str.as_str() for easy Deref to string slices

Working with `Box<str>` is cumbersome, because in places like `iter.filter()` it can end up being `&Box<str>` or even `&&Box<str>`, and such type doesn't always get auto-dereferenced as expected.

Dereferencing such box to `&str` requires ugly syntax like `&**boxed_str` or `&***boxed_str`, with the exact amount of `*`s.

`Box<str>` is [not easily comparable with other string types](rust-lang#129852) via `PartialEq`. `Box<str>` won't work for lookups in types like `HashSet<String>`, because `Borrow<String>` won't take types like `&Box<str>`. OTOH `set.contains(s.as_str())` works nicely regardless of levels of indirection.

`String` has a simple solution for this: the `as_str()` method, and `Box<str>` should too.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 23, 2024
…,dtolnay

Add str.as_str() for easy Deref to string slices

Working with `Box<str>` is cumbersome, because in places like `iter.filter()` it can end up being `&Box<str>` or even `&&Box<str>`, and such type doesn't always get auto-dereferenced as expected.

Dereferencing such box to `&str` requires ugly syntax like `&**boxed_str` or `&***boxed_str`, with the exact amount of `*`s.

`Box<str>` is [not easily comparable with other string types](rust-lang#129852) via `PartialEq`. `Box<str>` won't work for lookups in types like `HashSet<String>`, because `Borrow<String>` won't take types like `&Box<str>`. OTOH `set.contains(s.as_str())` works nicely regardless of levels of indirection.

`String` has a simple solution for this: the `as_str()` method, and `Box<str>` should too.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 23, 2024
Rollup merge of rust-lang#129550 - kornelski:boxasstr, r=joshtriplett,dtolnay

Add str.as_str() for easy Deref to string slices

Working with `Box<str>` is cumbersome, because in places like `iter.filter()` it can end up being `&Box<str>` or even `&&Box<str>`, and such type doesn't always get auto-dereferenced as expected.

Dereferencing such box to `&str` requires ugly syntax like `&**boxed_str` or `&***boxed_str`, with the exact amount of `*`s.

`Box<str>` is [not easily comparable with other string types](rust-lang#129852) via `PartialEq`. `Box<str>` won't work for lookups in types like `HashSet<String>`, because `Borrow<String>` won't take types like `&Box<str>`. OTOH `set.contains(s.as_str())` works nicely regardless of levels of indirection.

`String` has a simple solution for this: the `as_str()` method, and `Box<str>` should too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. 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.

5 participants