Invoke decorate when error level is beyond warning, including error

Author: jieyouxu

compiler/rustc_middle/src/lint.rs

@@ -398,14 +398,23 @@ pub fn lint_level(
             }
         }
 
-        // Finally, run `decorate`. This is guarded by a `can_emit_warnings()` check so that any
-        // `def_path_str` called within `decorate` won't trigger a `must_produce_diag` ICE if the
-        // `err` isn't eventually emitted (e.g. due to `-A warnings`). If an `err` is force-warn,
-        // it's going to be emitted anyway.
-        if matches!(err_level, rustc_errors::Level::ForceWarning(_))
-            || sess.dcx().can_emit_warnings()
+        // Finally, run `decorate`. `decorate` can call `trimmed_path_str` (directly or indirectly),
+        // so we need to make sure when we do call `decorate` that the diagnostic is eventually
+        // emitted or we'll get a `must_produce_diag` ICE.
+        //
+        // When is a diagnostic *eventually* emitted? Well, that is determined by 2 factors:
+        // 1. If the corresponding `rustc_errors::Level` is beyond warning, i.e. `ForceWarning(_)`
+        //    or `Error`, then the diagnostic will be emitted regardless of CLI options.
+        // 2. If the corresponding `rustc_errors::Level` is warning, then that can be affected by
+        //    `-A warnings` or `--cap-lints=xxx` on the command line. In which case, the diagnostic
+        //    will be emitted if `can_emit_warnings` is true.
         {
-            decorate(&mut err);
+            use rustc_errors::Level as ELevel;
+            if matches!(err_level, ELevel::ForceWarning(_) | ELevel::Error)
+                || sess.dcx().can_emit_warnings()
+            {
+                decorate(&mut err);
+            }
         }
 
         explain_lint_level_source(lint, level, src, &mut err);

tests/ui/lint/decorate-def-path-str-ice.rs

@@ -0,0 +1,11 @@
+// Checks that the following does not ICE because `decorate` is incorrectly skipped.
+
+//@ compile-flags: -Dunused_must_use -Awarnings --crate-type=lib
+
+#[must_use]
+fn f() {}
+
+pub fn g() {
+    f();
+    //~^ ERROR unused return value
+}

tests/ui/lint/decorate-ice/decorate-can-emit-warnings.rs

@@ -0,0 +1,14 @@
+error: unused return value of `f` that must be used
+  --> $DIR/decorate-can-emit-warnings.rs:9:5
+   |
+LL |     f();
+   |     ^^^
+   |
+   = note: requested on the command line with `-D unused-must-use`
+help: use `let _ = ...` to ignore the resulting value
+   |
+LL |     let _ = f();
+   |     +++++++
+
+error: aborting due to 1 previous error
+

tests/ui/lint/decorate-ice/decorate-can-emit-warnings.stderr

@@ -0,0 +1,30 @@
+// Checks that the following does not ICE.
+//
+// Previously, this test ICEs when the `unused_must_use` lint is suppressed via the combination of
+// `-A warnings` and `--cap-lints=warn`, because:
+//
+// - Its lint diagnostic struct `UnusedDef` implements `LintDiagnostic` manually and in the impl
+//   `def_path_str` was called (which calls `trimmed_def_path`, which will produce a
+//   `must_produce_diag` ICE if a trimmed def path is constructed but never emitted in a diagnostic
+//   because it is expensive to compute).
+// - A `LintDiagnostic` has a `decorate_lint` method which decorates a `Diag` with lint-specific
+//   information. This method is wrapped by a `decorate` closure in `TyCtxt` diagnostic emission
+//   machinery, and the `decorate` closure called as late as possible.
+// - `decorate`'s invocation is delayed as late as possible until `lint_level` is called.
+// - If a lint's corresponding diagnostic is suppressed (to be effectively allow at the final
+//   emission time) via `-A warnings` or `--cap-lints=allow` (or `-A warnings` + `--cap-lints=warn`
+//   like in this test case), `decorate` is still called and a diagnostic is still constructed --
+//   but the diagnostic is never eventually emitted, triggering the aforementioned
+//   `must_produce_diag` ICE due to use of `trimmed_def_path`.
+//
+// Issue: <https://github.com/rust-lang/rust/issues/121774>.
+
+//@ compile-flags: -Dunused_must_use -Awarnings --cap-lints=warn --crate-type=lib
+//@ check-pass
+
+#[must_use]
+fn f() {}
+
+pub fn g() {
+    f();
+}

tests/ui/lint/decorate-ice/decorate-def-path-str-ice.rs

@@ -0,0 +1,13 @@
+// Checks that the following does not ICE because `decorate` is incorrectly skipped due to
+// `--force-warn`.
+
+//@ compile-flags: -Dunused_must_use -Awarnings --force-warn unused_must_use --crate-type=lib
+//@ check-pass
+
+#[must_use]
+fn f() {}
+
+pub fn g() {
+    f();
+    //~^ WARN unused return value
+}

tests/ui/lint/decorate-ice/decorate-force-warn.rs

@@ -0,0 +1,14 @@
+warning: unused return value of `f` that must be used
+  --> $DIR/decorate-force-warn.rs:11:5
+   |
+LL |     f();
+   |     ^^^
+   |
+   = note: requested on the command line with `--force-warn unused-must-use`
+help: use `let _ = ...` to ignore the resulting value
+   |
+LL |     let _ = f();
+   |     +++++++
+
+warning: 1 warning emitted
+