Improve diagnostic deduplication

compiler/rustc_errors/src/dedup_hash.rs

@@ -0,0 +1,63 @@
+use std::hash::Hash;
+
+use rustc_data_structures::stable_hash::StableHasher;
+use rustc_serialize::Encoder;
+use rustc_span::def_id::{CrateNum, DefId, DefIndex};
+use rustc_span::{ByteSymbol, ExpnId, Span, SpanEncoder, Symbol, SyntaxContext};
+
+/// Hasher used to determine if two diagnostics are the same and should be deduplicated. Used by
+/// `DiagCtxtInner::emitted_diagnostics`. It's a stable hash with one special behaviour: parents of
+/// spans within the diagnostics are ignored so that incremental and non-incremental compilation
+/// get the same behaviour.
+///
+/// Although this has nothing to do with encoding data to/from file, it is implemented on top of
+/// `Encoder`/`SpanEncoder` because they provide traversals of all the relevant types used within
+/// diagnostics. There is no corresponding decoder.
+pub(crate) struct DedupHashEncoder(pub(crate) StableHasher);
+
+macro_rules! encoder_methods {
+    ($($name:ident($ty:ty);)*) => {
+        $(
+            #[inline]
+            fn $name(&mut self, value: $ty) {
+                value.hash(&mut self.0)
+            }
+        )*
+    }
+}
+
+impl Encoder for DedupHashEncoder {
+    encoder_methods! {
+        emit_usize(usize);
+        emit_u128(u128);
+        emit_u64(u64);
+        emit_u32(u32);
+        emit_u16(u16);
+        emit_u8(u8);
+
+        emit_isize(isize);
+        emit_i128(i128);
+        emit_i64(i64);
+        emit_i32(i32);
+        emit_i16(i16);
+
+        emit_raw_bytes(&[u8]);
+    }
+}
+
+impl SpanEncoder for DedupHashEncoder {
+    fn encode_span(&mut self, span: Span) {
+        // The raison d'être of `DedupHashEncoder` is to ignore `parent` here.
+        span.with_parent(None).hash(&mut self.0);
+    }
+
+    encoder_methods! {
+        encode_symbol(Symbol);
+        encode_byte_symbol(ByteSymbol);
+        encode_expn_id(ExpnId);
+        encode_syntax_context(SyntaxContext);
+        encode_crate_num(CrateNum);
+        encode_def_index(DefIndex);
+        encode_def_id(DefId);
+    }
+}

compiler/rustc_errors/src/diagnostic.rs

@@ -12,9 +12,11 @@ use rustc_error_messages::{DiagArgMap, DiagArgName, IntoDiagArg};
 use rustc_hashes::Hash128;
 use rustc_lint_defs::Applicability;
 use rustc_macros::{Decodable, Encodable};
+use rustc_serialize::Encodable;
 use rustc_span::{Span, Spanned, Symbol};
 use tracing::debug;
 
+use crate::dedup_hash::DedupHashEncoder;
 use crate::{
     CodeSuggestion, DiagCtxtHandle, DiagMessage, ErrCode, ErrorGuaranteed, ExplicitBug, Level,
     MultiSpan, StashKey, Style, Sublevel, Substitution, SubstitutionPart, SuggestionStyle,
@@ -298,20 +300,22 @@ impl DiagInner {
             emitted_at: _,   // ignore
         } = self;
 
-        let hashed_parts = (
-            std::mem::discriminant(level), // ignore the field within `Warning`
-            messages,
-            code,
-            span,
-            children,
-            suggestions,
-            args.as_slice(),
-            is_lint,
-        );
+        let mut e = DedupHashEncoder(StableHasher::new());
 
-        let mut hasher = StableHasher::new();
-        hashed_parts.hash(&mut hasher);
-        hasher.finish()
+        // We use `discriminant` because we want to ignore the field within `Warning`.
+        // `Encoder`/`SpanEncoder` can't traverse a `Discriminant<T>` so we hash it directly.
+        std::mem::discriminant(level).hash(&mut e.0);
+
+        // All the other fields are hashed via `encode`.
+        messages.encode(&mut e);
+        code.encode(&mut e);
+        span.encode(&mut e);
+        children.encode(&mut e);
+        suggestions.encode(&mut e);
+        args.encode(&mut e);
+        is_lint.encode(&mut e);
+
+        e.0.finish()
     }
 }
 
@@ -324,6 +328,16 @@ pub struct Subdiag {
     pub span: MultiSpan,
 }
 
+impl Subdiag {
+    /// Hash used to determine if two subdiagnostics are the same. Used by
+    /// `DiagCtxtInner::emitted_diagnostics`.
+    pub(crate) fn dedup_hash(&self) -> Hash128 {
+        let mut e = DedupHashEncoder(StableHasher::new());
+        self.encode(&mut e);
+        e.0.finish()
+    }
+}
+
 /// Used for emitting structured error messages and other diagnostic information.
 /// Wraps a `DiagInner`, adding some useful things.
 /// - The `dcx` field, allowing it to (a) emit itself, and (b) do a drop check

