Skip to content

Commit

Permalink
Move emit_stashed_diagnostic call in rustfmt.
Browse files Browse the repository at this point in the history
This call was added to `parse_crate_mod` in #121487, to fix a case where
a stashed diagnostic wasn't emitted. But there is another path where a
stashed diagnostic might fail to be emitted if there's a parse error, if
the `build` call in `parse_crate_inner` fails before `parse_crate_mod`
is reached.

So this commit moves the `emit_stashed_diagnostic` call outwards, from
`parse_crate_mod` to `format_project`, just after the
`Parser::parse_crate` call. This should be far out enough to catch any
parsing errors.

Fixes #121517.
  • Loading branch information
nnethercote committed Feb 25, 2024
1 parent 8c0b1fc commit bf7d1b5
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 19 deletions.
22 changes: 16 additions & 6 deletions src/tools/rustfmt/src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,19 +109,29 @@ fn format_project<T: FormatHandler>(
let main_file = input.file_name();
let input_is_stdin = main_file == FileName::Stdin;

let parse_session = ParseSess::new(config)?;
let mut parse_session = ParseSess::new(config)?;
if config.skip_children() && parse_session.ignore_file(&main_file) {
return Ok(FormatReport::new());
}

// Parse the crate.
let mut report = FormatReport::new();
let directory_ownership = input.to_directory_ownership();
let krate = match Parser::parse_crate(input, &parse_session) {
Ok(krate) => krate,
// Surface parse error via Session (errors are merged there from report)
Err(e) => {
let forbid_verbose = input_is_stdin || e != ParserError::ParsePanicError;

// rustfmt doesn't use `run_compiler` like other tools, so it must emit any
// stashed diagnostics itself, otherwise the `DiagCtxt` will assert when
// dropped. The final result here combines the parsing result and the
// `emit_stashed_diagnostics` result.
let parse_res = Parser::parse_crate(input, &parse_session);
let stashed_res = parse_session.emit_stashed_diagnostics();
let krate = match (parse_res, stashed_res) {
(Ok(krate), None) => krate,
(parse_res, _) => {
// Surface parse error via Session (errors are merged there from report).
let forbid_verbose = match parse_res {
Err(e) if e != ParserError::ParsePanicError => true,
_ => input_is_stdin,
};
should_emit_verbose(forbid_verbose, config, || {
eprintln!("The Rust parser panicked");
});
Expand Down
16 changes: 4 additions & 12 deletions src/tools/rustfmt/src/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,22 +162,14 @@ impl<'a> Parser<'a> {

fn parse_crate_mod(&mut self) -> Result<ast::Crate, ParserError> {
let mut parser = AssertUnwindSafe(&mut self.parser);

// rustfmt doesn't use `run_compiler` like other tools, so it must emit
// any stashed diagnostics itself, otherwise the `DiagCtxt` will assert
// when dropped. The final result here combines the parsing result and
// the `emit_stashed_diagnostics` result.
let parse_res = catch_unwind(move || parser.parse_crate_mod());
let stashed_res = self.parser.dcx().emit_stashed_diagnostics();
let err = Err(ParserError::ParsePanicError);
match (parse_res, stashed_res) {
(Ok(Ok(k)), None) => Ok(k),
(Ok(Ok(_)), Some(_guar)) => err,
(Ok(Err(db)), _) => {
match catch_unwind(move || parser.parse_crate_mod()) {
Ok(Ok(k)) => Ok(k),
Ok(Err(db)) => {
db.emit();
err
}
(Err(_), _) => err,
Err(_) => err,
}
}
}
6 changes: 5 additions & 1 deletion src/tools/rustfmt/src/parse/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_data_structures::sync::{IntoDynSyncSend, Lrc};
use rustc_errors::emitter::{DynEmitter, Emitter, HumanEmitter};
use rustc_errors::translation::Translate;
use rustc_errors::{
ColorConfig, DiagCtxt, Diagnostic, DiagnosticBuilder, Level as DiagnosticLevel,
ColorConfig, DiagCtxt, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, Level as DiagnosticLevel,
};
use rustc_session::parse::ParseSess as RawParseSess;
use rustc_span::{
Expand Down Expand Up @@ -230,6 +230,10 @@ impl ParseSess {
self.ignore_path_set.as_ref().is_match(path)
}

pub(crate) fn emit_stashed_diagnostics(&mut self) -> Option<ErrorGuaranteed> {
self.parse_sess.dcx.emit_stashed_diagnostics()
}

pub(crate) fn set_silent_emitter(&mut self) {
self.parse_sess.dcx = DiagCtxt::with_emitter(silent_emitter());
}
Expand Down
7 changes: 7 additions & 0 deletions src/tools/rustfmt/src/test/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,10 @@ fn crate_parsing_stashed_diag() {
let filename = "tests/parser/stashed-diag.rs";
assert_parser_error(filename);
}

#[test]
fn crate_parsing_stashed_diag2() {
// See also https://github.com/rust-lang/rust/issues/121517
let filename = "tests/parser/stashed-diag2.rs";
assert_parser_error(filename);
}
3 changes: 3 additions & 0 deletions src/tools/rustfmt/tests/parser/stashed-diag2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trait Trait<'1> { s> {}

fn main() {}

0 comments on commit bf7d1b5

Please sign in to comment.