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

Add more impls of PartialEq and PartialOrd for strings #135536

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions library/alloc/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

#![stable(feature = "rust1", since = "1.0.0")]

use core::cmp::Ordering;
use core::error::Error;
use core::iter::FusedIterator;
#[cfg(not(no_global_oom_handling))]
Expand Down Expand Up @@ -2530,12 +2531,51 @@ macro_rules! impl_eq {

impl_eq! { String, str }
impl_eq! { String, &'a str }
impl_eq! { String, &String }
impl_eq! { &String, str }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two impls end up with incorrect stability attributes, which means Clippy msrv autodetection can't be added later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fixable, but is that worth complicating the macro for? There are many other examples of macro-generated instances with the same discrepancy.

#[cfg(not(no_global_oom_handling))]
impl_eq! { Cow<'a, str>, str }
#[cfg(not(no_global_oom_handling))]
impl_eq! { Cow<'a, str>, &'b str }
#[cfg(not(no_global_oom_handling))]
impl_eq! { Cow<'a, str>, String }
#[cfg(not(no_global_oom_handling))]
impl_eq! { Cow<'a, str>, &String }

macro_rules! impl_ord {
($lhs:ty, $rhs: ty) => {
#[stable(feature = "string_partialord_impls", since = "CURRENT_RUSTC_VERSION")]
#[allow(unused_lifetimes)]
impl<'a, 'b> PartialOrd<$rhs> for $lhs {
#[inline]
fn partial_cmp(&self, other: &$rhs) -> Option<Ordering> {
PartialOrd::partial_cmp(&self[..], &other[..])
}
}

#[stable(feature = "string_partialord_impls", since = "CURRENT_RUSTC_VERSION")]
#[allow(unused_lifetimes)]
impl<'a, 'b> PartialOrd<$lhs> for $rhs {
#[inline]
fn partial_cmp(&self, other: &$lhs) -> Option<Ordering> {
PartialOrd::partial_cmp(&self[..], &other[..])
}
}
};
}

impl_ord! { String, str }
impl_ord! { String, &'a str }
impl_ord! { &String, str }
impl_ord! { String, &String }
#[cfg(not(no_global_oom_handling))]
impl_ord! { Cow<'a, str>, str }
#[cfg(not(no_global_oom_handling))]
impl_ord! { Cow<'a, str>, &'b str }
#[cfg(not(no_global_oom_handling))]
impl_ord! { Cow<'a, str>, String }
#[cfg(not(no_global_oom_handling))]
impl_ord! { Cow<'a, str>, &String }

#[stable(feature = "rust1", since = "1.0.0")]
impl Default for String {
Expand Down
32 changes: 32 additions & 0 deletions library/core/src/str/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,22 @@ impl PartialEq for str {
}
}

#[stable(feature = "more_str_partialeq_impls", since = "CURRENT_RUSTC_VERSION")]
impl PartialEq<&str> for str {
#[inline]
fn eq(&self, other: &&str) -> bool {
self.as_bytes() == other.as_bytes()
}
}

#[stable(feature = "more_str_partialeq_impls", since = "CURRENT_RUSTC_VERSION")]
impl PartialEq<str> for &str {
#[inline]
fn eq(&self, other: &str) -> bool {
self.as_bytes() == other.as_bytes()
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl Eq for str {}

Expand All @@ -48,6 +64,22 @@ impl PartialOrd for str {
}
}

#[stable(feature = "more_str_partialord_impls", since = "CURRENT_RUSTC_VERSION")]
impl PartialOrd<&str> for str {
#[inline]
fn partial_cmp(&self, other: &&str) -> Option<Ordering> {
Some(self.cmp(*other))
}
}

#[stable(feature = "more_str_partialord_impls", since = "CURRENT_RUSTC_VERSION")]
impl PartialOrd<str> for &str {
#[inline]
fn partial_cmp(&self, other: &str) -> Option<Ordering> {
Some(self.cmp(&other))
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<I> ops::Index<I> for str
where
Expand Down
3 changes: 2 additions & 1 deletion src/bootstrap/src/core/build_steps/clippy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ pub fn get_clippy_rules_in_order(all_args: &[String], config: &LintConfig) -> Ve
let rule = format!("{prefix}{v}");
// Arguments added by bootstrap in LintConfig won't show up in the all_args list, so
// put them at the end of the command line.
let position = all_args.iter().position(|t| t == &rule || t == v).unwrap_or(usize::MAX);
let position =
all_args.iter().position(|t| t == rule.as_str() || t == v).unwrap_or(usize::MAX);
result.push((position, rule));
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/tools/clippy/clippy_lints/src/operators/cmp_owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,12 @@ fn check_op(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left: bool)
let arg_snip = snippet(cx, arg_span, "..");
let expr_snip;
let eq_impl;
if with_deref.is_implemented() {
expr_snip = format!("*{arg_snip}");
eq_impl = with_deref;
} else {
if without_deref.is_implemented() {
expr_snip = arg_snip.to_string();
eq_impl = without_deref;
} else {
expr_snip = format!("*{arg_snip}");
eq_impl = with_deref;
}

let span;
Expand Down
8 changes: 4 additions & 4 deletions src/tools/clippy/tests/ui/iter_overeager_cloned.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ fn main() {

let _: Option<String> = vec.iter().chain(vec.iter()).next().cloned();

let _: usize = vec.iter().filter(|x| x == &"2").count();
let _: usize = vec.iter().filter(|x| x == "2").count();

let _: Vec<_> = vec.iter().take(2).cloned().collect();

let _: Vec<_> = vec.iter().skip(2).cloned().collect();

let _ = vec.iter().filter(|x| x == &"2").nth(2).cloned();
let _ = vec.iter().filter(|x| x == "2").nth(2).cloned();

let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))]
.iter()
Expand All @@ -46,7 +46,7 @@ fn main() {
iter: impl Iterator<Item = &'a (&'a u32, String)> + 'a,
target: String,
) -> impl Iterator<Item = (&'a u32, String)> + 'a {
iter.filter(move |&(&a, b)| a == 1 && b == &target).cloned()
iter.filter(move |&(&a, b)| a == 1 && b == target).cloned()
}

{
Expand All @@ -57,7 +57,7 @@ fn main() {
}

fn bar<'a>(iter: impl Iterator<Item = &'a S<'a>> + 'a, target: String) -> impl Iterator<Item = S<'a>> + 'a {
iter.filter(move |&S { a, b }| **a == 1 && b == &target).cloned()
iter.filter(move |&S { a, b }| **a == 1 && b == target).cloned()
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/tests/ui/iter_overeager_cloned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn main() {
iter: impl Iterator<Item = &'a (&'a u32, String)> + 'a,
target: String,
) -> impl Iterator<Item = (&'a u32, String)> + 'a {
iter.cloned().filter(move |(&a, b)| a == 1 && b == &target)
iter.cloned().filter(move |(&a, b)| a == 1 && b == target)
}

{
Expand All @@ -58,7 +58,7 @@ fn main() {
}

fn bar<'a>(iter: impl Iterator<Item = &'a S<'a>> + 'a, target: String) -> impl Iterator<Item = S<'a>> + 'a {
iter.cloned().filter(move |S { a, b }| **a == 1 && b == &target)
iter.cloned().filter(move |S { a, b }| **a == 1 && b == target)
}
}

Expand Down
33 changes: 26 additions & 7 deletions src/tools/clippy/tests/ui/iter_overeager_cloned.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ LL | let _: usize = vec.iter().filter(|x| x == &"2").cloned().count();
= note: `-D clippy::redundant-clone` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::redundant_clone)]`

error: taken reference of right operand
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lint shouldn't trigger in the test file. Please fix the two new occurrences by removing the ref.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, will fix.

--> tests/ui/iter_overeager_cloned.rs:16:42
|
LL | let _: usize = vec.iter().filter(|x| x == &"2").cloned().count();
| ^^^^^----
| |
| help: use the right value directly: `"2"`
|
= note: `-D clippy::op-ref` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::op_ref)]`

error: unnecessarily eager cloning of iterator items
--> tests/ui/iter_overeager_cloned.rs:18:21
|
Expand All @@ -52,6 +63,14 @@ LL | let _ = vec.iter().filter(|x| x == &"2").cloned().nth(2);
| |
| help: try: `.nth(2).cloned()`

error: taken reference of right operand
--> tests/ui/iter_overeager_cloned.rs:22:35
|
LL | let _ = vec.iter().filter(|x| x == &"2").cloned().nth(2);
| ^^^^^----
| |
| help: use the right value directly: `"2"`

error: unnecessarily eager cloning of iterator items
--> tests/ui/iter_overeager_cloned.rs:24:13
|
Expand Down Expand Up @@ -119,18 +138,18 @@ LL | let _ = vec.iter().cloned().find(f);
error: unnecessarily eager cloning of iterator items
--> tests/ui/iter_overeager_cloned.rs:50:9
|
LL | iter.cloned().filter(move |(&a, b)| a == 1 && b == &target)
| ^^^^-------------------------------------------------------
LL | iter.cloned().filter(move |(&a, b)| a == 1 && b == target)
| ^^^^------------------------------------------------------
| |
| help: try: `.filter(move |&(&a, b)| a == 1 && b == &target).cloned()`
| help: try: `.filter(move |&(&a, b)| a == 1 && b == target).cloned()`

error: unnecessarily eager cloning of iterator items
--> tests/ui/iter_overeager_cloned.rs:61:13
|
LL | iter.cloned().filter(move |S { a, b }| **a == 1 && b == &target)
| ^^^^------------------------------------------------------------
LL | iter.cloned().filter(move |S { a, b }| **a == 1 && b == target)
| ^^^^-----------------------------------------------------------
| |
| help: try: `.filter(move |&S { a, b }| **a == 1 && b == &target).cloned()`
| help: try: `.filter(move |&S { a, b }| **a == 1 && b == target).cloned()`

error: unneeded cloning of iterator items
--> tests/ui/iter_overeager_cloned.rs:65:13
Expand Down Expand Up @@ -164,5 +183,5 @@ LL | let _ = vec.iter().cloned().any(|x| x.len() == 1);
| |
| help: try: `.any(|x| x.len() == 1)`

error: aborting due to 19 previous errors
error: aborting due to 21 previous errors

2 changes: 1 addition & 1 deletion src/tools/clippy/tests/ui/op_ref.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main() {
let a = "a".to_string();
let b = "a";

if b < &a {
if b < a {
println!("OK");
}

Expand Down
10 changes: 9 additions & 1 deletion src/tools/clippy/tests/ui/op_ref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ help: use the values directly
LL | let foo = 5 - 6;
| ~ ~

error: taken reference of right operand
--> tests/ui/op_ref.rs:21:8
|
LL | if b < &a {
| ^^^^--
| |
| help: use the right value directly: `a`

error: taken reference of right operand
--> tests/ui/op_ref.rs:58:13
|
Expand All @@ -35,5 +43,5 @@ LL | let _ = two + &three;
| |
| help: use the right value directly: `three`

error: aborting due to 4 previous errors
error: aborting due to 5 previous errors

2 changes: 0 additions & 2 deletions tests/ui/binop/binary-op-suggest-deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ fn baz() {
let string_ref = &owned;
let partial = "foobar";
_ = string_ref == partial[..3];
//~^ERROR can't compare `&String` with `str` [E0277]
_ = partial[..3] == string_ref;
//~^ERROR can't compare `str` with `&String` [E0277]
}

fn qux() {
Expand Down
30 changes: 3 additions & 27 deletions tests/ui/binop/binary-op-suggest-deref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -271,32 +271,8 @@ note: an implementation of `PartialEq<&&{integer}>` might be missing for `Foo`
LL | struct Foo;
| ^^^^^^^^^^ must implement `PartialEq<&&{integer}>`

error[E0277]: can't compare `&String` with `str`
--> $DIR/binary-op-suggest-deref.rs:69:20
|
LL | _ = string_ref == partial[..3];
| ^^ no implementation for `&String == str`
|
= help: the trait `PartialEq<str>` is not implemented for `&String`
help: consider dereferencing here
|
LL | _ = *string_ref == partial[..3];
| +

error[E0277]: can't compare `str` with `&String`
--> $DIR/binary-op-suggest-deref.rs:71:22
|
LL | _ = partial[..3] == string_ref;
| ^^ no implementation for `str == &String`
|
= help: the trait `PartialEq<&String>` is not implemented for `str`
help: consider dereferencing here
|
LL | _ = partial[..3] == *string_ref;
| +

error[E0277]: no implementation for `i32 & str`
--> $DIR/binary-op-suggest-deref.rs:78:17
--> $DIR/binary-op-suggest-deref.rs:76:17
|
LL | let _ = FOO & (*"Sized".to_string().into_boxed_str());
| ^ no implementation for `i32 & str`
Expand All @@ -309,14 +285,14 @@ LL | let _ = FOO & (*"Sized".to_string().into_boxed_str());
`i32` implements `BitAnd`

error[E0277]: the size for values of type `str` cannot be known at compilation time
--> $DIR/binary-op-suggest-deref.rs:78:17
--> $DIR/binary-op-suggest-deref.rs:76:17
|
LL | let _ = FOO & (*"Sized".to_string().into_boxed_str());
| ^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `str`

error: aborting due to 24 previous errors
error: aborting due to 22 previous errors

Some errors have detailed explanations: E0277, E0308, E0369.
For more information about an error, try `rustc --explain E0277`.
7 changes: 5 additions & 2 deletions tests/ui/inference/issue-72616.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@ LL | if String::from("a") == "a".try_into().unwrap() {}
|
= note: cannot satisfy `String: PartialEq<_>`
= help: the following types implement trait `PartialEq<Rhs>`:
`&String` implements `PartialEq<Cow<'_, str>>`
`&String` implements `PartialEq<String>`
`&String` implements `PartialEq<str>`
`String` implements `PartialEq<&String>`
`String` implements `PartialEq<&str>`
`String` implements `PartialEq<ByteStr>`
`String` implements `PartialEq<ByteString>`
`String` implements `PartialEq<Cow<'_, str>>`
`String` implements `PartialEq<str>`
`String` implements `PartialEq`
and 2 others
help: try using a fully qualified path to specify the expected types
|
LL | if String::from("a") == <&str as TryInto<T>>::try_into("a").unwrap() {}
Expand Down
Loading