compiler/rustc_errors/src/lib.rs

@@ -44,7 +44,6 @@ use emitter::{DynEmitter, Emitter};
 use rustc_ast::attr::version::RustcVersion;
 use rustc_data_structures::AtomicRef;
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
-use rustc_data_structures::stable_hash::StableHasher;
 use rustc_data_structures::sync::{DynSend, Lock};
 pub use rustc_error_messages::{
     DiagArg, DiagArgFromDisplay, DiagArgMap, DiagArgName, DiagArgValue, DiagMessage, IntoDiagArg,
@@ -70,6 +69,7 @@ use crate::timings::TimingRecord;
 pub mod annotate_snippet_emitter_writer;
 pub mod codes;
 mod decorate_diag;
+mod dedup_hash;
 mod diagnostic;
 mod diagnostic_impls;
 pub mod emitter;
@@ -1321,21 +1321,18 @@ impl DiagCtxtInner {
                 debug!(?diagnostic);
                 debug!(?self.emitted_diagnostics);
 
-                let not_yet_emitted = |sub: &mut Subdiag| {
+                let show_sub = |sub: &Subdiag| {
                     debug!(?sub);
                     match sub.level {
                         Sublevel::Error | Sublevel::Warning | Sublevel::Note | Sublevel::Help => {
-                            return true;
+                            true
+                        }
+                        Sublevel::OnceNote | Sublevel::OnceHelp => {
+                            self.emitted_diagnostics.insert(sub.dedup_hash())
                         }
-                        Sublevel::OnceNote | Sublevel::OnceHelp => {}
                     }
-                    let mut hasher = StableHasher::new();
-                    sub.hash(&mut hasher);
-                    let diagnostic_hash = hasher.finish();
-                    debug!(?diagnostic_hash);
-                    self.emitted_diagnostics.insert(diagnostic_hash)
                 };
-                diagnostic.children.retain_mut(not_yet_emitted);
+                diagnostic.children.retain(show_sub);
                 if already_emitted {
                     let msg = "duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`";
                     diagnostic.sub(Sublevel::Note, msg, MultiSpan::new());

compiler/rustc_middle/src/query/on_disk_cache.rs

@@ -929,10 +929,12 @@ impl<'tcx> TyEncoder<'tcx> for CacheEncoder<'tcx> {
 
 macro_rules! encoder_methods {
     ($($name:ident($ty:ty);)*) => {
-        #[inline]
-        $(fn $name(&mut self, value: $ty) {
-            self.encoder.$name(value)
-        })*
+        $(
+            #[inline]
+            fn $name(&mut self, value: $ty) {
+                self.encoder.$name(value)
+            }
+        )*
     }
 }
 

tests/ui/diagnostic-flags/deduplicate-diagnostics-incr.dedup.stderr

@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.

tests/ui/diagnostic-flags/deduplicate-diagnostics-incr.dup.stderr

@@ -0,0 +1,54 @@
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

tests/ui/diagnostic-flags/deduplicate-diagnostics-incr.incr-dedup.stderr

@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.

tests/ui/diagnostic-flags/deduplicate-diagnostics-incr.incr-dup.stderr

@@ -0,0 +1,54 @@
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error[E0277]: the trait bound `usize: GlobalAlloc` is not satisfied
+  --> $DIR/deduplicate-diagnostics-incr.rs:11:11
+   |
+LL | #[global_allocator]
+   | ------------------- in this attribute macro expansion
+LL | static A: usize = 0;
+   |           ^^^^^ the nightly-only, unstable trait `GlobalAllocator` is not implemented for `usize`
+   |
+help: the trait `GlobalAllocator` is implemented for `System`
+  --> $SRC_DIR/std/src/alloc.rs:LL:COL
+   = note: required for `usize` to implement `GlobalAlloc`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

tests/ui/diagnostic-flags/deduplicate-diagnostics-incr.rs

@@ -0,0 +1,17 @@
+//! Test that span parents (which are incremental only) are ignored when deduplicating diagnostics.
+//! Regression test for #162901.
+
+//@ revisions: dedup dup incr-dedup incr-dup
+//@[dedup] compile-flags: -Z deduplicate-diagnostics=yes
+//@[incr-dup] incremental
+//@[incr-dedup] incremental
+//@[incr-dedup] compile-flags: -Z deduplicate-diagnostics=yes
+
+#[global_allocator]
+static A: usize = 0;
+//[dedup,dup,incr-dedup,incr-dup]~^ ERROR E0277
+//[dup,incr-dup]~| ERROR E0277
+//[dup,incr-dup]~| ERROR E0277
+//[dup,incr-dup]~| ERROR E0277
+
+fn main() {}

typos.toml

@@ -35,6 +35,7 @@ numer = "numer" # short for numerator, not a typo for "number"
 old-skool = "old-skool" # variant spelling of "old-school"
 optin = "optin" # short for opt-in
 publically = "publically"
+raison = "raison" # part of the French phrase "raison d'être"
 rplace = "rplace" # short for R-place
 splitted = "splitted"
 sythetic = "sythetic" # typo in vendored LLVM sources