Add notes for missing PartialEq and PartialOrd, closes #28837

[fhahn]

this PR adds notes for missing PartialEq and PartialOrd. I've added a test case but it seems like NOTE is ignored by the test runner.
#28837

[rust-highfive]

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

[alexcrichton]

This file I believe will need a license to pass make tidy

[alexcrichton]

Perhaps this could also handle all other binary operations? I think we've basically got a trait for all of them, so it could print a message out for each.

[apasel422]

I guess it's not possible to use the #[rustc_on_unimplemented] attribute for this?

[fhahn]

I've pushed a new commit that adds notes for all relevant std::ops::* traits. At the moment the note only states that a required trait is not implemented by the type A or one of its type arguments, which is not really precise and not correct as I just noticed.

It should be more like "it is not implemented by type A or any other type that's used in its eq method", but I am not sure how express this.

However, the ideal note would say exactly which type does not have an PartialEq implementation.

Consider

#[derive(PartialEq)]
struct Foo<T> {
    f: T,
}

struct Bar;

fn main() {
    let x = Foo { f: Bar };
    x == x;
}

Now the note would look like an implementation of "std::cmp::PartialEq" might be missing for "Foo" or one of its type arguments. The better note would be an implementation of "std::cmp::PartialEq" might be missing for "Bar". The problem here is with the comparison self.f == other.f in the derived eq function, but it gets raised when checking whether Foo implements PartialEq.

Note that this does not happen when PartialEq is implemented manually for Foo.

Do you have any pointers where to look in order to improve the note in the derive case?

[alexcrichton]

I'm not 100% sure how the deriving case could be improved, but dropping the "or one of its type arguments" seems fine to me, the qualification of "might" is probably good enough for "this message isn't always correct"

[fhahn]

Ok, I dropped the "or one of its type arguments" from the message. I still think handling the deriving case better would be good, as the new note is still somewhat confusing in the case @AndiDog mentioned in the issue, especially for beginners I think.

But maybe improving it something to do in another pull request?

[alexcrichton]

This may be more idiomatically done via Some("...") with a None at the end followed by a if let Some(missing_trait) = display_note at the printing location.

[fhahn]

Thanks, I've updated the commit.

[alexcrichton]

@bors: r+ da169028e3093ea6e74d50be3750efca9f3465fc

Thanks!

[bors]

⌛ Testing commit da16902 with merge a72423a...

[bors]

💔 Test failed - auto-mac-64-opt

[fhahn]

Unfortunately there was a trailing whitespace (sorry for that). I've updated the commit.

[alexcrichton]

@bors: r+ b21ae1a

[fhahn]

Is it possible bors missed this PR?

[bluss]

@bors r=alexcrichton

[bors]

📌 Commit b21ae1a has been approved by alexcrichton

[bluss]

@bors retry

[bluss]

Ok, now it's back on the board

[fhahn]

Wow that was fast. Thanks you very much for your swift response :)

[bors]

⌛ Testing commit b21ae1a with merge c7a58b5...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt