From 8d1c20a539b72abdbe1ede872f19e0a1a6f34be4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 6 Feb 2024 17:13:32 +1100 Subject: [PATCH 1/6] Remove return value from `emit_stashed_diagnostics`. It's never used. --- compiler/rustc_errors/src/lib.rs | 9 +++------ compiler/rustc_session/src/session.rs | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index a4112d717d029..7c62e3aa42281 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -708,7 +708,7 @@ impl DiagCtxt { } /// Emit all stashed diagnostics. - pub fn emit_stashed_diagnostics(&self) -> Option { + pub fn emit_stashed_diagnostics(&self) { self.inner.borrow_mut().emit_stashed_diagnostics() } @@ -1216,9 +1216,8 @@ impl DiagCtxt { // `DiagCtxtInner::foo`. impl DiagCtxtInner { /// Emit all stashed diagnostics. - fn emit_stashed_diagnostics(&mut self) -> Option { + fn emit_stashed_diagnostics(&mut self) { let has_errors = self.has_errors(); - let mut reported = None; 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() { @@ -1235,10 +1234,8 @@ impl DiagCtxtInner { continue; } } - let reported_this = self.emit_diagnostic(diag); - reported = reported.or(reported_this); + self.emit_diagnostic(diag); } - reported } fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index f6af5a4f87e07..ec14385f40c62 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -315,7 +315,7 @@ impl Session { pub fn compile_status(&self) -> Result<(), ErrorGuaranteed> { // We must include lint errors here. if let Some(reported) = self.dcx().has_errors_or_lint_errors() { - let _ = self.dcx().emit_stashed_diagnostics(); + self.dcx().emit_stashed_diagnostics(); Err(reported) } else { Ok(()) From e55df623ead33023fe6c4488064e5d5e4e141b9e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 6 Feb 2024 17:40:11 +1100 Subject: [PATCH 2/6] Remove an `unchecked_claim_error_was_emitted` call. When `catch_fatal_errors` catches a `FatalErrorMarker`, it returns an `ErrorGuaranteed` that is conjured out of thin air with `unchecked_claim_error_was_emitted`. But that `ErrorGuaranteed` is never used. This commit changes it to instead conjure a `FatalError` out of thin air. (A non-deprecated action!) This makes more sense because `FatalError` and `FatalErrorMarker` are a natural pairing -- a `FatalErrorMarker` is created by calling `FatalError::raise`, so this is effectively getting back the original `FatalError`. This requires a tiny change in `catch_with_exit_code`. The old result of the `catch_fatal_errors` call there was `Result, ErrorGuaranteed>` which could be `flatten`ed into `Result<(), ErrorGuaranteed>`. The new result of the `catch_fatal_errors` calls is `Result, FatalError>`, which can't be `flatten`ed but is still easily matched for the success case. --- compiler/rustc_driver_impl/src/lib.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 5903c43ae98af..519bde988200e 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -24,7 +24,9 @@ use rustc_data_structures::profiling::{ get_resident_set_size, print_time_passes_entry, TimePassesFormat, }; use rustc_errors::registry::Registry; -use rustc_errors::{markdown, ColorConfig, DiagCtxt, ErrCode, ErrorGuaranteed, PResult}; +use rustc_errors::{ + markdown, ColorConfig, DiagCtxt, ErrCode, ErrorGuaranteed, FatalError, PResult, +}; use rustc_feature::find_gated_cfg; use rustc_interface::util::{self, collect_crate_types, get_codegen_backend}; use rustc_interface::{interface, Queries}; @@ -1231,11 +1233,10 @@ fn parse_crate_attrs<'a>(sess: &'a Session) -> PResult<'a, ast::AttrVec> { /// The compiler currently unwinds with a special sentinel value to abort /// compilation on fatal errors. This function catches that sentinel and turns /// the panic into a `Result` instead. -pub fn catch_fatal_errors R, R>(f: F) -> Result { +pub fn catch_fatal_errors R, R>(f: F) -> Result { catch_unwind(panic::AssertUnwindSafe(f)).map_err(|value| { if value.is::() { - #[allow(deprecated)] - ErrorGuaranteed::unchecked_claim_error_was_emitted() + FatalError } else { panic::resume_unwind(value); } @@ -1245,9 +1246,9 @@ pub fn catch_fatal_errors R, R>(f: F) -> Result interface::Result<()>) -> i32 { - match catch_fatal_errors(f).flatten() { - Ok(()) => EXIT_SUCCESS, - Err(_) => EXIT_FAILURE, + match catch_fatal_errors(f) { + Ok(Ok(())) => EXIT_SUCCESS, + _ => EXIT_FAILURE, } } From e6794ddfb00e14b29b3befc5bc054a15094321b7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 6 Feb 2024 20:47:47 +1100 Subject: [PATCH 3/6] rustdoc: make `main` more like rustc's. By making non-unicode arguments a fatal error instead of a warning, we don't need to handle what comes after, which avoids the need for an `unchecked_claim_error_was_emitted` call. --- src/librustdoc/lib.rs | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 8c10f14116a0c..0fe3adadba347 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -177,13 +177,16 @@ pub fn main() { init_logging(&early_dcx); rustc_driver::init_logger(&early_dcx, rustc_log::LoggerConfig::from_env("RUSTDOC_LOG")); - let exit_code = rustc_driver::catch_with_exit_code(|| match get_args(&early_dcx) { - Some(args) => main_args(&mut early_dcx, &args, using_internal_features), - _ => - { - #[allow(deprecated)] - Err(ErrorGuaranteed::unchecked_claim_error_was_emitted()) - } + let exit_code = rustc_driver::catch_with_exit_code(|| { + let args = env::args_os() + .enumerate() + .map(|(i, arg)| { + arg.into_string().unwrap_or_else(|arg| { + early_dcx.early_fatal(format!("argument {i} is not valid Unicode: {arg:?}")) + }) + }) + .collect::>(); + main_args(&mut early_dcx, &args, using_internal_features) }); process::exit(exit_code); } @@ -219,19 +222,6 @@ fn init_logging(early_dcx: &EarlyDiagCtxt) { tracing::subscriber::set_global_default(subscriber).unwrap(); } -fn get_args(early_dcx: &EarlyDiagCtxt) -> Option> { - env::args_os() - .enumerate() - .map(|(i, arg)| { - arg.into_string() - .map_err(|arg| { - early_dcx.early_warn(format!("Argument {i} is not valid Unicode: {arg:?}")); - }) - .ok() - }) - .collect() -} - fn opts() -> Vec { let stable: fn(_, fn(&mut getopts::Options) -> &mut _) -> _ = RustcOptGroup::stable; let unstable: fn(_, fn(&mut getopts::Options) -> &mut _) -> _ = RustcOptGroup::unstable; From 83adf883a20fc7531e886996ce07b8555c943fba Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 Feb 2024 09:01:49 +1100 Subject: [PATCH 4/6] rustdoc: remove `unchecked_claim_error_was_emitted` call in `main_args`. `main_args` calls `from_matches`, which does lots of initialization. If anything goes wrong, `from_matches` emits an error message and returns `Err(1)` (or `Err(3)`). `main_args` then turns the `Err(1)` into `Err(ErrorGuaranteed)`, because that's what `catch_with_exit_code` requires on error. But `catch_with_exit_code` doesn't do anything with the `ErrorGuaranteed`, it just exits with `EXIT_FAILURE`. We can avoid the creation of the `ErrorGuaranteed` (which requires an undesirable `unchecked_claim_error_was_emitted` call), by changing `from_matches` to instead eagerly abort if anything goes wrong. The behaviour from the user's point of view is the same: an early abort with an `EXIT_FAILURE` exit code. And we can also simplify `from_matches` to return an `Option` instead of a `Result`: - Old `Err(0)` case --> `None` - Old `Err(_)` case --> fatal error. This requires similar changes to `ScrapeExamplesOptions::new` and `load_call_locations`. --- src/librustdoc/config.rs | 96 +++++++++++-------------------- src/librustdoc/lib.rs | 11 +--- src/librustdoc/scrape_examples.rs | 49 +++++++--------- 3 files changed, 54 insertions(+), 102 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 3f7f9270b2da0..c46047aa0dbad 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -323,20 +323,20 @@ impl Options { early_dcx: &mut EarlyDiagCtxt, matches: &getopts::Matches, args: Vec, - ) -> Result<(Options, RenderOptions), i32> { + ) -> Option<(Options, RenderOptions)> { // Check for unstable options. nightly_options::check_nightly_options(early_dcx, matches, &opts()); if args.is_empty() || matches.opt_present("h") || matches.opt_present("help") { crate::usage("rustdoc"); - return Err(0); + return None; } else if matches.opt_present("version") { rustc_driver::version!(&early_dcx, "rustdoc", matches); - return Err(0); + return None; } if rustc_driver::describe_flag_categories(early_dcx, &matches) { - return Err(0); + return None; } let color = config::parse_color(early_dcx, matches); @@ -382,7 +382,7 @@ impl Options { } } - return Err(0); + return None; } let mut emit = Vec::new(); @@ -390,10 +390,7 @@ impl Options { for kind in list.split(',') { match kind.parse() { Ok(kind) => emit.push(kind), - Err(()) => { - dcx.err(format!("unrecognized emission type: {kind}")); - return Err(1); - } + Err(()) => dcx.fatal(format!("unrecognized emission type: {kind}")), } } } @@ -403,7 +400,7 @@ impl Options { && !matches.opt_present("show-coverage") && !nightly_options::is_unstable_enabled(matches) { - early_dcx.early_fatal( + dcx.fatal( "the -Z unstable-options flag must be passed to enable --output-format for documentation generation (see https://github.com/rust-lang/rust/issues/76578)", ); } @@ -420,10 +417,7 @@ impl Options { } let paths = match theme::load_css_paths(content) { Ok(p) => p, - Err(e) => { - dcx.err(e); - return Err(1); - } + Err(e) => dcx.fatal(e), }; let mut errors = 0; @@ -442,9 +436,9 @@ impl Options { } } if errors != 0 { - return Err(1); + dcx.fatal("[check-theme] one or more tests failed"); } - return Err(0); + return None; } let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(early_dcx, matches); @@ -452,11 +446,9 @@ impl Options { let input = PathBuf::from(if describe_lints { "" // dummy, this won't be used } else if matches.free.is_empty() { - dcx.err("missing file operand"); - return Err(1); + dcx.fatal("missing file operand"); } else if matches.free.len() > 1 { - dcx.err("too many file operands"); - return Err(1); + dcx.fatal("too many file operands"); } else { &matches.free[0] }); @@ -466,10 +458,7 @@ impl Options { let externs = parse_externs(early_dcx, matches, &unstable_opts); let extern_html_root_urls = match parse_extern_html_roots(matches) { Ok(ex) => ex, - Err(err) => { - dcx.err(err); - return Err(1); - } + Err(err) => dcx.fatal(err), }; let default_settings: Vec> = vec![ @@ -526,16 +515,14 @@ impl Options { let no_run = matches.opt_present("no-run"); if !should_test && no_run { - dcx.err("the `--test` flag must be passed to enable `--no-run`"); - return Err(1); + dcx.fatal("the `--test` flag must be passed to enable `--no-run`"); } let out_dir = matches.opt_str("out-dir").map(|s| PathBuf::from(&s)); let output = matches.opt_str("output").map(|s| PathBuf::from(&s)); let output = match (out_dir, output) { (Some(_), Some(_)) => { - dcx.err("cannot use both 'out-dir' and 'output' at once"); - return Err(1); + dcx.fatal("cannot use both 'out-dir' and 'output' at once"); } (Some(out_dir), None) => out_dir, (None, Some(output)) => output, @@ -549,8 +536,7 @@ impl Options { if let Some(ref p) = extension_css { if !p.is_file() { - dcx.err("option --extend-css argument must be a file"); - return Err(1); + dcx.fatal("option --extend-css argument must be a file"); } } @@ -566,31 +552,25 @@ impl Options { } let paths = match theme::load_css_paths(content) { Ok(p) => p, - Err(e) => { - dcx.err(e); - return Err(1); - } + Err(e) => dcx.fatal(e), }; for (theme_file, theme_s) in matches.opt_strs("theme").iter().map(|s| (PathBuf::from(&s), s.to_owned())) { if !theme_file.is_file() { - dcx.struct_err(format!("invalid argument: \"{theme_s}\"")) + dcx.struct_fatal(format!("invalid argument: \"{theme_s}\"")) .with_help("arguments to --theme must be files") .emit(); - return Err(1); } if theme_file.extension() != Some(OsStr::new("css")) { - dcx.struct_err(format!("invalid argument: \"{theme_s}\"")) + dcx.struct_fatal(format!("invalid argument: \"{theme_s}\"")) .with_help("arguments to --theme must have a .css extension") .emit(); - return Err(1); } let (success, ret) = theme::test_theme_against(&theme_file, &paths, &dcx); if !success { - dcx.err(format!("error loading theme file: \"{theme_s}\"")); - return Err(1); + dcx.fatal(format!("error loading theme file: \"{theme_s}\"")); } else if !ret.is_empty() { dcx.struct_warn(format!( "theme file \"{theme_s}\" is missing CSS rules from the default theme", @@ -620,22 +600,18 @@ impl Options { edition, &None, ) else { - return Err(3); + dcx.fatal("`ExternalHtml::load` failed"); }; match matches.opt_str("r").as_deref() { Some("rust") | None => {} - Some(s) => { - dcx.err(format!("unknown input format: {s}")); - return Err(1); - } + Some(s) => dcx.fatal(format!("unknown input format: {s}")), } let index_page = matches.opt_str("index-page").map(|s| PathBuf::from(&s)); if let Some(ref index_page) = index_page { if !index_page.is_file() { - dcx.err("option `--index-page` argument must be a file"); - return Err(1); + dcx.fatal("option `--index-page` argument must be a file"); } } @@ -646,8 +622,7 @@ impl Options { let crate_types = match parse_crate_types_from_list(matches.opt_strs("crate-type")) { Ok(types) => types, Err(e) => { - dcx.err(format!("unknown crate type: {e}")); - return Err(1); + dcx.fatal(format!("unknown crate type: {e}")); } }; @@ -655,18 +630,13 @@ impl Options { Some(s) => match OutputFormat::try_from(s.as_str()) { Ok(out_fmt) => { if !out_fmt.is_json() && show_coverage { - dcx.struct_err( + dcx.fatal( "html output format isn't supported for the --show-coverage option", - ) - .emit(); - return Err(1); + ); } out_fmt } - Err(e) => { - dcx.err(e); - return Err(1); - } + Err(e) => dcx.fatal(e), }, None => OutputFormat::default(), }; @@ -709,16 +679,14 @@ impl Options { let html_no_source = matches.opt_present("html-no-source"); if generate_link_to_definition && (show_coverage || output_format != OutputFormat::Html) { - dcx.struct_err( + dcx.fatal( "--generate-link-to-definition option can only be used with HTML output format", - ) - .emit(); - return Err(1); + ); } - let scrape_examples_options = ScrapeExamplesOptions::new(matches, &dcx)?; + let scrape_examples_options = ScrapeExamplesOptions::new(matches, &dcx); let with_examples = matches.opt_strs("with-examples"); - let call_locations = crate::scrape_examples::load_call_locations(with_examples, &dcx)?; + let call_locations = crate::scrape_examples::load_call_locations(with_examples, &dcx); let unstable_features = rustc_feature::UnstableFeatures::from_environment(crate_name.as_deref()); @@ -793,7 +761,7 @@ impl Options { no_emit_shared: false, html_no_source, }; - Ok((options, render_options)) + Some((options, render_options)) } /// Returns `true` if the file given as `self.input` is a Markdown file. diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 0fe3adadba347..097bbeb6d2857 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -720,15 +720,8 @@ fn main_args( // Note that we discard any distinction between different non-zero exit // codes from `from_matches` here. let (options, render_options) = match config::Options::from_matches(early_dcx, &matches, args) { - Ok(opts) => opts, - Err(code) => { - return if code == 0 { - Ok(()) - } else { - #[allow(deprecated)] - Err(ErrorGuaranteed::unchecked_claim_error_was_emitted()) - }; - } + Some(opts) => opts, + None => return Ok(()), }; let diag = diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index b7d9c16f34837..9c9b386edda7c 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -38,28 +38,23 @@ pub(crate) struct ScrapeExamplesOptions { } impl ScrapeExamplesOptions { - pub(crate) fn new( - matches: &getopts::Matches, - dcx: &rustc_errors::DiagCtxt, - ) -> Result, i32> { + pub(crate) fn new(matches: &getopts::Matches, dcx: &rustc_errors::DiagCtxt) -> Option { let output_path = matches.opt_str("scrape-examples-output-path"); let target_crates = matches.opt_strs("scrape-examples-target-crate"); let scrape_tests = matches.opt_present("scrape-tests"); match (output_path, !target_crates.is_empty(), scrape_tests) { - (Some(output_path), true, _) => Ok(Some(ScrapeExamplesOptions { + (Some(output_path), true, _) => Some(ScrapeExamplesOptions { output_path: PathBuf::from(output_path), target_crates, scrape_tests, - })), + }), (Some(_), false, _) | (None, true, _) => { - dcx.err("must use --scrape-examples-output-path and --scrape-examples-target-crate together"); - Err(1) + dcx.fatal("must use --scrape-examples-output-path and --scrape-examples-target-crate together"); } (None, false, true) => { - dcx.err("must use --scrape-examples-output-path and --scrape-examples-target-crate with --scrape-tests"); - Err(1) + dcx.fatal("must use --scrape-examples-output-path and --scrape-examples-target-crate with --scrape-tests"); } - (None, false, false) => Ok(None), + (None, false, false) => None, } } } @@ -342,24 +337,20 @@ pub(crate) fn run( pub(crate) fn load_call_locations( with_examples: Vec, dcx: &rustc_errors::DiagCtxt, -) -> Result { - let inner = || { - let mut all_calls: AllCallLocations = FxHashMap::default(); - for path in with_examples { - let bytes = fs::read(&path).map_err(|e| format!("{e} (for path {path})"))?; - let mut decoder = MemDecoder::new(&bytes, 0); - let calls = AllCallLocations::decode(&mut decoder); - - for (function, fn_calls) in calls.into_iter() { - all_calls.entry(function).or_default().extend(fn_calls.into_iter()); - } - } +) -> AllCallLocations { + let mut all_calls: AllCallLocations = FxHashMap::default(); + for path in with_examples { + let bytes = match fs::read(&path) { + Ok(bytes) => bytes, + Err(e) => dcx.fatal(format!("failed to load examples: {e}")), + }; + let mut decoder = MemDecoder::new(&bytes, 0); + let calls = AllCallLocations::decode(&mut decoder); - Ok(all_calls) - }; + for (function, fn_calls) in calls.into_iter() { + all_calls.entry(function).or_default().extend(fn_calls.into_iter()); + } + } - inner().map_err(|e: String| { - dcx.err(format!("failed to load examples: {e}")); - 1 - }) + all_calls } From 97c157fe1e52eeabe14a64865b4731794a23393a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 Feb 2024 10:26:50 +1100 Subject: [PATCH 5/6] Tighten up `ErrorGuaranteed` handling. - In `emit_producing_error_guaranteed`, only allow `Level::Error`. - In `emit_diagnostic`, only produce `ErrorGuaranteed` for `Level` and `DelayedBug`. (Not `Bug` or `Fatal`. They don't need it, because the relevant `emit` methods abort.) - Add/update various comments. --- compiler/rustc_errors/src/diagnostic_builder.rs | 16 ++++++++++------ compiler/rustc_errors/src/lib.rs | 14 ++++++++++---- compiler/rustc_span/src/lib.rs | 5 ++--- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index faff7f0b52673..e484bef0e0bc9 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -99,16 +99,20 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> { } /// `ErrorGuaranteed::emit_producing_guarantee` uses this. - // FIXME(eddyb) make `ErrorGuaranteed` impossible to create outside `.emit()`. fn emit_producing_error_guaranteed(mut self) -> ErrorGuaranteed { let diag = self.take_diag(); - // Only allow a guarantee if the `level` wasn't switched to a - // non-error. The field isn't `pub`, but the whole `Diagnostic` can be - // overwritten with a new one, thanks to `DerefMut`. + // The only error levels that produce `ErrorGuaranteed` are + // `Error` and `DelayedBug`. But `DelayedBug` should never occur here + // because delayed bugs have their level changed to `Bug` when they are + // actually printed, so they produce an ICE. + // + // (Also, even though `level` isn't `pub`, the whole `Diagnostic` could + // be overwritten with a new one thanks to `DerefMut`. So this assert + // protects against that, too.) assert!( - diag.is_error(), - "emitted non-error ({:?}) diagnostic from `DiagnosticBuilder`", + matches!(diag.level, Level::Error | Level::DelayedBug), + "invalid diagnostic level ({:?})", diag.level, ); diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 7c62e3aa42281..74cccda6cbff6 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -931,6 +931,7 @@ impl DiagCtxt { /// This excludes lint errors and delayed bugs. pub fn has_errors(&self) -> Option { self.inner.borrow().has_errors().then(|| { + // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() }) @@ -942,6 +943,7 @@ impl DiagCtxt { let inner = self.inner.borrow(); let result = inner.has_errors() || inner.lint_err_count > 0; result.then(|| { + // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() }) @@ -954,6 +956,7 @@ impl DiagCtxt { let result = inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty(); result.then(|| { + // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() }) @@ -1238,6 +1241,7 @@ impl DiagCtxtInner { } } + // Return value is only `Some` if the level is `Error` or `DelayedBug`. fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { assert!(diagnostic.level.can_be_top_or_sub().0); @@ -1316,6 +1320,7 @@ impl DiagCtxtInner { !self.emitted_diagnostics.insert(diagnostic_hash) }; + let level = diagnostic.level; let is_error = diagnostic.is_error(); let is_lint = diagnostic.is_lint.is_some(); @@ -1352,6 +1357,7 @@ impl DiagCtxtInner { self.emitter.emit_diagnostic(diagnostic); } + if is_error { if is_lint { self.lint_err_count += 1; @@ -1359,11 +1365,11 @@ impl DiagCtxtInner { self.err_count += 1; } self.panic_if_treat_err_as_bug(); + } - #[allow(deprecated)] - { - guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); - } + #[allow(deprecated)] + if level == Level::Error { + guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); } }); diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index ea6766ea583be..1a1e1de6734ac 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -2477,9 +2477,8 @@ where pub struct ErrorGuaranteed(()); impl ErrorGuaranteed { - /// To be used only if you really know what you are doing... ideally, we would find a way to - /// eliminate all calls to this method. - #[deprecated = "`Session::span_delayed_bug` should be preferred over this function"] + /// Don't use this outside of `DiagCtxtInner::emit_diagnostic`! + #[deprecated = "should only be used in `DiagCtxtInner::emit_diagnostic`"] pub fn unchecked_claim_error_was_emitted() -> Self { ErrorGuaranteed(()) } From 6889fe3806fa44addca89193e5c45ced8c7f532d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 Feb 2024 19:30:59 +1100 Subject: [PATCH 6/6] Rename `unchecked_claim_error_was_emitted` as `unchecked_error_guaranteed`. It's more to-the-point. --- compiler/rustc_errors/src/lib.rs | 10 +++++----- compiler/rustc_span/src/lib.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 74cccda6cbff6..ec5029e505f96 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -933,7 +933,7 @@ impl DiagCtxt { self.inner.borrow().has_errors().then(|| { // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] - ErrorGuaranteed::unchecked_claim_error_was_emitted() + ErrorGuaranteed::unchecked_error_guaranteed() }) } @@ -945,7 +945,7 @@ impl DiagCtxt { result.then(|| { // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] - ErrorGuaranteed::unchecked_claim_error_was_emitted() + ErrorGuaranteed::unchecked_error_guaranteed() }) } @@ -958,7 +958,7 @@ impl DiagCtxt { result.then(|| { // FIXME(nnethercote) find a way to store an `ErrorGuaranteed`. #[allow(deprecated)] - ErrorGuaranteed::unchecked_claim_error_was_emitted() + ErrorGuaranteed::unchecked_error_guaranteed() }) } @@ -1286,7 +1286,7 @@ impl DiagCtxtInner { let backtrace = std::backtrace::Backtrace::capture(); self.delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); #[allow(deprecated)] - return Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); + return Some(ErrorGuaranteed::unchecked_error_guaranteed()); } GoodPathDelayedBug => { let backtrace = std::backtrace::Backtrace::capture(); @@ -1369,7 +1369,7 @@ impl DiagCtxtInner { #[allow(deprecated)] if level == Level::Error { - guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); + guaranteed = Some(ErrorGuaranteed::unchecked_error_guaranteed()); } }); diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 1a1e1de6734ac..bcf04a71ae223 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -2479,7 +2479,7 @@ pub struct ErrorGuaranteed(()); impl ErrorGuaranteed { /// Don't use this outside of `DiagCtxtInner::emit_diagnostic`! #[deprecated = "should only be used in `DiagCtxtInner::emit_diagnostic`"] - pub fn unchecked_claim_error_was_emitted() -> Self { + pub fn unchecked_error_guaranteed() -> Self { ErrorGuaranteed(()) } }