Rollup merge of #136445 - bjorn3:diag_ctxt_cleanup, r=oli-obk

Couple of cleanups to DiagCtxt and EarlyDiagCtxt

Author: matthiaskrgr

Diff for: compiler/rustc_driver_impl/src/args.rs

@@ -1,7 +1,6 @@
 use std::{env, error, fmt, fs, io};
 
 use rustc_session::EarlyDiagCtxt;
-use rustc_span::ErrorGuaranteed;
 
 /// Expands argfiles in command line arguments.
 #[derive(Default)]
@@ -118,22 +117,22 @@ pub fn arg_expand_all(early_dcx: &EarlyDiagCtxt, at_args: &[String]) -> Vec<Stri
 ///
 /// This function is identical to [`env::args()`] except that it emits an error when it encounters
 /// non-Unicode arguments instead of panicking.
-pub fn raw_args(early_dcx: &EarlyDiagCtxt) -> Result<Vec<String>, ErrorGuaranteed> {
-    let mut res = Ok(Vec::new());
+pub fn raw_args(early_dcx: &EarlyDiagCtxt) -> Vec<String> {
+    let mut args = Vec::new();
+    let mut guar = Ok(());
     for (i, arg) in env::args_os().enumerate() {
         match arg.into_string() {
-            Ok(arg) => {
-                if let Ok(args) = &mut res {
-                    args.push(arg);
-                }
-            }
+            Ok(arg) => args.push(arg),
             Err(arg) => {
-                res =
+                guar =
                     Err(early_dcx.early_err(format!("argument {i} is not valid Unicode: {arg:?}")))
             }
         }
     }
-    res
+    if let Err(guar) = guar {
+        guar.raise_fatal();
+    }
+    args
 }
 
 #[derive(Debug)]

Diff for: compiler/rustc_driver_impl/src/lib.rs

@@ -1212,9 +1212,9 @@ pub fn catch_fatal_errors<F: FnOnce() -> R, R>(f: F) -> Result<R, FatalError> {
 
 /// Variant of `catch_fatal_errors` for the `interface::Result` return type
 /// that also computes the exit code.
-pub fn catch_with_exit_code(f: impl FnOnce() -> interface::Result<()>) -> i32 {
+pub fn catch_with_exit_code(f: impl FnOnce()) -> i32 {
     match catch_fatal_errors(f) {
-        Ok(Ok(())) => EXIT_SUCCESS,
+        Ok(()) => EXIT_SUCCESS,
         _ => EXIT_FAILURE,
     }
 }
@@ -1499,10 +1499,8 @@ pub fn main() -> ! {
     install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ());
     install_ctrlc_handler();
 
-    let exit_code = catch_with_exit_code(|| {
-        run_compiler(&args::raw_args(&early_dcx)?, &mut callbacks);
-        Ok(())
-    });
+    let exit_code =
+        catch_with_exit_code(|| run_compiler(&args::raw_args(&early_dcx), &mut callbacks));
 
     if let Some(format) = callbacks.time_passes {
         let end_rss = get_resident_set_size();

Diff for: compiler/rustc_errors/src/emitter.rs

@@ -35,8 +35,8 @@ use crate::snippet::{
 use crate::styled_buffer::StyledBuffer;
 use crate::translation::{Translate, to_fluent_args};
 use crate::{
-    CodeSuggestion, DiagCtxt, DiagInner, DiagMessage, ErrCode, FluentBundle, LazyFallbackBundle,
-    Level, MultiSpan, Subdiag, SubstitutionHighlight, SuggestionStyle, TerminalUrl,
+    CodeSuggestion, DiagInner, DiagMessage, ErrCode, FluentBundle, LazyFallbackBundle, Level,
+    MultiSpan, Subdiag, SubstitutionHighlight, SuggestionStyle, TerminalUrl,
 };
 
 /// Default column width, used in tests and when terminal dimensions cannot be determined.
@@ -537,11 +537,10 @@ impl Emitter for HumanEmitter {
 }
 
 /// An emitter that does nothing when emitting a non-fatal diagnostic.
-/// Fatal diagnostics are forwarded to `fatal_dcx` to avoid silent
+/// Fatal diagnostics are forwarded to `fatal_emitter` to avoid silent
 /// failures of rustc, as witnessed e.g. in issue #89358.
 pub struct SilentEmitter {
-    pub fallback_bundle: LazyFallbackBundle,
-    pub fatal_dcx: DiagCtxt,
+    pub fatal_emitter: Box<dyn Emitter + DynSend>,
     pub fatal_note: Option<String>,
     pub emit_fatal_diagnostic: bool,
 }
@@ -552,9 +551,7 @@ impl Translate for SilentEmitter {
     }
 
     fn fallback_fluent_bundle(&self) -> &FluentBundle {
-        // Ideally this field wouldn't be necessary and the fallback bundle in `fatal_dcx` would be
-        // used but the lock prevents this.
-        &self.fallback_bundle
+        self.fatal_emitter.fallback_fluent_bundle()
     }
 }
 
@@ -563,12 +560,12 @@ impl Emitter for SilentEmitter {
         None
     }
 
-    fn emit_diagnostic(&mut self, mut diag: DiagInner, _registry: &Registry) {
+    fn emit_diagnostic(&mut self, mut diag: DiagInner, registry: &Registry) {
         if self.emit_fatal_diagnostic && diag.level == Level::Fatal {
             if let Some(fatal_note) = &self.fatal_note {
                 diag.sub(Level::Note, fatal_note.clone(), MultiSpan::new());
             }
-            self.fatal_dcx.handle().emit_diagnostic(diag);
+            self.fatal_emitter.emit_diagnostic(diag, registry);
         }
     }
 }

Diff for: compiler/rustc_errors/src/lib.rs

@@ -59,7 +59,7 @@ use emitter::{DynEmitter, Emitter, is_case_difference, is_different};
 use rustc_data_structures::AtomicRef;
 use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_data_structures::stable_hasher::{Hash128, StableHasher};
-use rustc_data_structures::sync::Lock;
+use rustc_data_structures::sync::{DynSend, Lock};
 pub use rustc_error_messages::{
     DiagMessage, FluentBundle, LanguageIdentifier, LazyFallbackBundle, MultiSpan, SpanLabel,
     SubdiagMessage, fallback_fluent_bundle, fluent_bundle,
@@ -676,57 +676,44 @@ impl DiagCtxt {
         Self { inner: Lock::new(DiagCtxtInner::new(emitter)) }
     }
 
-    pub fn make_silent(
-        &self,
-        fallback_bundle: LazyFallbackBundle,
-        fatal_note: Option<String>,
-        emit_fatal_diagnostic: bool,
-    ) {
-        self.wrap_emitter(|old_dcx| {
-            Box::new(emitter::SilentEmitter {
-                fallback_bundle,
-                fatal_dcx: DiagCtxt { inner: Lock::new(old_dcx) },
-                fatal_note,
-                emit_fatal_diagnostic,
-            })
-        });
-    }
-
-    fn wrap_emitter<F>(&self, f: F)
-    where
-        F: FnOnce(DiagCtxtInner) -> Box<DynEmitter>,
-    {
-        // A empty type that implements `Emitter` so that a `DiagCtxtInner` can be constructed
-        // to temporarily swap in place of the real one, which will be used in constructing
-        // its replacement.
+    pub fn make_silent(&self, fatal_note: Option<String>, emit_fatal_diagnostic: bool) {
+        // An empty type that implements `Emitter` to temporarily swap in place of the real one,
+        // which will be used in constructing its replacement.
         struct FalseEmitter;
 
         impl Emitter for FalseEmitter {
             fn emit_diagnostic(&mut self, _: DiagInner, _: &Registry) {
-                unimplemented!("false emitter must only used during `wrap_emitter`")
+                unimplemented!("false emitter must only used during `make_silent`")
             }
 
             fn source_map(&self) -> Option<&SourceMap> {
-                unimplemented!("false emitter must only used during `wrap_emitter`")
+                unimplemented!("false emitter must only used during `make_silent`")
             }
         }
 
         impl translation::Translate for FalseEmitter {
             fn fluent_bundle(&self) -> Option<&FluentBundle> {
-                unimplemented!("false emitter must only used during `wrap_emitter`")
+                unimplemented!("false emitter must only used during `make_silent`")
             }
 
             fn fallback_fluent_bundle(&self) -> &FluentBundle {
-                unimplemented!("false emitter must only used during `wrap_emitter`")
+                unimplemented!("false emitter must only used during `make_silent`")
             }
         }
 
         let mut inner = self.inner.borrow_mut();
-        let mut prev_dcx = DiagCtxtInner::new(Box::new(FalseEmitter));
-        std::mem::swap(&mut *inner, &mut prev_dcx);
-        let new_emitter = f(prev_dcx);
-        let mut new_dcx = DiagCtxtInner::new(new_emitter);
-        std::mem::swap(&mut *inner, &mut new_dcx);
+        let mut prev_emitter = Box::new(FalseEmitter) as Box<dyn Emitter + DynSend>;
+        std::mem::swap(&mut inner.emitter, &mut prev_emitter);
+        let new_emitter = Box::new(emitter::SilentEmitter {
+            fatal_emitter: prev_emitter,
+            fatal_note,
+            emit_fatal_diagnostic,
+        });
+        inner.emitter = new_emitter;
+    }
+
+    pub fn set_emitter(&self, emitter: Box<dyn Emitter + DynSend>) {
+        self.inner.borrow_mut().emitter = emitter;
     }
 
     /// Translate `message` eagerly with `args` to `SubdiagMessage::Eager`.

Diff for: compiler/rustc_session/src/config.rs

@@ -1819,7 +1819,7 @@ pub fn parse_error_format(
                 ErrorOutputType::HumanReadable(HumanReadableErrorType::Unicode, color)
             }
             Some(arg) => {
-                early_dcx.abort_if_error_and_set_error_format(ErrorOutputType::HumanReadable(
+                early_dcx.set_error_format(ErrorOutputType::HumanReadable(
                     HumanReadableErrorType::Default,
                     color,
                 ));
@@ -2360,7 +2360,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
 
     let error_format = parse_error_format(early_dcx, matches, color, json_color, json_rendered);
 
-    early_dcx.abort_if_error_and_set_error_format(error_format);
+    early_dcx.set_error_format(error_format);
 
     let diagnostic_width = matches.opt_get("diagnostic-width").unwrap_or_else(|_| {
         early_dcx.early_fatal("`--diagnostic-width` must be an positive integer");
@@ -2770,6 +2770,7 @@ pub mod nightly_options {
                         "the option `{}` is only accepted on the nightly compiler",
                         opt.name
                     );
+                    // The non-zero nightly_options_on_stable will force an early_fatal eventually.
                     let _ = early_dcx.early_err(msg);
                 }
                 OptionStability::Stable => {}

Diff for: compiler/rustc_session/src/parse.rs

@@ -277,14 +277,10 @@ impl ParseSess {
     ) -> Self {
         let fallback_bundle = fallback_fluent_bundle(locale_resources, false);
         let sm = Lrc::new(SourceMap::new(FilePathMapping::empty()));
-        let emitter = Box::new(HumanEmitter::new(
-            stderr_destination(ColorConfig::Auto),
-            Lrc::clone(&fallback_bundle),
-        ));
-        let fatal_dcx = DiagCtxt::new(emitter);
+        let fatal_emitter =
+            Box::new(HumanEmitter::new(stderr_destination(ColorConfig::Auto), fallback_bundle));
         let dcx = DiagCtxt::new(Box::new(SilentEmitter {
-            fallback_bundle,
-            fatal_dcx,
+            fatal_emitter,
             fatal_note: Some(fatal_note),
             emit_fatal_diagnostic,
         }))
@@ -341,8 +337,4 @@ impl ParseSess {
     pub fn dcx(&self) -> DiagCtxtHandle<'_> {
         self.dcx.handle()
     }
-
-    pub fn set_dcx(&mut self, dcx: DiagCtxt) {
-        self.dcx = dcx;
-    }
 }

Diff for: compiler/rustc_session/src/session.rs

@@ -1362,23 +1362,16 @@ pub struct EarlyDiagCtxt {
     dcx: DiagCtxt,
 }
 
-impl Default for EarlyDiagCtxt {
-    fn default() -> Self {
-        Self::new(ErrorOutputType::default())
-    }
-}
-
 impl EarlyDiagCtxt {
     pub fn new(output: ErrorOutputType) -> Self {
         let emitter = mk_emitter(output);
         Self { dcx: DiagCtxt::new(emitter) }
     }
 
     /// Swap out the underlying dcx once we acquire the user's preference on error emission
-    /// format. Any errors prior to that will cause an abort and all stashed diagnostics of the
-    /// previous dcx will be emitted.
-    pub fn abort_if_error_and_set_error_format(&mut self, output: ErrorOutputType) {
-        self.dcx.handle().abort_if_errors();
+    /// format. If `early_err` was previously called this will panic.
+    pub fn set_error_format(&mut self, output: ErrorOutputType) {
+        assert!(self.dcx.handle().has_errors().is_none());
 
         let emitter = mk_emitter(output);
         self.dcx = DiagCtxt::new(emitter);
@@ -1398,7 +1391,7 @@ impl EarlyDiagCtxt {
 
     #[allow(rustc::untranslatable_diagnostic)]
     #[allow(rustc::diagnostic_outside_of_impl)]
-    #[must_use = "ErrorGuaranteed must be returned from `run_compiler` in order to exit with a non-zero status code"]
+    #[must_use = "raise_fatal must be called on the returned ErrorGuaranteed in order to exit with a non-zero status code"]
     pub fn early_err(&self, msg: impl Into<DiagMessage>) -> ErrorGuaranteed {
         self.dcx.handle().err(msg)
     }

Diff for: src/doc/rustc-dev-guide/examples/rustc-interface-getting-diagnostics.rs

@@ -10,15 +10,15 @@ extern crate rustc_interface;
 extern crate rustc_session;
 extern crate rustc_span;
 
+use std::sync::{Arc, Mutex};
+
 use rustc_errors::emitter::Emitter;
 use rustc_errors::registry::{self, Registry};
 use rustc_errors::translation::Translate;
 use rustc_errors::{DiagCtxt, DiagInner, FluentBundle};
 use rustc_session::config;
 use rustc_span::source_map::SourceMap;
 
-use std::sync::{Arc, Mutex};
-
 struct DebugEmitter {
     source_map: Arc<SourceMap>,
     diagnostics: Arc<Mutex<Vec<DiagInner>>>,
@@ -67,10 +67,10 @@ fn main() {
         locale_resources: rustc_driver::DEFAULT_LOCALE_RESOURCES.to_owned(),
         lint_caps: rustc_hash::FxHashMap::default(),
         psess_created: Some(Box::new(|parse_sess| {
-            parse_sess.set_dcx(DiagCtxt::new(Box::new(DebugEmitter {
+            parse_sess.dcx().set_emitter(Box::new(DebugEmitter {
                 source_map: parse_sess.clone_source_map(),
                 diagnostics,
-            })));
+            }));
         })),
         register_lints: None,
         override_queries: None,

Diff for: src/librustdoc/lib.rs

@@ -177,9 +177,8 @@ pub fn main() {
     rustc_driver::init_logger(&early_dcx, rustc_log::LoggerConfig::from_env("RUSTDOC_LOG"));
 
     let exit_code = rustc_driver::catch_with_exit_code(|| {
-        let at_args = rustc_driver::args::raw_args(&early_dcx)?;
+        let at_args = rustc_driver::args::raw_args(&early_dcx);
         main_args(&mut early_dcx, &at_args);
-        Ok(())
     });
     process::exit(exit_code);
 }

Diff for: src/tools/clippy/src/driver.rs

@@ -197,7 +197,7 @@ pub fn main() {
     });
 
     exit(rustc_driver::catch_with_exit_code(move || {
-        let mut orig_args = rustc_driver::args::raw_args(&early_dcx)?;
+        let mut orig_args = rustc_driver::args::raw_args(&early_dcx);
 
         let has_sysroot_arg = |args: &mut [String]| -> bool {
             if has_arg(args, "--sysroot") {
@@ -239,7 +239,7 @@ pub fn main() {
             pass_sysroot_env_if_given(&mut args, sys_root_env);
 
             rustc_driver::run_compiler(&args, &mut DefaultCallbacks);
-            return Ok(());
+            return;
         }
 
         if orig_args.iter().any(|a| a == "--version" || a == "-V") {
@@ -301,7 +301,6 @@ pub fn main() {
         } else {
             rustc_driver::run_compiler(&args, &mut RustcCallbacks { clippy_args_var });
         }
-        Ok(())
     }))
 }
 

Diff for: src/tools/miri/src/bin/miri.rs

@@ -379,10 +379,8 @@ fn run_compiler_and_exit(
     callbacks: &mut (dyn rustc_driver::Callbacks + Send),
 ) -> ! {
     // Invoke compiler, and handle return code.
-    let exit_code = rustc_driver::catch_with_exit_code(move || {
-        rustc_driver::run_compiler(args, callbacks);
-        Ok(())
-    });
+    let exit_code =
+        rustc_driver::catch_with_exit_code(move || rustc_driver::run_compiler(args, callbacks));
     std::process::exit(exit_code)
 }
 
@@ -461,7 +459,7 @@ fn main() {
     // (`install_ice_hook` might change `RUST_BACKTRACE`.)
     let env_snapshot = env::vars_os().collect::<Vec<_>>();
 
-    let args = rustc_driver::args::raw_args(&early_dcx)
+    let args = rustc_driver::catch_fatal_errors(|| rustc_driver::args::raw_args(&early_dcx))
         .unwrap_or_else(|_| std::process::exit(rustc_driver::EXIT_FAILURE));
 
     // Install the ctrlc handler that sets `rustc_const_eval::CTRL_C_RECEIVED`, even if