From 4225a1e186afe39c1ae81c5065d24287d6d49fb9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 30 Jan 2024 16:57:29 +1100 Subject: [PATCH] Don't hash lints differently to non-lints. `Diagnostic::keys`, which is used for hashing and equating diagnostics, has a surprising behaviour: it ignores children, but only for lints. This was added in #88493 to fix some duplicated diagnostics, but it doesn't seem necessary any more. This commit removes the special case and only four tests have changed output, with additional errors. And those additional errors aren't exact duplicates, they're just similar. For example, in src/tools/clippy/tests/ui/same_name_method.rs we currently have this error: ``` error: method's name is the same as an existing method in a trait --> $DIR/same_name_method.rs:75:13 | LL | fn foo() {} | ^^^^^^^^^^^ | note: existing `foo` defined here --> $DIR/same_name_method.rs:79:9 | LL | impl T1 for S {} | ^^^^^^^^^^^^^^^^ ``` and with this change we also get this error: ``` error: method's name is the same as an existing method in a trait --> $DIR/same_name_method.rs:75:13 | LL | fn foo() {} | ^^^^^^^^^^^ | note: existing `foo` defined here --> $DIR/same_name_method.rs:81:9 | LL | impl T2 for S {} | ^^^^^^^^^^^^^^^^ ``` I think printing this second argument is reasonable, possibly even preferable to hiding it. And the other cases are similar. --- compiler/rustc_errors/src/diagnostic.rs | 16 ++++++++-------- src/tools/clippy/tests/ui/same_name_method.rs | 1 + .../clippy/tests/ui/same_name_method.stderr | 16 ++++++++++++++-- tests/rustdoc-ui/unescaped_backticks.stderr | 3 --- .../stable-metric/ctfe-simple-loop.warn.stderr | 1 - tests/ui/imports/ambiguous-9.stderr | 1 - 6 files changed, 23 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index bf1ab37a1cf48..b87eef07fd53a 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -109,8 +109,6 @@ pub struct Diagnostic { /// `span` if there is one. Otherwise, it is `DUMMY_SP`. pub sort_span: Span, - /// If diagnostic is from Lint, custom hash function ignores children. - /// Otherwise hash is based on the all the fields. pub is_lint: Option, /// With `-Ztrack_diagnostics` enabled, @@ -980,22 +978,24 @@ impl Diagnostic { ) -> ( &Level, &[(DiagnosticMessage, Style)], - Vec<(&Cow<'static, str>, &DiagnosticArgValue)>, &Option, - &Option, &MultiSpan, + &[SubDiagnostic], &Result, SuggestionsDisabled>, - Option<&[SubDiagnostic]>, + Vec<(&DiagnosticArgName, &DiagnosticArgValue)>, + &Option, ) { ( &self.level, &self.messages, - self.args().collect(), &self.code, - &self.is_lint, &self.span, + &self.children, &self.suggestions, - (if self.is_lint.is_some() { None } else { Some(&self.children) }), + self.args().collect(), + // omit self.sort_span + &self.is_lint, + // omit self.emitted_at ) } } diff --git a/src/tools/clippy/tests/ui/same_name_method.rs b/src/tools/clippy/tests/ui/same_name_method.rs index 1c166a19b0ad7..26b1a299ba1cf 100644 --- a/src/tools/clippy/tests/ui/same_name_method.rs +++ b/src/tools/clippy/tests/ui/same_name_method.rs @@ -74,6 +74,7 @@ mod should_lint { impl S { fn foo() {} //~^ ERROR: method's name is the same as an existing method in a trait + //~| ERROR: method's name is the same as an existing method in a trait } impl T1 for S {} diff --git a/src/tools/clippy/tests/ui/same_name_method.stderr b/src/tools/clippy/tests/ui/same_name_method.stderr index 3c5c4a53ad1f6..82f5ef6a9e827 100644 --- a/src/tools/clippy/tests/ui/same_name_method.stderr +++ b/src/tools/clippy/tests/ui/same_name_method.stderr @@ -56,10 +56,22 @@ LL | fn foo() {} | ^^^^^^^^^^^ | note: existing `foo` defined here - --> $DIR/same_name_method.rs:79:9 + --> $DIR/same_name_method.rs:80:9 | LL | impl T1 for S {} | ^^^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: method's name is the same as an existing method in a trait + --> $DIR/same_name_method.rs:75:13 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + | +note: existing `foo` defined here + --> $DIR/same_name_method.rs:82:9 + | +LL | impl T2 for S {} + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors diff --git a/tests/rustdoc-ui/unescaped_backticks.stderr b/tests/rustdoc-ui/unescaped_backticks.stderr index bd21dcb6e1af9..000a5b597d299 100644 --- a/tests/rustdoc-ui/unescaped_backticks.stderr +++ b/tests/rustdoc-ui/unescaped_backticks.stderr @@ -302,7 +302,6 @@ LL | | /// level changes. = help: if you meant to use a literal backtick, escape it change: or `None` if it isn't. to this: or `None\` if it isn't. - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: unescaped backtick --> $DIR/unescaped_backticks.rs:323:5 @@ -322,7 +321,6 @@ LL | | /// level changes. = help: if you meant to use a literal backtick, escape it change: `on_event` should be called. to this: `on_event\` should be called. - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: unescaped backtick --> $DIR/unescaped_backticks.rs:323:5 @@ -342,7 +340,6 @@ LL | | /// level changes. = help: if you meant to use a literal backtick, escape it change: [`rebuild_interest_cache`][rebuild] is called after the value of the max to this: [`rebuild_interest_cache\`][rebuild] is called after the value of the max - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error: unescaped backtick --> $DIR/unescaped_backticks.rs:349:56 diff --git a/tests/ui/consts/const-eval/stable-metric/ctfe-simple-loop.warn.stderr b/tests/ui/consts/const-eval/stable-metric/ctfe-simple-loop.warn.stderr index 8eee392d15cd5..40fc4a876e9e5 100644 --- a/tests/ui/consts/const-eval/stable-metric/ctfe-simple-loop.warn.stderr +++ b/tests/ui/consts/const-eval/stable-metric/ctfe-simple-loop.warn.stderr @@ -40,7 +40,6 @@ help: the constant being evaluated | LL | const Y: u32 = simple_loop(35); | ^^^^^^^^^^^^ - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: constant evaluation is taking a long time --> $DIR/ctfe-simple-loop.rs:9:5 diff --git a/tests/ui/imports/ambiguous-9.stderr b/tests/ui/imports/ambiguous-9.stderr index 2731ed2ba86d3..6c7d79174daf6 100644 --- a/tests/ui/imports/ambiguous-9.stderr +++ b/tests/ui/imports/ambiguous-9.stderr @@ -60,7 +60,6 @@ note: `date_range` could also refer to the function imported here LL | use prelude::*; | ^^^^^^^^^^ = help: consider adding an explicit import of `date_range` to disambiguate - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` warning: 4 warnings emitted