From 82961c0abcd7b9ea73fb87fd049e2e853abd5787 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 Feb 2024 11:00:27 +1100 Subject: [PATCH] Reinstate `emit_stashed_diagnostics` in `DiagCtxtInner::drop`. I removed it in #121206 because I thought thought it wasn't necessary. But then I had to add an `emit_stashed_diagnostics` call elsewhere in rustfmt to avoid the assertion failure (which took two attempts to get right, #121487 and #121615), and now there's an assertion failure in clippy as well (https://github.com/rust-lang/rust-clippy/issues/12364). So this commit just reinstates the call in `DiagCtxtInner::drop`. It also reverts the rustfmt changes from #121487 and #121615, though it keeps the tests added for those PRs. --- compiler/rustc_errors/src/lib.rs | 10 ++++++---- src/tools/rustfmt/src/formatting.rs | 21 ++++++--------------- src/tools/rustfmt/src/parse/session.rs | 8 +------- 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 60bda9f4c9495..186ec41f7cd1b 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -555,12 +555,14 @@ pub struct DiagCtxtFlags { impl Drop for DiagCtxtInner { fn drop(&mut self) { - // Any stashed diagnostics should have been handled by - // `emit_stashed_diagnostics` by now. + // For tools using `interface::run_compiler` (e.g. rustc, rustdoc) + // stashed diagnostics will have already been emitted. But for others + // that don't use `interface::run_compiler` (e.g. rustfmt, some clippy + // lints) this fallback is necessary. // // Important: it is sound to produce an `ErrorGuaranteed` when stashing - // errors because they are guaranteed to have been emitted by here. - assert!(self.stashed_diagnostics.is_empty()); + // errors because they are guaranteed to be emitted here or earlier. + self.emit_stashed_diagnostics(); // Important: it is sound to produce an `ErrorGuaranteed` when emitting // delayed bugs because they are guaranteed to be emitted here if diff --git a/src/tools/rustfmt/src/formatting.rs b/src/tools/rustfmt/src/formatting.rs index 323ae83fe6ede..1c64921b1a68d 100644 --- a/src/tools/rustfmt/src/formatting.rs +++ b/src/tools/rustfmt/src/formatting.rs @@ -109,7 +109,7 @@ fn format_project( let main_file = input.file_name(); let input_is_stdin = main_file == FileName::Stdin; - let mut parse_session = ParseSess::new(config)?; + let parse_session = ParseSess::new(config)?; if config.skip_children() && parse_session.ignore_file(&main_file) { return Ok(FormatReport::new()); } @@ -118,20 +118,11 @@ fn format_project( let mut report = FormatReport::new(); let directory_ownership = input.to_directory_ownership(); - // 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, - }; + 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; should_emit_verbose(forbid_verbose, config, || { eprintln!("The Rust parser panicked"); }); diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index e33f1ca755ca2..f5defe63c13ba 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -5,9 +5,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use rustc_data_structures::sync::{IntoDynSyncSend, Lrc}; use rustc_errors::emitter::{DynEmitter, Emitter, HumanEmitter}; use rustc_errors::translation::Translate; -use rustc_errors::{ - ColorConfig, Diag, DiagCtxt, DiagInner, ErrorGuaranteed, Level as DiagnosticLevel, -}; +use rustc_errors::{ColorConfig, Diag, DiagCtxt, DiagInner, Level as DiagnosticLevel}; use rustc_session::parse::ParseSess as RawParseSess; use rustc_span::{ source_map::{FilePathMapping, SourceMap}, @@ -230,10 +228,6 @@ impl ParseSess { self.ignore_path_set.as_ref().is_match(path) } - pub(crate) fn emit_stashed_diagnostics(&mut self) -> Option { - self.parse_sess.dcx.emit_stashed_diagnostics() - } - pub(crate) fn set_silent_emitter(&mut self) { self.parse_sess.dcx = DiagCtxt::with_emitter(silent_emitter()); }