From 714fbd35679bc957b2708def1117a32953792c92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Sat, 16 Mar 2024 14:36:03 +0000 Subject: [PATCH] Invoke decorate when error level is beyond warning, including error --- compiler/rustc_middle/src/lint.rs | 23 +++++++++----- tests/ui/lint/decorate-def-path-str-ice.rs | 14 --------- .../decorate-can-emit-warnings.rs | 11 +++++++ .../decorate-can-emit-warnings.stderr | 14 +++++++++ .../decorate-ice/decorate-def-path-str-ice.rs | 30 +++++++++++++++++++ .../lint/decorate-ice/decorate-force-warn.rs | 13 ++++++++ .../decorate-ice/decorate-force-warn.stderr | 14 +++++++++ 7 files changed, 98 insertions(+), 21 deletions(-) delete mode 100644 tests/ui/lint/decorate-def-path-str-ice.rs create mode 100644 tests/ui/lint/decorate-ice/decorate-can-emit-warnings.rs create mode 100644 tests/ui/lint/decorate-ice/decorate-can-emit-warnings.stderr create mode 100644 tests/ui/lint/decorate-ice/decorate-def-path-str-ice.rs create mode 100644 tests/ui/lint/decorate-ice/decorate-force-warn.rs create mode 100644 tests/ui/lint/decorate-ice/decorate-force-warn.stderr diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index b83793df641b0..e79db835da48d 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/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) + || (err_level == ELevel::Warning && sess.dcx().can_emit_warnings()) + { + decorate(&mut err); + } } explain_lint_level_source(lint, level, src, &mut err); diff --git a/tests/ui/lint/decorate-def-path-str-ice.rs b/tests/ui/lint/decorate-def-path-str-ice.rs deleted file mode 100644 index 176f66ba1f4ab..0000000000000 --- a/tests/ui/lint/decorate-def-path-str-ice.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Checks that compiling this file with -// `-Dunused_must_use -Awarnings --cap-lints=warn --crate-type=lib` does not ICE when emitting -// diagnostics. -// Issue: . - -//@ compile-flags: -Dunused_must_use -Awarnings --cap-lints=warn --crate-type=lib -//@ check-pass - -#[must_use] -fn f() {} - -pub fn g() { - f(); -} diff --git a/tests/ui/lint/decorate-ice/decorate-can-emit-warnings.rs b/tests/ui/lint/decorate-ice/decorate-can-emit-warnings.rs new file mode 100644 index 0000000000000..5adb5f526dcc3 --- /dev/null +++ b/tests/ui/lint/decorate-ice/decorate-can-emit-warnings.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 +} diff --git a/tests/ui/lint/decorate-ice/decorate-can-emit-warnings.stderr b/tests/ui/lint/decorate-ice/decorate-can-emit-warnings.stderr new file mode 100644 index 0000000000000..fde81de6136ed --- /dev/null +++ b/tests/ui/lint/decorate-ice/decorate-can-emit-warnings.stderr @@ -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 + diff --git a/tests/ui/lint/decorate-ice/decorate-def-path-str-ice.rs b/tests/ui/lint/decorate-ice/decorate-def-path-str-ice.rs new file mode 100644 index 0000000000000..8116e36ed0d9a --- /dev/null +++ b/tests/ui/lint/decorate-ice/decorate-def-path-str-ice.rs @@ -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: . + +//@ compile-flags: -Dunused_must_use -Awarnings --cap-lints=warn --crate-type=lib +//@ check-pass + +#[must_use] +fn f() {} + +pub fn g() { + f(); +} diff --git a/tests/ui/lint/decorate-ice/decorate-force-warn.rs b/tests/ui/lint/decorate-ice/decorate-force-warn.rs new file mode 100644 index 0000000000000..e33210ed0cefa --- /dev/null +++ b/tests/ui/lint/decorate-ice/decorate-force-warn.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 +} diff --git a/tests/ui/lint/decorate-ice/decorate-force-warn.stderr b/tests/ui/lint/decorate-ice/decorate-force-warn.stderr new file mode 100644 index 0000000000000..5e6b74d414b27 --- /dev/null +++ b/tests/ui/lint/decorate-ice/decorate-force-warn.stderr @@ -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 +