Skip to content

Commit

Permalink
Rollup merge of rust-lang#121206 - nnethercote:top-level-error-handli…
Browse files Browse the repository at this point in the history
…ng, r=oli-obk

Top level error handling

The interactions between the following things are surprisingly complicated:
- `emit_stashed_diagnostics`,
- `flush_delayed`,
- normal return vs `abort_if_errors`/`FatalError.raise()` unwinding in the call to the closure in `interface::run_compiler`.

This PR disentangles it all.

r? ``@oli-obk``
  • Loading branch information
Noratrieb authored Feb 20, 2024
2 parents a3519ed + a094903 commit 01c13f8
Show file tree
Hide file tree
Showing 27 changed files with 174 additions and 156 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
&item.vis,
errors::VisibilityNotPermittedNote::TraitImpl,
);
// njn: use Dummy here
if let TyKind::Err(_) = self_ty.kind {
this.dcx().emit_err(errors::ObsoleteAuto { span: item.span });
}
Expand Down
13 changes: 6 additions & 7 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rustc_ast::CRATE_NODE_ID;
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::memmap::Mmap;
use rustc_data_structures::temp_dir::MaybeTempDir;
use rustc_errors::{DiagCtxt, ErrorGuaranteed};
use rustc_errors::{DiagCtxt, ErrorGuaranteed, FatalError};
use rustc_fs_util::{fix_windows_verbatim_for_gcc, try_canonicalize};
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
use rustc_metadata::find_native_static_library;
Expand Down Expand Up @@ -487,7 +487,9 @@ fn collate_raw_dylibs<'a, 'b>(
}
}
}
sess.compile_status()?;
if let Some(guar) = sess.dcx().has_errors() {
return Err(guar);
}
Ok(dylib_table
.into_iter()
.map(|(name, imports)| {
Expand Down Expand Up @@ -720,10 +722,7 @@ fn link_dwarf_object<'a>(
Ok(())
}) {
Ok(()) => {}
Err(e) => {
sess.dcx().emit_err(errors::ThorinErrorWrapper(e));
sess.dcx().abort_if_errors();
}
Err(e) => sess.dcx().emit_fatal(errors::ThorinErrorWrapper(e)),
}
}

Expand Down Expand Up @@ -999,7 +998,7 @@ fn link_natively<'a>(
sess.dcx().emit_note(errors::CheckInstalledVisualStudio);
sess.dcx().emit_note(errors::InsufficientVSCodeProduct);
}
sess.dcx().abort_if_errors();
FatalError.raise();
}
}

Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,10 +449,7 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
let Some(llfn) = cx.declare_c_main(llfty) else {
// FIXME: We should be smart and show a better diagnostic here.
let span = cx.tcx().def_span(rust_main_def_id);
let dcx = cx.tcx().dcx();
dcx.emit_err(errors::MultipleMainFunctions { span });
dcx.abort_if_errors();
bug!();
cx.tcx().dcx().emit_fatal(errors::MultipleMainFunctions { span });
};

// `main` should respect same config for frame pointer elimination as rest of code
Expand Down
39 changes: 18 additions & 21 deletions compiler/rustc_driver_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,16 +144,6 @@ pub const EXIT_FAILURE: i32 = 1;
pub const DEFAULT_BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/issues/new\
?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md";

pub fn abort_on_err<T>(result: Result<T, ErrorGuaranteed>, sess: &Session) -> T {
match result {
Err(..) => {
sess.dcx().abort_if_errors();
panic!("error reported but abort_if_errors didn't abort???");
}
Ok(x) => x,
}
}

pub trait Callbacks {
/// Called before creating the compiler instance
fn config(&mut self, _config: &mut interface::Config) {}
Expand Down Expand Up @@ -349,27 +339,33 @@ fn run_compiler(
},
};

callbacks.config(&mut config);

default_early_dcx.abort_if_errors();
drop(default_early_dcx);

callbacks.config(&mut config);

