Avoid redundant note when a #[derive] is already suggested

[Dnreikronos]

Fixes #157118

The Debug #[rustc_on_unimplemented] note ("add #[derive(Debug)] to X or manually impl Debug for X") just repeats the consider annotating X with #[derive(..)] suggestion that suggest_derive already emits, so on these bound errors it's pure noise.

Now the note only gets dropped when that suggestion will actually show, which is whenever can_suggest_derive holds: a crate-local ADT, not a union, with every field already implementing the trait. Deleting the note from the attribute outright would be too blunt, since when can_suggest_derive is false there's no suggestion and the note is the only hint the user gets. A crate-local struct with a non-Debug field still ends up with just:

error[E0277]: `Outer` doesn't implement `Debug`
   = note: add `#[derive(Debug)]` to `Outer` or manually `impl Debug for Outer`

Same story when main_trait_predicate != leaf_trait_predicate, since the note comes from the main predicate and the suggestion from the leaf.

[rustbot]

r? @adwinwhite

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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 17 candidates

[estebank]

Happy to see the results of the change, they certainly were redundant.

View changes since this review

[estebank]

It makes more sense to not enter into the loop if every element will be skipped if a condition is true. The way it is written we're just wasting time iterating over every element in order to do nothing.

[estebank]

Just to confirm, what is the difference between the output in this PR and the output if we remove the following lines? Are there some cases where the note is useful? It'd be a good idea to include that context in the comment or at least in this PR.

rust/library/core/src/fmt/mod.rs

Lines 1039 to 1043 in 6368fd5

	on(
	all(crate_local, not(Self = "{union}")),
	note = "add `#[derive(Debug)]` to `{Self}` or manually `impl {This} for {Self}`"
	),
	on(all(crate_local, Self = "{union}"), note = "manually `impl {This} for {Self}`"),

[Dnreikronos]

Yeah, good call flagging this. Short version: deleting those lines would kill the note everywhere, but it's only redundant some of the time, so I skip it conditionally instead.

suggest_derive shows consider annotating X with #[derive(..)] only when can_suggest_derive holds: a crate-local ADT, not a union, and all fields already implement the trait. When that fires, the note and the suggestion are saying the same thing, so the note is just clutter (that's what the stderr diffs are trimming).

When can_suggest_derive is false there's no suggestion at all, and then the note is the only nudge the user gets. Easiest case to picture is a local struct with a field that isn't Debug:

struct Inner; // no Debug

struct Outer {
    inner: Inner,
}

fn main() {
    println!("{:?}", Outer { inner: Inner });
}

error[E0277]: `Outer` doesn't implement `Debug`
   = note: add `#[derive(Debug)]` to `Outer` or manually `impl Debug for Outer`

No consider annotating line here, since Outer's field isn't Debug, so dropping the lines outright would leave this one with nothing actionable. Same story when main_trait_predicate != leaf_trait_predicate: the note's built from the main predicate and the suggestion from the leaf, so they don't overlap.

Happy to pull this into the PR description too. Thanks for pushing on it! :)

[estebank]

@bors r+

[rust-bors]

📌 Commit 96a2ded has been approved by estebank

It is now in the queue for this repository.