Skip to content

Commit acb39ce

Browse files
authored
Rollup merge of #120490 - nnethercote:Diagnostic-hashing, r=estebank
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. r? `@estebank`
2 parents 1187855 + 4225a1e commit acb39ce

File tree

6 files changed

+23
-15
lines changed

6 files changed

+23
-15
lines changed

compiler/rustc_errors/src/diagnostic.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ pub struct Diagnostic {
109109
/// `span` if there is one. Otherwise, it is `DUMMY_SP`.
110110
pub sort_span: Span,
111111

112-
/// If diagnostic is from Lint, custom hash function ignores children.
113-
/// Otherwise hash is based on the all the fields.
114112
pub is_lint: Option<IsLint>,
115113

116114
/// With `-Ztrack_diagnostics` enabled,
@@ -980,22 +978,24 @@ impl Diagnostic {
980978
) -> (
981979
&Level,
982980
&[(DiagnosticMessage, Style)],
983-
Vec<(&Cow<'static, str>, &DiagnosticArgValue)>,
984981
&Option<ErrCode>,
985-
&Option<IsLint>,
986982
&MultiSpan,
983+
&[SubDiagnostic],
987984
&Result<Vec<CodeSuggestion>, SuggestionsDisabled>,
988-
Option<&[SubDiagnostic]>,
985+
Vec<(&DiagnosticArgName, &DiagnosticArgValue)>,
986+
&Option<IsLint>,
989987
) {
990988
(
991989
&self.level,
992990
&self.messages,
993-
self.args().collect(),
994991
&self.code,
995-
&self.is_lint,
996992
&self.span,
993+
&self.children,
997994
&self.suggestions,
998-
(if self.is_lint.is_some() { None } else { Some(&self.children) }),
995+
self.args().collect(),
996+
// omit self.sort_span
997+
&self.is_lint,
998+
// omit self.emitted_at
999999
)
10001000
}
10011001
}

src/tools/clippy/tests/ui/same_name_method.rs

+1
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ mod should_lint {
7474
impl S {
7575
fn foo() {}
7676
//~^ ERROR: method's name is the same as an existing method in a trait
77+
//~| ERROR: method's name is the same as an existing method in a trait
7778
}
7879

7980
impl T1 for S {}

src/tools/clippy/tests/ui/same_name_method.stderr

+14-2
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,22 @@ LL | fn foo() {}
5656
| ^^^^^^^^^^^
5757
|
5858
note: existing `foo` defined here
59-
--> $DIR/same_name_method.rs:79:9
59+
--> $DIR/same_name_method.rs:80:9
6060
|
6161
LL | impl T1 for S {}
6262
| ^^^^^^^^^^^^^^^^
6363

64-
error: aborting due to 5 previous errors
64+
error: method's name is the same as an existing method in a trait
65+
--> $DIR/same_name_method.rs:75:13
66+
|
67+
LL | fn foo() {}
68+
| ^^^^^^^^^^^
69+
|
70+
note: existing `foo` defined here
71+
--> $DIR/same_name_method.rs:82:9
72+
|
73+
LL | impl T2 for S {}
74+
| ^^^^^^^^^^^^^^^^
75+
76+
error: aborting due to 6 previous errors
6577

tests/rustdoc-ui/unescaped_backticks.stderr

-3
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,6 @@ LL | | /// level changes.
302302
= help: if you meant to use a literal backtick, escape it
303303
change: or `None` if it isn't.
304304
to this: or `None\` if it isn't.
305-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
306305

307306
error: unescaped backtick
308307
--> $DIR/unescaped_backticks.rs:323:5
@@ -322,7 +321,6 @@ LL | | /// level changes.
322321
= help: if you meant to use a literal backtick, escape it
323322
change: `on_event` should be called.
324323
to this: `on_event\` should be called.
325-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
326324

327325
error: unescaped backtick
328326
--> $DIR/unescaped_backticks.rs:323:5
@@ -342,7 +340,6 @@ LL | | /// level changes.
342340
= help: if you meant to use a literal backtick, escape it
343341
change: [`rebuild_interest_cache`][rebuild] is called after the value of the max
344342
to this: [`rebuild_interest_cache\`][rebuild] is called after the value of the max
345-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
346343

347344
error: unescaped backtick
348345
--> $DIR/unescaped_backticks.rs:349:56

tests/ui/consts/const-eval/stable-metric/ctfe-simple-loop.warn.stderr

-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ help: the constant being evaluated
4040
|
4141
LL | const Y: u32 = simple_loop(35);
4242
| ^^^^^^^^^^^^
43-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
4443

4544
warning: constant evaluation is taking a long time
4645
--> $DIR/ctfe-simple-loop.rs:9:5

tests/ui/imports/ambiguous-9.stderr

-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ note: `date_range` could also refer to the function imported here
6060
LL | use prelude::*;
6161
| ^^^^^^^^^^
6262
= help: consider adding an explicit import of `date_range` to disambiguate
63-
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
6463

6564
warning: 4 warnings emitted
6665

0 commit comments

Comments
 (0)