Rollup merge of #120735 - nnethercote:rm-some-unchecked_claims, r=oli-obk

Remove some `unchecked_claim_error_was_emitted` calls

We want to drive the number of these calls down as much as possible. This PR gets rid of a bunch of them.

r? ``@oli-obk``
This commit is contained in:
Guillaume Boisseau 2024-02-07 18:24:46 +01:00 committed by GitHub
commit cb040f5ded
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 102 additions and 153 deletions

View file

@ -24,7 +24,9 @@
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<F: FnOnce() -> R, R>(f: F) -> Result<R, ErrorGuaranteed> {
pub fn catch_fatal_errors<F: FnOnce() -> R, R>(f: F) -> Result<R, FatalError> {
catch_unwind(panic::AssertUnwindSafe(f)).map_err(|value| {
if value.is::<rustc_errors::FatalErrorMarker>() {
#[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted()
FatalError
} else {
panic::resume_unwind(value);
}
@ -1245,9 +1246,9 @@ pub fn catch_fatal_errors<F: FnOnce() -> R, R>(f: F) -> Result<R, ErrorGuarantee
/// 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 {
match catch_fatal_errors(f).flatten() {
Ok(()) => EXIT_SUCCESS,
Err(_) => EXIT_FAILURE,
match catch_fatal_errors(f) {
Ok(Ok(())) => EXIT_SUCCESS,
_ => EXIT_FAILURE,
}
}

View file

@ -99,16 +99,20 @@ fn emit_producing_nothing(mut self) {
}
/// `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<ErrorGuaranteed>`",
matches!(diag.level, Level::Error | Level::DelayedBug),
"invalid diagnostic level ({:?})",
diag.level,
);

View file

@ -708,7 +708,7 @@ pub fn has_stashed_diagnostic(&self, span: Span, key: StashKey) -> bool {
}
/// Emit all stashed diagnostics.
pub fn emit_stashed_diagnostics(&self) -> Option<ErrorGuaranteed> {
pub fn emit_stashed_diagnostics(&self) {
self.inner.borrow_mut().emit_stashed_diagnostics()
}
@ -931,8 +931,9 @@ pub fn err_count(&self) -> usize {
/// This excludes lint errors and delayed bugs.
pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
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()
})
}
@ -942,8 +943,9 @@ pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
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()
ErrorGuaranteed::unchecked_error_guaranteed()
})
}
@ -954,8 +956,9 @@ pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuarantee
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()
ErrorGuaranteed::unchecked_error_guaranteed()
})
}
@ -1216,9 +1219,8 @@ pub fn flush_delayed(&self) {
// `DiagCtxtInner::foo`.
impl DiagCtxtInner {
/// Emit all stashed diagnostics.
fn emit_stashed_diagnostics(&mut self) -> Option<ErrorGuaranteed> {
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,12 +1237,11 @@ fn emit_stashed_diagnostics(&mut self) -> Option<ErrorGuaranteed> {
continue;
}
}
let reported_this = self.emit_diagnostic(diag);
reported = reported.or(reported_this);
self.emit_diagnostic(diag);
}
reported
}
// Return value is only `Some` if the level is `Error` or `DelayedBug`.
fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
assert!(diagnostic.level.can_be_top_or_sub().0);
@ -1285,7 +1286,7 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuarant
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();
@ -1319,6 +1320,7 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuarant
!self.emitted_diagnostics.insert(diagnostic_hash)
};
let level = diagnostic.level;
let is_error = diagnostic.is_error();
let is_lint = diagnostic.is_lint.is_some();
@ -1355,6 +1357,7 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuarant
self.emitter.emit_diagnostic(diagnostic);
}
if is_error {
if is_lint {
self.lint_err_count += 1;
@ -1362,11 +1365,11 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuarant
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_error_guaranteed());
}
});

View file

@ -315,7 +315,7 @@ pub fn create_feature_err<'a>(
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(())

View file

@ -2477,10 +2477,9 @@ fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
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"]
pub fn unchecked_claim_error_was_emitted() -> Self {
/// Don't use this outside of `DiagCtxtInner::emit_diagnostic`!
#[deprecated = "should only be used in `DiagCtxtInner::emit_diagnostic`"]
pub fn unchecked_error_guaranteed() -> Self {
ErrorGuaranteed(())
}
}

View file

@ -323,20 +323,20 @@ pub(crate) fn from_matches(
early_dcx: &mut EarlyDiagCtxt,
matches: &getopts::Matches,
args: Vec<String>,
) -> 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 @@ fn println_condition(condition: Condition) {
}
}
return Err(0);
return None;
}
let mut emit = Vec::new();
@ -390,10 +390,7 @@ fn println_condition(condition: Condition) {
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 @@ fn println_condition(condition: Condition) {
&& !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 @@ fn println_condition(condition: Condition) {
}
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 @@ fn println_condition(condition: Condition) {
}
}
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 @@ fn println_condition(condition: Condition) {
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 @@ fn println_condition(condition: Condition) {
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<(String, String)>> = vec![
@ -526,16 +515,14 @@ fn println_condition(condition: Condition) {
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 @@ fn println_condition(condition: Condition) {
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 @@ fn println_condition(condition: Condition) {
}
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 @@ fn println_condition(condition: Condition) {
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 @@ fn println_condition(condition: Condition) {
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 @@ fn println_condition(condition: Condition) {
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 @@ fn println_condition(condition: Condition) {
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 @@ fn println_condition(condition: Condition) {
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.

View file

@ -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::<Vec<_>>();
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<Vec<String>> {
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<RustcOptGroup> {
let stable: fn(_, fn(&mut getopts::Options) -> &mut _) -> _ = RustcOptGroup::stable;
let unstable: fn(_, fn(&mut getopts::Options) -> &mut _) -> _ = RustcOptGroup::unstable;
@ -730,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 =

View file

@ -38,28 +38,23 @@ pub(crate) struct ScrapeExamplesOptions {
}
impl ScrapeExamplesOptions {
pub(crate) fn new(
matches: &getopts::Matches,
dcx: &rustc_errors::DiagCtxt,
) -> Result<Option<Self>, i32> {
pub(crate) fn new(matches: &getopts::Matches, dcx: &rustc_errors::DiagCtxt) -> Option<Self> {
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<String>,
dcx: &rustc_errors::DiagCtxt,
) -> Result<AllCallLocations, i32> {
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);
) -> 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);
for (function, fn_calls) in calls.into_iter() {
all_calls.entry(function).or_default().extend(fn_calls.into_iter());
}
for (function, fn_calls) in calls.into_iter() {
all_calls.entry(function).or_default().extend(fn_calls.into_iter());
}
}
Ok(all_calls)
};
inner().map_err(|e: String| {
dcx.err(format!("failed to load examples: {e}"));
1
})
all_calls
}