interface::run_compiler(config, |compiler| {
let sess = &compiler.sess;
let codegen_backend = &*compiler.codegen_backend;

// This is used for early exits unrelated to errors. E.g. when just
// printing some information without compiling, or exiting immediately
// after parsing, etc.
let early_exit = || {
if let Some(guar) = sess.dcx().has_errors() { Err(guar) } else { Ok(()) }
};

// This implements `-Whelp`. It should be handled very early, like
// `--help`/`-Zhelp`/`-Chelp`. This is the earliest it can run, because
// it must happen after lints are registered, during session creation.
if sess.opts.describe_lints {
describe_lints(sess);
return sess.compile_status();
return early_exit();
}

let early_dcx = EarlyDiagCtxt::new(sess.opts.error_format);

if print_crate_info(&early_dcx, codegen_backend, sess, has_input) == Compilation::Stop {
return sess.compile_status();
return early_exit();
}

if !has_input {
Expand All @@ -378,16 +374,16 @@ fn run_compiler(

if !sess.opts.unstable_opts.ls.is_empty() {
list_metadata(&early_dcx, sess, &*codegen_backend.metadata_loader());
return sess.compile_status();
return early_exit();
}

if sess.opts.unstable_opts.link_only {
process_rlink(sess, compiler);
return sess.compile_status();
return early_exit();
}

let linker = compiler.enter(|queries| {
let early_exit = || sess.compile_status().map(|_| None);
let early_exit = || early_exit().map(|_| None);
queries.parse()?;

if let Some(ppm) = &sess.opts.pretty {
Expand Down Expand Up @@ -659,10 +655,11 @@ fn process_rlink(sess: &Session, compiler: &interface::Compiler) {
};
}
};
let result = compiler.codegen_backend.link(sess, codegen_results, &outputs);
abort_on_err(result, sess);
if compiler.codegen_backend.link(sess, codegen_results, &outputs).is_err() {
FatalError.raise();
}
} else {
dcx.emit_fatal(RlinkNotAFile {})
dcx.emit_fatal(RlinkNotAFile {});
}
}

Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_driver_impl/src/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use rustc_ast as ast;
use rustc_ast_pretty::pprust as pprust_ast;
use rustc_errors::FatalError;
use rustc_hir as hir;
use rustc_hir_pretty as pprust_hir;
use rustc_middle::bug;
Expand All @@ -18,7 +19,6 @@ use std::fmt::Write;

pub use self::PpMode::*;
pub use self::PpSourceMode::*;
use crate::abort_on_err;

struct AstNoAnn;

Expand Down Expand Up @@ -243,7 +243,9 @@ impl<'tcx> PrintExtra<'tcx> {

pub fn print<'tcx>(sess: &Session, ppm: PpMode, ex: PrintExtra<'tcx>) {
if ppm.needs_analysis() {
abort_on_err(ex.tcx().analysis(()), sess);
if ex.tcx().analysis(()).is_err() {
FatalError.raise();
}
}

let (src, src_name) = get_source(sess);
Expand Down Expand Up @@ -334,7 +336,9 @@ pub fn print<'tcx>(sess: &Session, ppm: PpMode, ex: PrintExtra<'tcx>) {
ThirTree => {
let tcx = ex.tcx();
let mut out = String::new();
abort_on_err(rustc_hir_analysis::check_crate(tcx), tcx.sess);
if rustc_hir_analysis::check_crate(tcx).is_err() {
FatalError.raise();
}
debug!("pretty printing THIR tree");
for did in tcx.hir().body_owners() {
let _ = writeln!(out, "{:?}:\n{}\n", did, tcx.thir_tree(did));
Expand All @@ -344,7 +348,9 @@ pub fn print<'tcx>(sess: &Session, ppm: PpMode, ex: PrintExtra<'tcx>) {
ThirFlat => {
let tcx = ex.tcx();
let mut out = String::new();
abort_on_err(rustc_hir_analysis::check_crate(tcx), tcx.sess);
if rustc_hir_analysis::check_crate(tcx).is_err() {
FatalError.raise();
}
debug!("pretty printing THIR flat");
for did in tcx.hir().body_owners() {
let _ = writeln!(out, "{:?}:\n{}\n", did, tcx.thir_flat(did));
Expand Down
83 changes: 51 additions & 32 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,9 +476,10 @@ struct DiagCtxtInner {
emitted_diagnostics: FxHashSet<Hash128>,

/// Stashed diagnostics emitted in one stage of the compiler that may be
/// stolen by other stages (e.g. to improve them and add more information).
/// The stashed diagnostics count towards the total error count.
/// When `.abort_if_errors()` is called, these are also emitted.
/// stolen and emitted/cancelled by other stages (e.g. to improve them and
/// add more information). All stashed diagnostics must be emitted with
/// `emit_stashed_diagnostics` by the time the `DiagCtxtInner` is dropped,
/// otherwise an assertion failure will occur.
stashed_diagnostics: FxIndexMap<(Span, StashKey), Diagnostic>,

future_breakage_diagnostics: Vec<Diagnostic>,
Expand Down Expand Up @@ -563,7 +564,9 @@ pub struct DiagCtxtFlags {

impl Drop for DiagCtxtInner {
fn drop(&mut self) {
self.emit_stashed_diagnostics();
// Any stashed diagnostics should have been handled by
// `emit_stashed_diagnostics` by now.
assert!(self.stashed_diagnostics.is_empty());

if self.err_guars.is_empty() {
self.flush_delayed()
Expand Down Expand Up @@ -755,17 +758,24 @@ impl DiagCtxt {
}

/// Emit all stashed diagnostics.
pub fn emit_stashed_diagnostics(&self) {
pub fn emit_stashed_diagnostics(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow_mut().emit_stashed_diagnostics()
}

/// This excludes lint errors, delayed bugs, and stashed errors.
/// This excludes lint errors, delayed bugs and stashed errors.
#[inline]
pub fn err_count(&self) -> usize {
pub fn err_count_excluding_lint_errs(&self) -> usize {
self.inner.borrow().err_guars.len()
}

/// This excludes normal errors, lint errors and delayed bugs. Unless
/// This excludes delayed bugs and stashed errors.
#[inline]
pub fn err_count(&self) -> usize {
let inner = self.inner.borrow();
inner.err_guars.len() + inner.lint_err_guars.len()
}

/// This excludes normal errors, lint errors, and delayed bugs. Unless
/// absolutely necessary, avoid using this. It's dubious because stashed
/// errors can later be cancelled, so the presence of a stashed error at
/// some point of time doesn't guarantee anything -- there are no
Expand All @@ -774,27 +784,29 @@ impl DiagCtxt {
self.inner.borrow().stashed_err_count
}

/// This excludes lint errors, delayed bugs, and stashed errors.
pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow().has_errors()
/// This excludes lint errors, delayed bugs, and stashed errors. Unless
/// absolutely necessary, prefer `has_errors` to this method.
pub fn has_errors_excluding_lint_errors(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow().has_errors_excluding_lint_errors()
}

/// This excludes delayed bugs and stashed errors. Unless absolutely
/// necessary, prefer `has_errors` to this method.
pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow().has_errors_or_lint_errors()
/// This excludes delayed bugs and stashed errors.
pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow().has_errors()
}

/// This excludes stashed errors. Unless absolutely necessary, prefer
/// `has_errors` or `has_errors_or_lint_errors` to this method.
pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow().has_errors_or_lint_errors_or_delayed_bugs()
/// `has_errors` to this method.
pub fn has_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow().has_errors_or_delayed_bugs()
}

pub fn print_error_count(&self, registry: &Registry) {
let mut inner = self.inner.borrow_mut();

inner.emit_stashed_diagnostics();
// Any stashed diagnostics should have been handled by
// `emit_stashed_diagnostics` by now.
assert!(inner.stashed_diagnostics.is_empty());

if inner.treat_err_as_bug() {
return;
Expand Down Expand Up @@ -869,10 +881,12 @@ impl DiagCtxt {
}
}

/// This excludes delayed bugs and stashed errors. Used for early aborts
/// after errors occurred -- e.g. because continuing in the face of errors is
/// likely to lead to bad results, such as spurious/uninteresting
/// additional errors -- when returning an error `Result` is difficult.
pub fn abort_if_errors(&self) {
let mut inner = self.inner.borrow_mut();
inner.emit_stashed_diagnostics();
if !inner.err_guars.is_empty() {
if self.has_errors().is_some() {
FatalError.raise();
}
}
Expand Down Expand Up @@ -1273,10 +1287,10 @@ impl DiagCtxt {
// `DiagCtxtInner::foo`.
impl DiagCtxtInner {
/// Emit all stashed diagnostics.
fn emit_stashed_diagnostics(&mut self) {
fn emit_stashed_diagnostics(&mut self) -> Option<ErrorGuaranteed> {
let mut guar = None;
let has_errors = !self.err_guars.is_empty();
for (_, diag) in std::mem::take(&mut self.stashed_diagnostics).into_iter() {
// Decrement the count tracking the stash; emitting will increment it.
if diag.is_error() {
if diag.is_lint.is_none() {
self.stashed_err_count -= 1;
Expand All @@ -1289,8 +1303,9 @@ impl DiagCtxtInner {
continue;
}
}
self.emit_diagnostic(diag);
guar = guar.or(self.emit_diagnostic(diag));
}
guar
}

// Return value is only `Some` if the level is `Error` or `DelayedBug`.
Expand Down Expand Up @@ -1334,7 +1349,7 @@ impl DiagCtxtInner {
DelayedBug => {
// If we have already emitted at least one error, we don't need
// to record the delayed bug, because it'll never be used.
return if let Some(guar) = self.has_errors_or_lint_errors() {
return if let Some(guar) = self.has_errors() {
Some(guar)
} else {
let backtrace = std::backtrace::Backtrace::capture();
Expand Down Expand Up @@ -1451,17 +1466,16 @@ impl DiagCtxtInner {
.is_some_and(|c| self.err_guars.len() + self.lint_err_guars.len() + 1 >= c.get())
}

fn has_errors(&self) -> Option<ErrorGuaranteed> {
fn has_errors_excluding_lint_errors(&self) -> Option<ErrorGuaranteed> {
self.err_guars.get(0).copied()
}

fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
self.has_errors().or_else(|| self.lint_err_guars.get(0).copied())
fn has_errors(&self) -> Option<ErrorGuaranteed> {
self.has_errors_excluding_lint_errors().or_else(|| self.lint_err_guars.get(0).copied())
}

fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
self.has_errors_or_lint_errors()
.or_else(|| self.delayed_bugs.get(0).map(|(_, guar)| guar).copied())
fn has_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
self.has_errors().or_else(|| self.delayed_bugs.get(0).map(|(_, guar)| guar).copied())
}

/// Translate `message` eagerly with `args` to `SubdiagnosticMessage::Eager`.
Expand All @@ -1484,6 +1498,11 @@ impl DiagCtxtInner {
}

fn flush_delayed(&mut self) {
// Stashed diagnostics must be emitted before delayed bugs are flushed.
// Otherwise, we might ICE prematurely when errors would have
// eventually happened.
assert!(self.stashed_diagnostics.is_empty());

if self.delayed_bugs.is_empty() {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/persist/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ pub fn finalize_session_directory(sess: &Session, svh: Option<Svh>) {

let incr_comp_session_dir: PathBuf = sess.incr_comp_session_dir().clone();

if sess.dcx().has_errors_or_lint_errors_or_delayed_bugs().is_some() {
if sess.dcx().has_errors_or_delayed_bugs().is_some() {
// If there have been any errors during compilation, we don't want to
// publish this session directory. Rather, we'll just delete it.

Expand Down
Loading

0 comments on commit 01c13f8

Please sign in to comment.