From 706eb1604b45e922858e04cf6abee7e58aa92730 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 11:01:26 +1100 Subject: [PATCH 1/5] Rename `early_error_handler` as `default_handler`. Yes, its type is `EarlyErrorHandler`, but there is another value of that type later on in the function called `handler` that is initialized with `sopts.error_format`. So `default_handler` is a better name because it clarifies that it is initialized with `ErrorOutputType::default()`. --- compiler/rustc_driver_impl/src/lib.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index f38a8945c7ff1..c24d74d1b9bc7 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -293,7 +293,7 @@ fn run_compiler( >, using_internal_features: Arc, ) -> interface::Result<()> { - let mut early_error_handler = EarlyErrorHandler::new(ErrorOutputType::default()); + let mut default_handler = EarlyErrorHandler::new(ErrorOutputType::default()); // Throw away the first argument, the name of the binary. // In case of at_args being empty, as might be the case by @@ -305,14 +305,14 @@ fn run_compiler( // the compiler with @empty_file as argv[0] and no more arguments. let at_args = at_args.get(1..).unwrap_or_default(); - let args = args::arg_expand_all(&early_error_handler, at_args); + let args = args::arg_expand_all(&default_handler, at_args); - let Some(matches) = handle_options(&early_error_handler, &args) else { return Ok(()) }; + let Some(matches) = handle_options(&default_handler, &args) else { return Ok(()) }; - let sopts = config::build_session_options(&mut early_error_handler, &matches); + let sopts = config::build_session_options(&mut default_handler, &matches); if let Some(ref code) = matches.opt_str("explain") { - handle_explain(&early_error_handler, diagnostics_registry(), code, sopts.color); + handle_explain(&default_handler, diagnostics_registry(), code, sopts.color); return Ok(()); } @@ -338,7 +338,7 @@ fn run_compiler( expanded_args: args, }; - match make_input(&early_error_handler, &matches.free) { + match make_input(&default_handler, &matches.free) { Err(reported) => return Err(reported), Ok(Some(input)) => { config.input = input; @@ -349,7 +349,7 @@ fn run_compiler( 0 => { callbacks.config(&mut config); - early_error_handler.abort_if_errors(); + default_handler.abort_if_errors(); interface::run_compiler(config, |compiler| { let sopts = &compiler.session().opts; @@ -374,14 +374,14 @@ fn run_compiler( return Ok(()); } 1 => panic!("make_input should have provided valid inputs"), - _ => early_error_handler.early_error(format!( + _ => default_handler.early_error(format!( "multiple input filenames provided (first two filenames are `{}` and `{}`)", matches.free[0], matches.free[1], )), }, }; - early_error_handler.abort_if_errors(); + default_handler.abort_if_errors(); interface::run_compiler(config, |compiler| { let sess = compiler.session(); From 8aee35e2ed51a9f782c1bff3f8a165b5a85f0c43 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 11:04:38 +1100 Subject: [PATCH 2/5] Merge `interface::run_compiler` calls. `rustc_driver_impl::run_compiler` currently has two `interface::run_compiler` calls: one for the "no input" case, and one for the normal case. This commit merges the former into the latter, which makes the control flow easier to read and avoids some duplication. It also makes it clearer that the "no input" case will describe lints before printing crate info, while the normal case does it in the reverse order. Possibly a bug? --- compiler/rustc_driver_impl/src/lib.rs | 48 ++++++++++----------------- 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index c24d74d1b9bc7..f74eb0283649e 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -338,41 +338,14 @@ fn run_compiler( expanded_args: args, }; - match make_input(&default_handler, &matches.free) { + let has_input = match make_input(&default_handler, &matches.free) { Err(reported) => return Err(reported), Ok(Some(input)) => { config.input = input; - - callbacks.config(&mut config); + true // has input: normal compilation } Ok(None) => match matches.free.len() { - 0 => { - callbacks.config(&mut config); - - default_handler.abort_if_errors(); - - interface::run_compiler(config, |compiler| { - let sopts = &compiler.session().opts; - let handler = EarlyErrorHandler::new(sopts.error_format); - - if sopts.describe_lints { - describe_lints(compiler.session()); - return; - } - let should_stop = print_crate_info( - &handler, - compiler.codegen_backend(), - compiler.session(), - false, - ); - - if should_stop == Compilation::Stop { - return; - } - handler.early_error("no input filename given") - }); - return Ok(()); - } + 0 => false, // no input: we will exit early 1 => panic!("make_input should have provided valid inputs"), _ => default_handler.early_error(format!( "multiple input filenames provided (first two filenames are `{}` and `{}`)", @@ -381,13 +354,28 @@ fn run_compiler( }, }; + callbacks.config(&mut config); + default_handler.abort_if_errors(); + drop(default_handler); interface::run_compiler(config, |compiler| { let sess = compiler.session(); let codegen_backend = compiler.codegen_backend(); let handler = EarlyErrorHandler::new(sess.opts.error_format); + if !has_input { + if sess.opts.describe_lints { + describe_lints(sess); + return sess.compile_status(); + } + let should_stop = print_crate_info(&handler, codegen_backend, sess, false); + if should_stop == Compilation::Continue { + handler.early_error("no input filename given") + } + return sess.compile_status(); + } + let should_stop = print_crate_info(&handler, codegen_backend, sess, true) .and_then(|| list_metadata(&handler, sess, &*codegen_backend.metadata_loader())) .and_then(|| try_process_rlink(sess, compiler)); From 446c8e06d9480ac195077aa82d94759ed8eb637d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 13:22:06 +1100 Subject: [PATCH 3/5] Move `describe_lints` calls. Currently we have an inconsistency between the "input" and "no input" cases: - no input: `rustc --print=sysroot -Whelp` prints the lint help. - input: `rustc --print=sysroot -Whelp a.rs` prints the sysroot. It makes sense to print the lint help in both cases, because that's what happens with `--help`/`-Zhelp`/`-Chelp`. In fact, the `describe_lints` in the "input" case happens amazingly late, after *parsing*. This is because, with plugins, lints used to be registered much later, when the global context was created. But #117649 moved lint registration much earlier, during session construction. So this commit moves the `describe_lints` call to a single spot for both for both the "input" and "no input" cases, as early as possible. This is still not as early as `--help`/`-Zhelp`/`-Chelp`, because `-Whelp` must wait until the session is constructed. --- compiler/rustc_driver_impl/src/lib.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index f74eb0283649e..9c1a5d502306f 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -362,13 +362,18 @@ fn run_compiler( interface::run_compiler(config, |compiler| { let sess = compiler.session(); let codegen_backend = compiler.codegen_backend(); + + // 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(); + } + let handler = EarlyErrorHandler::new(sess.opts.error_format); if !has_input { - if sess.opts.describe_lints { - describe_lints(sess); - return sess.compile_status(); - } let should_stop = print_crate_info(&handler, codegen_backend, sess, false); if should_stop == Compilation::Continue { handler.early_error("no input filename given") @@ -419,11 +424,6 @@ fn run_compiler( return early_exit(); } - if sess.opts.describe_lints { - describe_lints(sess); - return early_exit(); - } - // Make sure name resolution and macro expansion is run. queries.global_ctxt()?.enter(|tcx| tcx.resolver_for_lowering(())); From 5659cc57e96a858ce57380553d2cae8f6da6ec5f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 13:45:30 +1100 Subject: [PATCH 4/5] Factor out two `print_crate_info` calls. --- compiler/rustc_driver_impl/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 9c1a5d502306f..a2e78336e71aa 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -373,15 +373,16 @@ fn run_compiler( let handler = EarlyErrorHandler::new(sess.opts.error_format); + let should_stop = print_crate_info(&handler, codegen_backend, sess, has_input); + if !has_input { - let should_stop = print_crate_info(&handler, codegen_backend, sess, false); if should_stop == Compilation::Continue { handler.early_error("no input filename given") } return sess.compile_status(); } - let should_stop = print_crate_info(&handler, codegen_backend, sess, true) + let should_stop = should_stop .and_then(|| list_metadata(&handler, sess, &*codegen_backend.metadata_loader())) .and_then(|| try_process_rlink(sess, compiler)); From 472f7c97a6a65e8b05c941d99d7a7ae54c4b59e0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 14:15:56 +1100 Subject: [PATCH 5/5] Simplify `run_compiler` control flow. I find `Compilation::and_then` hard to read. This commit removes it, simplifying the control flow in `run_compiler`, and reducing the number of lines of code. In particular, `list_metadata` and `process_try_link` (renamed `rlink`) are now only called if the relevant condition is true, rather than that condition being checked within the function. --- compiler/rustc_driver_impl/src/lib.rs | 126 +++++++++++--------------- 1 file changed, 54 insertions(+), 72 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index a2e78336e71aa..6ee7213a19d83 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -373,20 +373,21 @@ fn run_compiler( let handler = EarlyErrorHandler::new(sess.opts.error_format); - let should_stop = print_crate_info(&handler, codegen_backend, sess, has_input); + if print_crate_info(&handler, codegen_backend, sess, has_input) == Compilation::Stop { + return sess.compile_status(); + } if !has_input { - if should_stop == Compilation::Continue { - handler.early_error("no input filename given") - } - return sess.compile_status(); + handler.early_error("no input filename given"); // this is fatal } - let should_stop = should_stop - .and_then(|| list_metadata(&handler, sess, &*codegen_backend.metadata_loader())) - .and_then(|| try_process_rlink(sess, compiler)); + if !sess.opts.unstable_opts.ls.is_empty() { + list_metadata(&handler, sess, &*codegen_backend.metadata_loader()); + return sess.compile_status(); + } - if should_stop == Compilation::Stop { + if sess.opts.unstable_opts.link_only { + process_rlink(sess, compiler); return sess.compile_status(); } @@ -539,15 +540,6 @@ pub enum Compilation { Continue, } -impl Compilation { - fn and_then Compilation>(self, next: F) -> Compilation { - match self { - Compilation::Stop => Compilation::Stop, - Compilation::Continue => next(), - } - } -} - fn handle_explain(handler: &EarlyErrorHandler, registry: Registry, code: &str, color: ColorConfig) { let upper_cased_code = code.to_ascii_uppercase(); let normalised = @@ -652,44 +644,34 @@ fn show_md_content_with_pager(content: &str, color: ColorConfig) { } } -fn try_process_rlink(sess: &Session, compiler: &interface::Compiler) -> Compilation { - if sess.opts.unstable_opts.link_only { - if let Input::File(file) = &sess.io.input { - let outputs = compiler.build_output_filenames(sess, &[]); - let rlink_data = fs::read(file).unwrap_or_else(|err| { - sess.emit_fatal(RlinkUnableToRead { err }); - }); - let codegen_results = match CodegenResults::deserialize_rlink(sess, rlink_data) { - Ok(codegen) => codegen, - Err(err) => { - match err { - CodegenErrors::WrongFileType => sess.emit_fatal(RLinkWrongFileType), - CodegenErrors::EmptyVersionNumber => { - sess.emit_fatal(RLinkEmptyVersionNumber) - } - CodegenErrors::EncodingVersionMismatch { version_array, rlink_version } => { - sess.emit_fatal(RLinkEncodingVersionMismatch { - version_array, - rlink_version, - }) - } - CodegenErrors::RustcVersionMismatch { rustc_version } => { - sess.emit_fatal(RLinkRustcVersionMismatch { - rustc_version, - current_version: sess.cfg_version, - }) - } - }; - } - }; - let result = compiler.codegen_backend().link(sess, codegen_results, &outputs); - abort_on_err(result, sess); - } else { - sess.emit_fatal(RlinkNotAFile {}) - } - Compilation::Stop +fn process_rlink(sess: &Session, compiler: &interface::Compiler) { + assert!(sess.opts.unstable_opts.link_only); + if let Input::File(file) = &sess.io.input { + let outputs = compiler.build_output_filenames(sess, &[]); + let rlink_data = fs::read(file).unwrap_or_else(|err| { + sess.emit_fatal(RlinkUnableToRead { err }); + }); + let codegen_results = match CodegenResults::deserialize_rlink(sess, rlink_data) { + Ok(codegen) => codegen, + Err(err) => { + match err { + CodegenErrors::WrongFileType => sess.emit_fatal(RLinkWrongFileType), + CodegenErrors::EmptyVersionNumber => sess.emit_fatal(RLinkEmptyVersionNumber), + CodegenErrors::EncodingVersionMismatch { version_array, rlink_version } => sess + .emit_fatal(RLinkEncodingVersionMismatch { version_array, rlink_version }), + CodegenErrors::RustcVersionMismatch { rustc_version } => { + sess.emit_fatal(RLinkRustcVersionMismatch { + rustc_version, + current_version: sess.cfg_version, + }) + } + }; + } + }; + let result = compiler.codegen_backend().link(sess, codegen_results, &outputs); + abort_on_err(result, sess); } else { - Compilation::Continue + sess.emit_fatal(RlinkNotAFile {}) } } @@ -697,25 +679,25 @@ fn list_metadata( handler: &EarlyErrorHandler, sess: &Session, metadata_loader: &dyn MetadataLoader, -) -> Compilation { - let ls_kinds = &sess.opts.unstable_opts.ls; - if !ls_kinds.is_empty() { - match sess.io.input { - Input::File(ref ifile) => { - let path = &(*ifile); - let mut v = Vec::new(); - locator::list_file_metadata(&sess.target, path, metadata_loader, &mut v, ls_kinds) - .unwrap(); - safe_println!("{}", String::from_utf8(v).unwrap()); - } - Input::Str { .. } => { - handler.early_error("cannot list metadata for stdin"); - } +) { + match sess.io.input { + Input::File(ref ifile) => { + let path = &(*ifile); + let mut v = Vec::new(); + locator::list_file_metadata( + &sess.target, + path, + metadata_loader, + &mut v, + &sess.opts.unstable_opts.ls, + ) + .unwrap(); + safe_println!("{}", String::from_utf8(v).unwrap()); + } + Input::Str { .. } => { + handler.early_error("cannot list metadata for stdin"); } - return Compilation::Stop; } - - Compilation::Continue } fn print_crate_info(