Skip to content

Commit

Permalink
Rollup merge of rust-lang#89500 - FabianWolff:issue-87308, r=petroche…
Browse files Browse the repository at this point in the history
…nkov

Fix ICE with buffered lint referring to AST node deleted by everybody_loops

Fixes rust-lang#87308. Note the following comment:
https://github.com/rust-lang/rust/blob/08759c691e2e9799a3c6780ffdf910240ebd4a6b/compiler/rustc_lint/src/early.rs#L415-L417

As it turns out, this is not _always_ a bug, because `-Zunpretty=everybody_loops` causes a lot of AST nodes to be deleted, and thus some buffered lints will refer to non-existent node ids. To fix this, my changes simply ignore buffered lints if `-Zunpretty=everybody_loops` is enabled, which, from my understanding, shouldn't be a big issue because it only affects pretty-printing. Of course, a more elegant solution would only ignore buffered lints that actually point at deleted node ids, but I haven't figured out an easy way of achieving this.

For the concrete example in rust-lang#87308, the buffered lint is created [here](https://github.com/rust-lang/rust/blob/08759c691e2e9799a3c6780ffdf910240ebd4a6b/compiler/rustc_expand/src/mbe/macro_rules.rs#L145-L151) with the `lint_node_id` from [here](https://github.com/rust-lang/rust/blob/08759c691e2e9799a3c6780ffdf910240ebd4a6b/compiler/rustc_expand/src/mbe/macro_rules.rs#L319), i.e. it points at the macro _expansion_, which then gets deleted by `ReplaceBodyWithLoop` [here](https://github.com/rust-lang/rust/blob/08759c691e2e9799a3c6780ffdf910240ebd4a6b/compiler/rustc_interface/src/passes.rs#L377).
  • Loading branch information
Manishearth authored Oct 4, 2021
2 parents e11685f + a28a78f commit ecd5bb8
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 6 deletions.
18 changes: 12 additions & 6 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,12 +437,18 @@ pub fn configure_and_expand(
});

// Add all buffered lints from the `ParseSess` to the `Session`.
sess.parse_sess.buffered_lints.with_lock(|buffered_lints| {
info!("{} parse sess buffered_lints", buffered_lints.len());
for early_lint in buffered_lints.drain(..) {
resolver.lint_buffer().add_early_lint(early_lint);
}
});
// The ReplaceBodyWithLoop pass may have deleted some AST nodes, potentially
// causing a delay_span_bug later if a buffered lint refers to such a deleted
// AST node (issue #87308). Since everybody_loops is for pretty-printing only,
// anyway, we simply skip all buffered lints here.
if !matches!(sess.opts.pretty, Some(PpMode::Source(PpSourceMode::EveryBodyLoops))) {
sess.parse_sess.buffered_lints.with_lock(|buffered_lints| {
info!("{} parse sess buffered_lints", buffered_lints.len());
for early_lint in buffered_lints.drain(..) {
resolver.lint_buffer().add_early_lint(early_lint);
}
});
}

Ok(krate)
}
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/lint/issue-87308.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Regression test for issue #87308.

// compile-flags: -Zunpretty=everybody_loops
// check-pass

macro_rules! foo {
() => { break 'x; }
}

pub fn main() {
'x: loop { foo!() }
}
14 changes: 14 additions & 0 deletions src/test/ui/lint/issue-87308.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use ::std::prelude::rust_2015::*;
#[macro_use]
extern crate std;
// Regression test for issue #87308.

// compile-flags: -Zunpretty=everybody_loops
// check-pass

macro_rules! foo { () => { break 'x ; } }

pub fn main() { loop { } }

0 comments on commit ecd5bb8

Please sign in to comment.