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

Standard library documentation for alternatives to [T]::contains suggests Borrow is needed when it is not #84877

Closed
jimblandy opened this issue May 3, 2021 · 2 comments · Fixed by #84878
Labels
C-bug Category: This is a bug.

Comments

@jimblandy
Copy link
Contributor

The documentation for [T]::contains where T: PartialEq<T> suggests that, if this method is not applicable because the type you need to compare with is not exactly T, you should use .iter().any(). This is good advice, but it goes on to suggest that somehow a Borrow implementation is a necessary part of the solution, when it is not.

The documentation should suggest the use of iter().any without mentioning Borrow.

Specifically, the documentation says:

If you do not have an &T, but just an &U such that T: Borrow (e.g. String: Borrow), you can use iter().any:

let v = [String::from("hello"), String::from("world")]; // slice of `String`
assert!(v.iter().any(|e| e == "hello")); // search with `&str`
assert!(!v.iter().any(|e| e == "hi"));

This code works because String has an explicit implementation of PartialEq<str> (note, no &), not because it implements Borrow<str>. As far as I know, Borrow is never invoked implicitly (it's not a lang item), and it doesn't help one use == in this way. For example:

#![allow(unused)]
use std::borrow::Borrow;

#[derive(PartialEq)]
struct S(i32);

#[derive(PartialEq)]
struct T(S);

impl Borrow<S> for T {
    fn borrow(&self) -> &S {
        &self.0
    }
}

fn main() {
    let v = [T(S(10)), T(S(20))];
    assert!(v.iter().any(|e| e == &S(10)));
    assert!(!v.iter().any(|e| e == &S(15)));
}

This fails:

error[E0277]: can't compare `T` with `S`
  --> src/main.rs:18:32
   |
18 |     assert!(v.iter().any(|e| e == &S(10)));
   |                                ^^ no implementation for `T == S`
   |
   = help: the trait `PartialEq<S>` is not implemented for `T`
   = note: required because of the requirements on the impl of `PartialEq<&S>` for `&T`

error[E0277]: can't compare `T` with `S`
  --> src/main.rs:19:33
   |
19 |     assert!(!v.iter().any(|e| e == &S(15)));
   |                                 ^^ no implementation for `T == S`
   |
   = help: the trait `PartialEq<S>` is not implemented for `T`
   = note: required because of the requirements on the impl of `PartialEq<&S>` for `&T`
@jimblandy jimblandy added the C-bug Category: This is a bug. label May 3, 2021
@jimblandy
Copy link
Contributor Author

CC: @RalfJung , since they're the author of the comment.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
…riplett

Clarify documentation for `[T]::contains`. Fixes rust-lang#84877.

Change the documentation to correctly characterize when the suggested alternative to `contains` applies, and correctly explain why it works.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue May 4, 2021
…riplett

Clarify documentation for `[T]::contains`. Fixes rust-lang#84877.

Change the documentation to correctly characterize when the suggested alternative to `contains` applies, and correctly explain why it works.
@RalfJung
Copy link
Member

RalfJung commented May 5, 2021

Introduced in #62656, also Cc #62367 which is still open. Looks like I simply misread this comment.

RalfJung added a commit to RalfJung/rust that referenced this issue May 5, 2021
…riplett

Clarify documentation for `[T]::contains`

Change the documentation to correctly characterize when the suggested alternative to `contains` applies, and correctly explain why it works.

Fixes rust-lang#84877
RalfJung added a commit to RalfJung/rust that referenced this issue May 5, 2021
…riplett

Clarify documentation for `[T]::contains`

Change the documentation to correctly characterize when the suggested alternative to `contains` applies, and correctly explain why it works.

Fixes rust-lang#84877
RalfJung added a commit to RalfJung/rust that referenced this issue May 5, 2021
…riplett

Clarify documentation for `[T]::contains`

Change the documentation to correctly characterize when the suggested alternative to `contains` applies, and correctly explain why it works.

Fixes rust-lang#84877
@bors bors closed this as completed in d53469c May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
2 participants