From 83adf883a20fc7531e886996ce07b8555c943fba Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 Feb 2024 09:01:49 +1100 Subject: [PATCH] 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 | 47 ++++++--------- 3 files changed, 53 insertions(+), 101 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 3f7f9270b2d..c46047aa0db 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 0fe3adadba3..097bbeb6d28 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 b7d9c16f348..9c9b386edda 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); +) -> 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 }