about summary refs log tree commit diff
diff options
context:
space:
mode:
authorNicholas Nethercote <n.nethercote@gmail.com>2024-02-28 11:00:27 +1100
committerNicholas Nethercote <n.nethercote@gmail.com>2024-02-29 11:08:29 +1100
commit82961c0abcd7b9ea73fb87fd049e2e853abd5787 (patch)
treebc3fa9ac95899fbcbdb78f3652a384534459c3de
parent9aff357e5398320962137a8885b924231904ef08 (diff)
downloadrust-82961c0abcd7b9ea73fb87fd049e2e853abd5787.tar.gz
rust-82961c0abcd7b9ea73fb87fd049e2e853abd5787.zip
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.
-rw-r--r--compiler/rustc_errors/src/lib.rs10
-rw-r--r--src/tools/rustfmt/src/formatting.rs21
-rw-r--r--src/tools/rustfmt/src/parse/session.rs8
3 files changed, 13 insertions, 26 deletions
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
index 60bda9f4c94..186ec41f7cd 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 323ae83fe6e..1c64921b1a6 100644
--- a/src/tools/rustfmt/src/formatting.rs
+++ b/src/tools/rustfmt/src/formatting.rs
@@ -109,7 +109,7 @@ fn format_project<T: FormatHandler>(
     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<T: FormatHandler>(
     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 e33f1ca755c..f5defe63c13 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<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());
     }