Skip to content

Commit cd654c6

Browse files
authored
Unrolled build for rust-lang#122578
Rollup merge of rust-lang#122578 - jieyouxu:guard-decorate, r=fee1-dead Only invoke `decorate` if the diag can eventually be emitted Lints can call [`trimmed_def_paths`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/print/fn.trimmed_def_paths.html#), such as through manual implementations of `LintDiagnostic` and calling `def_path_str`. https://github.com/rust-lang/rust/blob/05a2be3def211255dc7640b006ac10f0f02baf5c/compiler/rustc_lint/src/lints.rs#L1834-L1839 The emission of a lint eventually relies on [`TyCtxt::node_lint`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html#method.node_lint), which has a `decorate` closure which is responsible for decorating the diagnostic with "lint stuff". `node_lint` in turn relies on [`lint_level`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/lint/fn.lint_level.html). Within `lint_level`, `decorate` is eventually called just before `Diag::emit` is called to decorate the diagnostic. However, if `-A warnings` or `--cap-lint=allow` are set, or if the unused_must_use lint is explicitly allowed, then `decorate` would be called, which would call `def_path_str`, but the diagnostic would never be emitted and hence would trigger the `must_produce_diag` ICE. To avoid calling `decorate` when we don't eventually emit the diagnostic, we check that: - if `--force-warn` is specified, then call `decorate`; otherwise - if we can emit warnings (or higher), then call `decorate`. Fixes rust-lang#121774.
2 parents 35dfc67 + bdab02c commit cd654c6

File tree

6 files changed

+98
-2
lines changed

6 files changed

+98
-2
lines changed

compiler/rustc_middle/src/lint.rs

+16-2
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,22 @@ pub fn lint_level(
398398
}
399399
}
400400

401-
// Finally, run `decorate`.
402-
decorate(&mut err);
401+
// Finally, run `decorate`. `decorate` can call `trimmed_path_str` (directly or indirectly),
402+
// so we need to make sure when we do call `decorate` that the diagnostic is eventually
403+
// emitted or we'll get a `must_produce_diag` ICE.
404+
//
405+
// When is a diagnostic *eventually* emitted? Well, that is determined by 2 factors:
406+
// 1. If the corresponding `rustc_errors::Level` is beyond warning, i.e. `ForceWarning(_)`
407+
// or `Error`, then the diagnostic will be emitted regardless of CLI options.
408+
// 2. If the corresponding `rustc_errors::Level` is warning, then that can be affected by
409+
// `-A warnings` or `--cap-lints=xxx` on the command line. In which case, the diagnostic
410+
// will be emitted if `can_emit_warnings` is true.
411+
let skip = err_level == rustc_errors::Level::Warning && !sess.dcx().can_emit_warnings();
412+
413+
if !skip {
414+
decorate(&mut err);
415+
}
416+
403417
explain_lint_level_source(lint, level, src, &mut err);
404418
err.emit()
405419
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Checks that the following does not ICE because `decorate` is incorrectly skipped.
2+
3+
//@ compile-flags: -Dunused_must_use -Awarnings --crate-type=lib
4+
5+
#[must_use]
6+
fn f() {}
7+
8+
pub fn g() {
9+
f();
10+
//~^ ERROR unused return value
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: unused return value of `f` that must be used
2+
--> $DIR/decorate-can-emit-warnings.rs:9:5
3+
|
4+
LL | f();
5+
| ^^^
6+
|
7+
= note: requested on the command line with `-D unused-must-use`
8+
help: use `let _ = ...` to ignore the resulting value
9+
|
10+
LL | let _ = f();
11+
| +++++++
12+
13+
error: aborting due to 1 previous error
14+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Checks that the following does not ICE.
2+
//
3+
// Previously, this test ICEs when the `unused_must_use` lint is suppressed via the combination of
4+
// `-A warnings` and `--cap-lints=warn`, because:
5+
//
6+
// - Its lint diagnostic struct `UnusedDef` implements `LintDiagnostic` manually and in the impl
7+
// `def_path_str` was called (which calls `trimmed_def_path`, which will produce a
8+
// `must_produce_diag` ICE if a trimmed def path is constructed but never emitted in a diagnostic
9+
// because it is expensive to compute).
10+
// - A `LintDiagnostic` has a `decorate_lint` method which decorates a `Diag` with lint-specific
11+
// information. This method is wrapped by a `decorate` closure in `TyCtxt` diagnostic emission
12+
// machinery, and the `decorate` closure called as late as possible.
13+
// - `decorate`'s invocation is delayed as late as possible until `lint_level` is called.
14+
// - If a lint's corresponding diagnostic is suppressed (to be effectively allow at the final
15+
// emission time) via `-A warnings` or `--cap-lints=allow` (or `-A warnings` + `--cap-lints=warn`
16+
// like in this test case), `decorate` is still called and a diagnostic is still constructed --
17+
// but the diagnostic is never eventually emitted, triggering the aforementioned
18+
// `must_produce_diag` ICE due to use of `trimmed_def_path`.
19+
//
20+
// Issue: <https://github.com/rust-lang/rust/issues/121774>.
21+
22+
//@ compile-flags: -Dunused_must_use -Awarnings --cap-lints=warn --crate-type=lib
23+
//@ check-pass
24+
25+
#[must_use]
26+
fn f() {}
27+
28+
pub fn g() {
29+
f();
30+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Checks that the following does not ICE because `decorate` is incorrectly skipped due to
2+
// `--force-warn`.
3+
4+
//@ compile-flags: -Dunused_must_use -Awarnings --force-warn unused_must_use --crate-type=lib
5+
//@ check-pass
6+
7+
#[must_use]
8+
fn f() {}
9+
10+
pub fn g() {
11+
f();
12+
//~^ WARN unused return value
13+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
warning: unused return value of `f` that must be used
2+
--> $DIR/decorate-force-warn.rs:11:5
3+
|
4+
LL | f();
5+
| ^^^
6+
|
7+
= note: requested on the command line with `--force-warn unused-must-use`
8+
help: use `let _ = ...` to ignore the resulting value
9+
|
10+
LL | let _ = f();
11+
| +++++++
12+
13+
warning: 1 warning emitted
14+

0 commit comments

Comments
 (0)