Change E0369 to give note informations for foreign items.

[surechen]

Change E0369 to give note informations for foreign items.
Make it easy for developers to understand why the binop cannot be applied.

fixes #125631

[rustbot]

r? @fmease

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

[surechen]

I searched and finded it seems that foreign item define cannot be displayed correctly in rustc test's .stderr files, but I use the compiled rustc for a new project and can display all the information correctly.

For:

use std::io::{Error, ErrorKind};
use std::thread;
struct T1;
struct T2;

fn main() {
    (Error::new(ErrorKind::Other, "4"), thread::current(), T1, T2)
        == (Error::new(ErrorKind::Other, "4"), thread::current(), T1, T2);
    //~^ERROR binary operation `==` cannot be applied to type
}

error[E0369]: binary operation `==` cannot be applied to type `(std::io::Error, Thread, T1, T2)`
    --> ./src/main.rs:221:9
     |
220  |     (Error::new(ErrorKind::Other, "4"), thread::current(), T1, T2)
     |     -------------------------------------------------------------- (std::io::Error, Thread, T1, T2)
221  |         == (Error::new(ErrorKind::Other, "4"), thread::current(), T1, T2);
     |         ^^ -------------------------------------------------------------- (std::io::Error, Thread, T1, T2)
     |
note: the following types would have to `impl` their required traits for this operation to be valid
    --> ./src/main.rs:211:1
     |
211  | struct T1;
     | ^^^^^^^^^ must implement `PartialEq`
212  | struct T2;
     | ^^^^^^^^^ must implement `PartialEq`
note: the foreign item types don't implement required traits for this operation to be valid
    --> D:\source\rust\rust\library\std\src\io\error.rs:67:1
     |
67   | pub struct Error {
     | ^^^^^^^^^^^^^^^^ not implement `PartialEq`
     |
    ::: D:\source\rust\rust\library\std\src\thread\mod.rs:1313:1
     |
1313 | pub struct Thread {
     | ^^^^^^^^^^^^^^^^^ not implement `PartialEq`
help: consider annotating `T1` with `#[derive(PartialEq)]`
     |
211  + #[derive(PartialEq)]
212  | struct T1;
     |
help: consider annotating `T2` with `#[derive(PartialEq)]`
     |
212  + #[derive(PartialEq)]
213  | struct T2;
     |

error: aborting due to 1 previous error

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

[compiler-errors]

I think the message is really redundant. What is this PR trying to achieve?

The correct solution to #125631 IMO is that we should detect when we have diagnostic::on_unimplemented/rustc_on_unimplemented overwriting the default trait unimplemented message, and add the old primary message as a note if so.

In my opinion, there's nothing special about the Adt types, and this has nothing to do with foreign types or spans either?

[compiler-errors]

This, and most of the other messages that this PR adds, feels really redundant.

[compiler-errors]

@rustbot author

[surechen]

I think the message is really redundant. What is this PR trying to achieve?

The correct solution to #125631 IMO is that we should detect when we have diagnostic::on_unimplemented/rustc_on_unimplemented overwriting the default trait unimplemented message, and add the old primary message as a note if so.

In my opinion, there's nothing special about the Adt types, and this has nothing to do with foreign types or spans either?

Thank you very much.

As the example in #125631, for foreign items, E0369 will not show details information about which traits not implemeted causes the binop operation to be unavailable. But for local items, E0369 will tell the user this information.

My initial thought was that for foreign items, we cann't require them to implement these traits to satisfy the binop operation, so I separated the local and foreign items, and just use a note to explain the reasons for the foreign items.

For:
“In my opinion, there's nothing special about the Adt types, and this has nothing to do with foreign types or spans either?”
I'm not sure because it's the original code. I guess the Adt types restriction here is because for local items, developers are later advised to impl these missing traits.

For：
"we should detect when we have diagnostic::on_unimplemented/rustc_on_unimplemented overwriting the default trait unimplemented message, and add the old primary message as a note if so."

I don’t understand the mechanism you’re talking about yet. Could you give me more details?

@compiler-errors

[compiler-errors]

What does this have to do with foreign (non-local) types? I don't see anything in that issue that discusses that this is an limitation of non-local types.

[compiler-errors]

Oh --

although, if the member type that does not implement the trait is defined in the current crate, rustc will note that type, and say it might be missing a PartialEq impl. it's only with foreign types where no additional output is shown.

I don't actually think this is true? If it is, we should find the code that is filtering local types, rather than just adding a new note. Otherwise, this needs more investigation. I don't think we need to add a new note here.

[surechen]

What does this have to do with foreign (non-local) types? I don't see anything in that issue that discusses that this is an limitation of non-local types.

In his Desired output, he want add this information:
note: std::io::Error does not implement PartialEq

[compiler-errors]

Can you come up an example where this note gets produced for a local type?

I would then recommend using that example to find out how to fix that to make it produce the right note, rather than just adding a new one.

[surechen]

Can you come up an example where this note gets produced for a local type?

I would then recommend using that example to find out how to fix that to make it produce the right note, rather than just adding a new one.

OK. Thank you.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d1ebd57607cfd0b9a0cef89cfa1c8d36

[surechen]

This PR only involves this part about note, Not including the following message about help:

note: the following types would have to `impl` their required traits for this operation to be valid
 --> src/main.rs:1:1
  |
1 | struct T1;
  | ^^^^^^^^^ must implement `PartialEq`
2 | struct T2;
  | ^^^^^^^^^ must implement `PartialEq`

[compiler-errors]

Actually, nevermind. I think I understand the approach of this PR.

[compiler-errors]

@bors r+

[bors]

📌 Commit 2a6a423 has been approved by compiler-errors

It is now in the queue for this repository.

[surechen]

Actually, nevermind. I think I understand the approach of this PR.

Thank you.