From 678e01a3fc1d21eb8ea4b281b3905c6d3340ce54 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 13:37:34 +1100 Subject: [PATCH 01/11] Delay parsing of `--cfg` and `--check-cfg` options. By storing the unparsed values in `Config` and then parsing them within `run_compiler`, the parsing functions can use the main symbol interner, and not create their own short-lived interners. This change also eliminates the need for one `EarlyErrorHandler` in rustdoc, because parsing errors can be reported by another, slightly later `EarlyErrorHandler`. --- compiler/rustc_driver_impl/src/lib.rs | 6 +- compiler/rustc_interface/src/interface.rs | 497 +++++++++++----------- src/librustdoc/core.rs | 7 +- src/librustdoc/doctest.rs | 11 +- src/librustdoc/lib.rs | 5 +- 5 files changed, 254 insertions(+), 272 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 37897b8d702bb..e4cb3fd25cdbc 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -317,13 +317,11 @@ fn run_compiler( return Ok(()); } - let cfg = interface::parse_cfg(&early_error_handler, matches.opt_strs("cfg")); - let check_cfg = interface::parse_check_cfg(&early_error_handler, matches.opt_strs("check-cfg")); let (odir, ofile) = make_output(&matches); let mut config = interface::Config { opts: sopts, - crate_cfg: cfg, - crate_check_cfg: check_cfg, + crate_cfg: matches.opt_strs("cfg"), + crate_check_cfg: matches.opt_strs("check-cfg"), input: Input::File(PathBuf::new()), output_file: ofile, output_dir: odir, diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index e703fe228c3ba..6b51dfea9ecf8 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -65,295 +65,286 @@ impl Compiler { /// Converts strings provided as `--cfg [cfgspec]` into a `Cfg`. pub fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { - // This creates a short-lived `SessionGlobals`, containing an interner. The - // parsed values are converted from symbols to strings before exiting - // because the symbols are meaningless once the interner is gone. - rustc_span::create_default_session_if_not_set_then(move |_| { - cfgs.into_iter() - .map(|s| { - let sess = ParseSess::with_silent_emitter(Some(format!( - "this error occurred on the command line: `--cfg={s}`" - ))); - let filename = FileName::cfg_spec_source_code(&s); - - macro_rules! error { - ($reason: expr) => { - handler.early_error(format!( - concat!("invalid `--cfg` argument: `{}` (", $reason, ")"), - s - )); - }; - } - - match maybe_new_parser_from_source_str(&sess, filename, s.to_string()) { - Ok(mut parser) => match parser.parse_meta_item() { - Ok(meta_item) if parser.token == token::Eof => { - if meta_item.path.segments.len() != 1 { - error!("argument key must be an identifier"); - } - match &meta_item.kind { - MetaItemKind::List(..) => {} - MetaItemKind::NameValue(lit) if !lit.kind.is_str() => { - error!("argument value must be a string"); - } - MetaItemKind::NameValue(..) | MetaItemKind::Word => { - let ident = meta_item.ident().expect("multi-segment cfg key"); - return ( - ident.name.to_string(), - meta_item.value_str().map(|sym| sym.to_string()), - ); - } - } - } - Ok(..) => {} - Err(err) => err.cancel(), - }, - Err(errs) => drop(errs), - } - - // If the user tried to use a key="value" flag, but is missing the quotes, provide - // a hint about how to resolve this. - if s.contains('=') && !s.contains("=\"") && !s.ends_with('"') { - error!(concat!( - r#"expected `key` or `key="value"`, ensure escaping is appropriate"#, - r#" for your shell, try 'key="value"' or key=\"value\""# - )); - } else { - error!(r#"expected `key` or `key="value"`"#); - } - }) - .collect::>() - }) -} - -/// Converts strings provided as `--check-cfg [specs]` into a `CheckCfg`. -pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { - // The comment about `SessionGlobals` and symbols in `parse_cfg` above - // applies here too. - rustc_span::create_default_session_if_not_set_then(move |_| { - // If any --check-cfg is passed then exhaustive_values and exhaustive_names - // are enabled by default. - let exhaustive_names = !specs.is_empty(); - let exhaustive_values = !specs.is_empty(); - let mut check_cfg = CheckCfg { exhaustive_names, exhaustive_values, ..CheckCfg::default() }; - - let mut old_syntax = None; - for s in specs { + cfgs.into_iter() + .map(|s| { let sess = ParseSess::with_silent_emitter(Some(format!( - "this error occurred on the command line: `--check-cfg={s}`" + "this error occurred on the command line: `--cfg={s}`" ))); let filename = FileName::cfg_spec_source_code(&s); macro_rules! error { - ($reason:expr) => { + ($reason: expr) => { handler.early_error(format!( - concat!("invalid `--check-cfg` argument: `{}` (", $reason, ")"), + concat!("invalid `--cfg` argument: `{}` (", $reason, ")"), s - )) + )); }; } - let expected_error = || -> ! { - error!("expected `cfg(name, values(\"value1\", \"value2\", ... \"valueN\"))`") - }; + match maybe_new_parser_from_source_str(&sess, filename, s.to_string()) { + Ok(mut parser) => match parser.parse_meta_item() { + Ok(meta_item) if parser.token == token::Eof => { + if meta_item.path.segments.len() != 1 { + error!("argument key must be an identifier"); + } + match &meta_item.kind { + MetaItemKind::List(..) => {} + MetaItemKind::NameValue(lit) if !lit.kind.is_str() => { + error!("argument value must be a string"); + } + MetaItemKind::NameValue(..) | MetaItemKind::Word => { + let ident = meta_item.ident().expect("multi-segment cfg key"); + return ( + ident.name.to_string(), + meta_item.value_str().map(|sym| sym.to_string()), + ); + } + } + } + Ok(..) => {} + Err(err) => err.cancel(), + }, + Err(errs) => drop(errs), + } - let Ok(mut parser) = maybe_new_parser_from_source_str(&sess, filename, s.to_string()) - else { - expected_error(); - }; + // If the user tried to use a key="value" flag, but is missing the quotes, provide + // a hint about how to resolve this. + if s.contains('=') && !s.contains("=\"") && !s.ends_with('"') { + error!(concat!( + r#"expected `key` or `key="value"`, ensure escaping is appropriate"#, + r#" for your shell, try 'key="value"' or key=\"value\""# + )); + } else { + error!(r#"expected `key` or `key="value"`"#); + } + }) + .collect::>() +} - let meta_item = match parser.parse_meta_item() { - Ok(meta_item) if parser.token == token::Eof => meta_item, - Ok(..) => expected_error(), - Err(err) => { - err.cancel(); - expected_error(); - } +/// Converts strings provided as `--check-cfg [specs]` into a `CheckCfg`. +pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { + // If any --check-cfg is passed then exhaustive_values and exhaustive_names + // are enabled by default. + let exhaustive_names = !specs.is_empty(); + let exhaustive_values = !specs.is_empty(); + let mut check_cfg = CheckCfg { exhaustive_names, exhaustive_values, ..CheckCfg::default() }; + + let mut old_syntax = None; + for s in specs { + let sess = ParseSess::with_silent_emitter(Some(format!( + "this error occurred on the command line: `--check-cfg={s}`" + ))); + let filename = FileName::cfg_spec_source_code(&s); + + macro_rules! error { + ($reason:expr) => { + handler.early_error(format!( + concat!("invalid `--check-cfg` argument: `{}` (", $reason, ")"), + s + )) }; + } + + let expected_error = || -> ! { + error!("expected `cfg(name, values(\"value1\", \"value2\", ... \"valueN\"))`") + }; + + let Ok(mut parser) = maybe_new_parser_from_source_str(&sess, filename, s.to_string()) + else { + expected_error(); + }; - let Some(args) = meta_item.meta_item_list() else { + let meta_item = match parser.parse_meta_item() { + Ok(meta_item) if parser.token == token::Eof => meta_item, + Ok(..) => expected_error(), + Err(err) => { + err.cancel(); expected_error(); - }; + } + }; - if meta_item.has_name(sym::names) { - // defaults are flipped for the old syntax - if old_syntax == None { - check_cfg.exhaustive_names = false; - check_cfg.exhaustive_values = false; - } - old_syntax = Some(true); - - check_cfg.exhaustive_names = true; - for arg in args { - if arg.is_word() && arg.ident().is_some() { - let ident = arg.ident().expect("multi-segment cfg key"); - check_cfg - .expecteds - .entry(ident.name.to_string()) - .or_insert(ExpectedValues::Any); - } else { - error!("`names()` arguments must be simple identifiers"); - } - } - } else if meta_item.has_name(sym::values) { - // defaults are flipped for the old syntax - if old_syntax == None { - check_cfg.exhaustive_names = false; - check_cfg.exhaustive_values = false; + let Some(args) = meta_item.meta_item_list() else { + expected_error(); + }; + + if meta_item.has_name(sym::names) { + // defaults are flipped for the old syntax + if old_syntax == None { + check_cfg.exhaustive_names = false; + check_cfg.exhaustive_values = false; + } + old_syntax = Some(true); + + check_cfg.exhaustive_names = true; + for arg in args { + if arg.is_word() && arg.ident().is_some() { + let ident = arg.ident().expect("multi-segment cfg key"); + check_cfg + .expecteds + .entry(ident.name.to_string()) + .or_insert(ExpectedValues::Any); + } else { + error!("`names()` arguments must be simple identifiers"); } - old_syntax = Some(true); - - if let Some((name, values)) = args.split_first() { - if name.is_word() && name.ident().is_some() { - let ident = name.ident().expect("multi-segment cfg key"); - let expected_values = check_cfg - .expecteds - .entry(ident.name.to_string()) - .and_modify(|expected_values| match expected_values { - ExpectedValues::Some(_) => {} - ExpectedValues::Any => { - // handle the case where names(...) was done - // before values by changing to a list - *expected_values = ExpectedValues::Some(FxHashSet::default()); - } - }) - .or_insert_with(|| ExpectedValues::Some(FxHashSet::default())); - - let ExpectedValues::Some(expected_values) = expected_values else { - bug!("`expected_values` should be a list a values") - }; - - for val in values { - if let Some(LitKind::Str(s, _)) = val.lit().map(|lit| &lit.kind) { - expected_values.insert(Some(s.to_string())); - } else { - error!("`values()` arguments must be string literals"); + } + } else if meta_item.has_name(sym::values) { + // defaults are flipped for the old syntax + if old_syntax == None { + check_cfg.exhaustive_names = false; + check_cfg.exhaustive_values = false; + } + old_syntax = Some(true); + + if let Some((name, values)) = args.split_first() { + if name.is_word() && name.ident().is_some() { + let ident = name.ident().expect("multi-segment cfg key"); + let expected_values = check_cfg + .expecteds + .entry(ident.name.to_string()) + .and_modify(|expected_values| match expected_values { + ExpectedValues::Some(_) => {} + ExpectedValues::Any => { + // handle the case where names(...) was done + // before values by changing to a list + *expected_values = ExpectedValues::Some(FxHashSet::default()); } - } + }) + .or_insert_with(|| ExpectedValues::Some(FxHashSet::default())); + + let ExpectedValues::Some(expected_values) = expected_values else { + bug!("`expected_values` should be a list a values") + }; - if values.is_empty() { - expected_values.insert(None); + for val in values { + if let Some(LitKind::Str(s, _)) = val.lit().map(|lit| &lit.kind) { + expected_values.insert(Some(s.to_string())); + } else { + error!("`values()` arguments must be string literals"); } - } else { - error!("`values()` first argument must be a simple identifier"); } - } else if args.is_empty() { - check_cfg.exhaustive_values = true; + + if values.is_empty() { + expected_values.insert(None); + } } else { - expected_error(); + error!("`values()` first argument must be a simple identifier"); } - } else if meta_item.has_name(sym::cfg) { - old_syntax = Some(false); + } else if args.is_empty() { + check_cfg.exhaustive_values = true; + } else { + expected_error(); + } + } else if meta_item.has_name(sym::cfg) { + old_syntax = Some(false); - let mut names = Vec::new(); - let mut values: FxHashSet<_> = Default::default(); + let mut names = Vec::new(); + let mut values: FxHashSet<_> = Default::default(); - let mut any_specified = false; - let mut values_specified = false; - let mut values_any_specified = false; + let mut any_specified = false; + let mut values_specified = false; + let mut values_any_specified = false; - for arg in args { - if arg.is_word() && let Some(ident) = arg.ident() { - if values_specified { - error!("`cfg()` names cannot be after values"); - } - names.push(ident); - } else if arg.has_name(sym::any) - && let Some(args) = arg.meta_item_list() - { - if any_specified { - error!("`any()` cannot be specified multiple times"); - } - any_specified = true; - if !args.is_empty() { - error!("`any()` must be empty"); - } - } else if arg.has_name(sym::values) - && let Some(args) = arg.meta_item_list() - { - if names.is_empty() { - error!("`values()` cannot be specified before the names"); - } else if values_specified { - error!("`values()` cannot be specified multiple times"); - } - values_specified = true; - - for arg in args { - if let Some(LitKind::Str(s, _)) = - arg.lit().map(|lit| &lit.kind) - { - values.insert(Some(s.to_string())); - } else if arg.has_name(sym::any) - && let Some(args) = arg.meta_item_list() - { - if values_any_specified { - error!( - "`any()` in `values()` cannot be specified multiple times" - ); - } - values_any_specified = true; - if !args.is_empty() { - error!("`any()` must be empty"); - } - } else { + for arg in args { + if arg.is_word() && let Some(ident) = arg.ident() { + if values_specified { + error!("`cfg()` names cannot be after values"); + } + names.push(ident); + } else if arg.has_name(sym::any) + && let Some(args) = arg.meta_item_list() + { + if any_specified { + error!("`any()` cannot be specified multiple times"); + } + any_specified = true; + if !args.is_empty() { + error!("`any()` must be empty"); + } + } else if arg.has_name(sym::values) + && let Some(args) = arg.meta_item_list() + { + if names.is_empty() { + error!("`values()` cannot be specified before the names"); + } else if values_specified { + error!("`values()` cannot be specified multiple times"); + } + values_specified = true; + + for arg in args { + if let Some(LitKind::Str(s, _)) = + arg.lit().map(|lit| &lit.kind) + { + values.insert(Some(s.to_string())); + } else if arg.has_name(sym::any) + && let Some(args) = arg.meta_item_list() + { + if values_any_specified { error!( - "`values()` arguments must be string literals or `any()`" + "`any()` in `values()` cannot be specified multiple times" ); } + values_any_specified = true; + if !args.is_empty() { + error!("`any()` must be empty"); + } + } else { + error!( + "`values()` arguments must be string literals or `any()`" + ); } - } else { - error!( - "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`" - ); } - } - - if values.is_empty() && !values_any_specified && !any_specified { - values.insert(None); - } else if !values.is_empty() && values_any_specified { + } else { error!( - "`values()` arguments cannot specify string literals and `any()` at the same time" + "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`" ); } + } - if any_specified { - if names.is_empty() - && values.is_empty() - && !values_specified - && !values_any_specified - { - check_cfg.exhaustive_names = false; - } else { - error!("`cfg(any())` can only be provided in isolation"); - } + if values.is_empty() && !values_any_specified && !any_specified { + values.insert(None); + } else if !values.is_empty() && values_any_specified { + error!( + "`values()` arguments cannot specify string literals and `any()` at the same time" + ); + } + + if any_specified { + if names.is_empty() + && values.is_empty() + && !values_specified + && !values_any_specified + { + check_cfg.exhaustive_names = false; } else { - for name in names { - check_cfg - .expecteds - .entry(name.to_string()) - .and_modify(|v| match v { - ExpectedValues::Some(v) if !values_any_specified => { - v.extend(values.clone()) - } - ExpectedValues::Some(_) => *v = ExpectedValues::Any, - ExpectedValues::Any => {} - }) - .or_insert_with(|| { - if values_any_specified { - ExpectedValues::Any - } else { - ExpectedValues::Some(values.clone()) - } - }); - } + error!("`cfg(any())` can only be provided in isolation"); } } else { - expected_error(); + for name in names { + check_cfg + .expecteds + .entry(name.to_string()) + .and_modify(|v| match v { + ExpectedValues::Some(v) if !values_any_specified => { + v.extend(values.clone()) + } + ExpectedValues::Some(_) => *v = ExpectedValues::Any, + ExpectedValues::Any => {} + }) + .or_insert_with(|| { + if values_any_specified { + ExpectedValues::Any + } else { + ExpectedValues::Some(values.clone()) + } + }); + } } + } else { + expected_error(); } + } - check_cfg - }) + check_cfg } /// The compiler configuration @@ -361,9 +352,9 @@ pub struct Config { /// Command line options pub opts: config::Options, - /// cfg! configuration in addition to the default ones - pub crate_cfg: Cfg, - pub crate_check_cfg: CheckCfg, + /// Unparsed cfg! configuration in addition to the default ones. + pub crate_cfg: Vec, + pub crate_check_cfg: Vec, pub input: Input, pub output_dir: Option, @@ -436,8 +427,8 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se let (mut sess, codegen_backend) = util::create_session( &handler, config.opts, - config.crate_cfg, - config.crate_check_cfg, + parse_cfg(&handler, config.crate_cfg), + parse_check_cfg(&handler, config.crate_check_cfg), config.locale_resources, config.file_loader, CompilerIO { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index bcc61df7c4d84..17b117de5d471 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -14,8 +14,8 @@ use rustc_lint::{late_lint_mod, MissingDoc}; use rustc_middle::hir::nested_filter; use rustc_middle::ty::{ParamEnv, Ty, TyCtxt}; use rustc_session::config::{self, CrateType, ErrorOutputType, ResolveDocLinks}; +use rustc_session::lint; use rustc_session::Session; -use rustc_session::{lint, EarlyErrorHandler}; use rustc_span::symbol::sym; use rustc_span::{source_map, Span}; @@ -175,7 +175,6 @@ pub(crate) fn new_handler( /// Parse, resolve, and typecheck the given crate. pub(crate) fn create_config( - handler: &EarlyErrorHandler, RustdocOptions { input, crate_name, @@ -255,8 +254,8 @@ pub(crate) fn create_config( interface::Config { opts: sessopts, - crate_cfg: interface::parse_cfg(handler, cfgs), - crate_check_cfg: interface::parse_check_cfg(handler, check_cfgs), + crate_cfg: cfgs, + crate_check_cfg: check_cfgs, input, output_file: None, output_dir: None, diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index c61c98e4de9c6..2412865801d6d 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -13,7 +13,7 @@ use rustc_parse::parser::attr::InnerAttrPolicy; use rustc_resolve::rustdoc::span_of_fragments; use rustc_session::config::{self, CrateType, ErrorOutputType}; use rustc_session::parse::ParseSess; -use rustc_session::{lint, EarlyErrorHandler, Session}; +use rustc_session::{lint, Session}; use rustc_span::edition::Edition; use rustc_span::source_map::SourceMap; use rustc_span::symbol::sym; @@ -85,18 +85,13 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> { ..config::Options::default() }; - let early_error_handler = EarlyErrorHandler::new(ErrorOutputType::default()); - let mut cfgs = options.cfgs.clone(); cfgs.push("doc".to_owned()); cfgs.push("doctest".to_owned()); let config = interface::Config { opts: sessopts, - crate_cfg: interface::parse_cfg(&early_error_handler, cfgs), - crate_check_cfg: interface::parse_check_cfg( - &early_error_handler, - options.check_cfgs.clone(), - ), + crate_cfg: cfgs, + crate_check_cfg: options.check_cfgs.clone(), input, output_file: None, output_dir: None, diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 41aee06c8d26f..dda06d4c9c1d7 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -757,8 +757,7 @@ fn main_args( (false, true) => { let input = options.input.clone(); let edition = options.edition; - let config = - core::create_config(handler, options, &render_options, using_internal_features); + let config = core::create_config(options, &render_options, using_internal_features); // `markdown::render` can invoke `doctest::make_test`, which // requires session globals and a thread pool, so we use @@ -791,7 +790,7 @@ fn main_args( let scrape_examples_options = options.scrape_examples_options.clone(); let bin_crate = options.bin_crate; - let config = core::create_config(handler, options, &render_options, using_internal_features); + let config = core::create_config(options, &render_options, using_internal_features); interface::run_compiler(config, |compiler| { let sess = compiler.session(); From bfcff7933e9be5c9ff31854896e9582b84b19d36 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 13:52:03 +1100 Subject: [PATCH 02/11] Reduce exposure of cfg parsers. --- compiler/rustc_interface/src/interface.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 6b51dfea9ecf8..3b442697b5f32 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -64,7 +64,7 @@ impl Compiler { } /// Converts strings provided as `--cfg [cfgspec]` into a `Cfg`. -pub fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { +pub(crate) fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { cfgs.into_iter() .map(|s| { let sess = ParseSess::with_silent_emitter(Some(format!( @@ -122,7 +122,7 @@ pub fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg } /// Converts strings provided as `--check-cfg [specs]` into a `CheckCfg`. -pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { +pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { // If any --check-cfg is passed then exhaustive_values and exhaustive_names // are enabled by default. let exhaustive_names = !specs.is_empty(); From 8e4ac980fd775d311bc63642c8d6b5739aa6f34c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 14:01:33 +1100 Subject: [PATCH 03/11] Change cfg parsers to produce symbols instead of strings. --- compiler/rustc_interface/src/interface.rs | 29 ++++++++------------ compiler/rustc_interface/src/tests.rs | 4 +-- compiler/rustc_interface/src/util.rs | 5 ++-- compiler/rustc_session/src/config.rs | 32 +---------------------- 4 files changed, 16 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 3b442697b5f32..cf887bc665bde 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -24,6 +24,7 @@ use rustc_session::Session; use rustc_session::{lint, EarlyErrorHandler}; use rustc_span::source_map::{FileLoader, FileName}; use rustc_span::symbol::sym; +use rustc_span::Symbol; use std::path::PathBuf; use std::result; use std::sync::Arc; @@ -64,7 +65,7 @@ impl Compiler { } /// Converts strings provided as `--cfg [cfgspec]` into a `Cfg`. -pub(crate) fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { +pub(crate) fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { cfgs.into_iter() .map(|s| { let sess = ParseSess::with_silent_emitter(Some(format!( @@ -94,10 +95,7 @@ pub(crate) fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { let ident = meta_item.ident().expect("multi-segment cfg key"); - return ( - ident.name.to_string(), - meta_item.value_str().map(|sym| sym.to_string()), - ); + return (ident.name, meta_item.value_str()); } } } @@ -118,11 +116,11 @@ pub(crate) fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg>() + .collect::>() } /// Converts strings provided as `--check-cfg [specs]` into a `CheckCfg`. -pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { +pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { // If any --check-cfg is passed then exhaustive_values and exhaustive_names // are enabled by default. let exhaustive_names = !specs.is_empty(); @@ -179,10 +177,7 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - for arg in args { if arg.is_word() && arg.ident().is_some() { let ident = arg.ident().expect("multi-segment cfg key"); - check_cfg - .expecteds - .entry(ident.name.to_string()) - .or_insert(ExpectedValues::Any); + check_cfg.expecteds.entry(ident.name).or_insert(ExpectedValues::Any); } else { error!("`names()` arguments must be simple identifiers"); } @@ -200,7 +195,7 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - let ident = name.ident().expect("multi-segment cfg key"); let expected_values = check_cfg .expecteds - .entry(ident.name.to_string()) + .entry(ident.name) .and_modify(|expected_values| match expected_values { ExpectedValues::Some(_) => {} ExpectedValues::Any => { @@ -217,7 +212,7 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - for val in values { if let Some(LitKind::Str(s, _)) = val.lit().map(|lit| &lit.kind) { - expected_values.insert(Some(s.to_string())); + expected_values.insert(Some(*s)); } else { error!("`values()` arguments must be string literals"); } @@ -271,10 +266,8 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - values_specified = true; for arg in args { - if let Some(LitKind::Str(s, _)) = - arg.lit().map(|lit| &lit.kind) - { - values.insert(Some(s.to_string())); + if let Some(LitKind::Str(s, _)) = arg.lit().map(|lit| &lit.kind) { + values.insert(Some(*s)); } else if arg.has_name(sym::any) && let Some(args) = arg.meta_item_list() { @@ -322,7 +315,7 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - for name in names { check_cfg .expecteds - .entry(name.to_string()) + .entry(name.name) .and_modify(|v| match v { ExpectedValues::Some(v) if !values_any_specified => { v.extend(values.clone()) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 57ca709267a7e..466aa21ad8e73 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -26,8 +26,8 @@ use rustc_session::{build_session, getopts, Session}; use rustc_session::{CompilerIO, EarlyErrorHandler}; use rustc_span::edition::{Edition, DEFAULT_EDITION}; use rustc_span::symbol::sym; -use rustc_span::FileName; use rustc_span::SourceFileHashAlgorithm; +use rustc_span::{FileName, Symbol}; use rustc_target::spec::{CodeModel, LinkerFlavorCli, MergeFunctions, PanicStrategy, RelocModel}; use rustc_target::spec::{RelroLevel, SanitizerSet, SplitDebuginfo, StackProtector, TlsModel}; use std::collections::{BTreeMap, BTreeSet}; @@ -38,7 +38,7 @@ use std::sync::Arc; fn mk_session( handler: &mut EarlyErrorHandler, matches: getopts::Matches, -) -> (Session, Cfg) { +) -> (Session, Cfg) { let registry = registry::Registry::new(&[]); let sessopts = build_session_options(handler, &matches); let cfg = parse_cfg(handler, matches.opt_strs("cfg")); diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 4d0be65697a42..a974bdd3c0e61 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -59,8 +59,8 @@ pub fn add_configuration( pub fn create_session( handler: &EarlyErrorHandler, sopts: config::Options, - cfg: Cfg, - check_cfg: CheckCfg, + cfg: Cfg, + mut check_cfg: CheckCfg, locale_resources: &'static [&'static str], file_loader: Option>, io: CompilerIO, @@ -123,7 +123,6 @@ pub fn create_session( let mut cfg = config::build_configuration(&sess, cfg); add_configuration(&mut cfg, &mut sess, &*codegen_backend); - let mut check_cfg = check_cfg.intern(); check_cfg.fill_well_known(&sess.target); // These configs use symbols, rather than strings. diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 854e2b779935c..4a0bdd511a37f 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1386,30 +1386,6 @@ impl Default for CheckCfg { } } -impl CheckCfg { - pub fn intern(self) -> CheckCfg { - CheckCfg { - exhaustive_names: self.exhaustive_names, - exhaustive_values: self.exhaustive_values, - expecteds: self - .expecteds - .into_iter() - .map(|(name, values)| { - ( - Symbol::intern(&name), - match values { - ExpectedValues::Some(values) => ExpectedValues::Some( - values.into_iter().map(|b| b.map(|b| Symbol::intern(&b))).collect(), - ), - ExpectedValues::Any => ExpectedValues::Any, - }, - ) - }) - .collect(), - } - } -} - pub enum ExpectedValues { Some(FxHashSet>), Any, @@ -1582,13 +1558,7 @@ impl CheckCfg { } } -pub fn build_configuration(sess: &Session, user_cfg: Cfg) -> Cfg { - // We can now intern these strings. - let mut user_cfg: Cfg = user_cfg - .into_iter() - .map(|(a, b)| (Symbol::intern(&a), b.map(|b| Symbol::intern(&b)))) - .collect(); - +pub fn build_configuration(sess: &Session, mut user_cfg: Cfg) -> Cfg { // Combine the configuration requested by the session (command line) with // some default and generated configuration items. let default_cfg = default_configuration(sess); From 5c6a12c1affd7759eb793f3bcd2a97577fab7ab4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 14:05:06 +1100 Subject: [PATCH 04/11] Make `Cfg` and `CheckCfg` non-generic. They now only ever contains symbols. --- compiler/rustc_interface/src/interface.rs | 7 +++-- compiler/rustc_interface/src/tests.rs | 8 ++---- compiler/rustc_interface/src/util.rs | 10 +++----- compiler/rustc_session/src/config.rs | 31 ++++++++--------------- compiler/rustc_session/src/parse.rs | 4 +-- 5 files changed, 20 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index cf887bc665bde..66bcb199709e1 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -24,7 +24,6 @@ use rustc_session::Session; use rustc_session::{lint, EarlyErrorHandler}; use rustc_span::source_map::{FileLoader, FileName}; use rustc_span::symbol::sym; -use rustc_span::Symbol; use std::path::PathBuf; use std::result; use std::sync::Arc; @@ -65,7 +64,7 @@ impl Compiler { } /// Converts strings provided as `--cfg [cfgspec]` into a `Cfg`. -pub(crate) fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { +pub(crate) fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg { cfgs.into_iter() .map(|s| { let sess = ParseSess::with_silent_emitter(Some(format!( @@ -116,11 +115,11 @@ pub(crate) fn parse_cfg(handler: &EarlyErrorHandler, cfgs: Vec) -> Cfg>() + .collect::() } /// Converts strings provided as `--check-cfg [specs]` into a `CheckCfg`. -pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { +pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> CheckCfg { // If any --check-cfg is passed then exhaustive_values and exhaustive_names // are enabled by default. let exhaustive_names = !specs.is_empty(); diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 466aa21ad8e73..41ccb25f7893a 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -26,8 +26,7 @@ use rustc_session::{build_session, getopts, Session}; use rustc_session::{CompilerIO, EarlyErrorHandler}; use rustc_span::edition::{Edition, DEFAULT_EDITION}; use rustc_span::symbol::sym; -use rustc_span::SourceFileHashAlgorithm; -use rustc_span::{FileName, Symbol}; +use rustc_span::{FileName, SourceFileHashAlgorithm}; use rustc_target::spec::{CodeModel, LinkerFlavorCli, MergeFunctions, PanicStrategy, RelocModel}; use rustc_target::spec::{RelroLevel, SanitizerSet, SplitDebuginfo, StackProtector, TlsModel}; use std::collections::{BTreeMap, BTreeSet}; @@ -35,10 +34,7 @@ use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::sync::Arc; -fn mk_session( - handler: &mut EarlyErrorHandler, - matches: getopts::Matches, -) -> (Session, Cfg) { +fn mk_session(handler: &mut EarlyErrorHandler, matches: getopts::Matches) -> (Session, Cfg) { let registry = registry::Registry::new(&[]); let sessopts = build_session_options(handler, &matches); let cfg = parse_cfg(handler, matches.opt_strs("cfg")); diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index a974bdd3c0e61..9bda685df0134 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -36,11 +36,7 @@ pub type MakeBackendFn = fn() -> Box; /// /// This is performed by checking whether a set of permitted features /// is available on the target machine, by querying the codegen backend. -pub fn add_configuration( - cfg: &mut Cfg, - sess: &mut Session, - codegen_backend: &dyn CodegenBackend, -) { +pub fn add_configuration(cfg: &mut Cfg, sess: &mut Session, codegen_backend: &dyn CodegenBackend) { let tf = sym::target_feature; let unstable_target_features = codegen_backend.target_features(sess, true); @@ -59,8 +55,8 @@ pub fn add_configuration( pub fn create_session( handler: &EarlyErrorHandler, sopts: config::Options, - cfg: Cfg, - mut check_cfg: CheckCfg, + cfg: Cfg, + mut check_cfg: CheckCfg, locale_resources: &'static [&'static str], file_loader: Option>, io: CompilerIO, diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 4a0bdd511a37f..a8ebab4ae333c 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1247,8 +1247,8 @@ pub const fn default_lib_output() -> CrateType { CrateType::Rlib } -fn default_configuration(sess: &Session) -> Cfg { - // NOTE: This should be kept in sync with `CheckCfg::::fill_well_known` below. +fn default_configuration(sess: &Session) -> Cfg { + // NOTE: This should be kept in sync with `CheckCfg::fill_well_known` below. let end = &sess.target.endian; let arch = &sess.target.arch; let wordsz = sess.target.pointer_width.to_string(); @@ -1358,32 +1358,21 @@ fn default_configuration(sess: &Session) -> Cfg { } /// The parsed `--cfg` options that define the compilation environment of the -/// crate, used to drive conditional compilation. `T` is always `String` or -/// `Symbol`. Strings are used temporarily very early on. Once the the main -/// symbol interner is running, they are converted to symbols. +/// crate, used to drive conditional compilation. /// /// An `FxIndexSet` is used to ensure deterministic ordering of error messages /// relating to `--cfg`. -pub type Cfg = FxIndexSet<(T, Option)>; +pub type Cfg = FxIndexSet<(Symbol, Option)>; -/// The parsed `--check-cfg` options. The `` structure is similar to `Cfg`. -pub struct CheckCfg { +/// The parsed `--check-cfg` options. +#[derive(Default)] +pub struct CheckCfg { /// Is well known names activated pub exhaustive_names: bool, /// Is well known values activated pub exhaustive_values: bool, /// All the expected values for a config name - pub expecteds: FxHashMap>, -} - -impl Default for CheckCfg { - fn default() -> Self { - CheckCfg { - exhaustive_names: false, - exhaustive_values: false, - expecteds: FxHashMap::default(), - } - } + pub expecteds: FxHashMap>, } pub enum ExpectedValues { @@ -1418,7 +1407,7 @@ impl<'a, T: Eq + Hash + Copy + 'a> Extend<&'a T> for ExpectedValues { } } -impl CheckCfg { +impl CheckCfg { pub fn fill_well_known(&mut self, current_target: &Target) { if !self.exhaustive_values && !self.exhaustive_names { return; @@ -1558,7 +1547,7 @@ impl CheckCfg { } } -pub fn build_configuration(sess: &Session, mut user_cfg: Cfg) -> Cfg { +pub fn build_configuration(sess: &Session, mut user_cfg: Cfg) -> Cfg { // Combine the configuration requested by the session (command line) with // some default and generated configuration items. let default_cfg = default_configuration(sess); diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 5b98ee5d992df..4d20d6d418782 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -188,8 +188,8 @@ pub fn add_feature_diagnostics_for_issue( pub struct ParseSess { pub span_diagnostic: Handler, pub unstable_features: UnstableFeatures, - pub config: Cfg, - pub check_config: CheckCfg, + pub config: Cfg, + pub check_config: CheckCfg, pub edition: Edition, /// Places where raw identifiers were used. This is used to avoid complaining about idents /// clashing with keywords in new editions. From 371f97257147c88d102dd501992b48ca568cb79b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 15:06:48 +1100 Subject: [PATCH 05/11] Improve readability of `parse_check_cfg`. --- compiler/rustc_interface/src/interface.rs | 39 ++++++++--------------- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 66bcb199709e1..26034dbaf9977 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -164,34 +164,31 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - expected_error(); }; - if meta_item.has_name(sym::names) { + let mut set_old_syntax = || { // defaults are flipped for the old syntax if old_syntax == None { check_cfg.exhaustive_names = false; check_cfg.exhaustive_values = false; } old_syntax = Some(true); + }; + + if meta_item.has_name(sym::names) { + set_old_syntax(); check_cfg.exhaustive_names = true; for arg in args { - if arg.is_word() && arg.ident().is_some() { - let ident = arg.ident().expect("multi-segment cfg key"); + if arg.is_word() && let Some(ident) = arg.ident() { check_cfg.expecteds.entry(ident.name).or_insert(ExpectedValues::Any); } else { error!("`names()` arguments must be simple identifiers"); } } } else if meta_item.has_name(sym::values) { - // defaults are flipped for the old syntax - if old_syntax == None { - check_cfg.exhaustive_names = false; - check_cfg.exhaustive_values = false; - } - old_syntax = Some(true); + set_old_syntax(); if let Some((name, values)) = args.split_first() { - if name.is_word() && name.ident().is_some() { - let ident = name.ident().expect("multi-segment cfg key"); + if name.is_word() && let Some(ident) = name.ident() { let expected_values = check_cfg .expecteds .entry(ident.name) @@ -244,9 +241,7 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - error!("`cfg()` names cannot be after values"); } names.push(ident); - } else if arg.has_name(sym::any) - && let Some(args) = arg.meta_item_list() - { + } else if arg.has_name(sym::any) && let Some(args) = arg.meta_item_list() { if any_specified { error!("`any()` cannot be specified multiple times"); } @@ -254,9 +249,7 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - if !args.is_empty() { error!("`any()` must be empty"); } - } else if arg.has_name(sym::values) - && let Some(args) = arg.meta_item_list() - { + } else if arg.has_name(sym::values) && let Some(args) = arg.meta_item_list() { if names.is_empty() { error!("`values()` cannot be specified before the names"); } else if values_specified { @@ -267,22 +260,16 @@ pub(crate) fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) - for arg in args { if let Some(LitKind::Str(s, _)) = arg.lit().map(|lit| &lit.kind) { values.insert(Some(*s)); - } else if arg.has_name(sym::any) - && let Some(args) = arg.meta_item_list() - { + } else if arg.has_name(sym::any) && let Some(args) = arg.meta_item_list() { if values_any_specified { - error!( - "`any()` in `values()` cannot be specified multiple times" - ); + error!("`any()` in `values()` cannot be specified multiple times"); } values_any_specified = true; if !args.is_empty() { error!("`any()` must be empty"); } } else { - error!( - "`values()` arguments must be string literals or `any()`" - ); + error!("`values()` arguments must be string literals or `any()`"); } } } else { From 85e56e81f8b5ba947d2522369019088e5bd5d6e1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 15:26:47 +1100 Subject: [PATCH 06/11] Remove out-of-date comment. It was added in 51938c61f6f1b26e463f9071716f543543486e72, a commit with a 7,720 line diff and a one line commit message. Even then the comment was incorrect; there was a removed a `build_output_filenames` call with a `&[]` argument in rustdoc, but the commit removed that call. In such a large commit, it's easy for small errors to occur. --- compiler/rustc_interface/src/passes.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 884afae23d803..1736a1f6d5375 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -602,9 +602,7 @@ fn output_filenames(tcx: TyCtxt<'_>, (): ()) -> Arc { let (_, krate) = &*tcx.resolver_for_lowering(()).borrow(); let crate_name = tcx.crate_name(LOCAL_CRATE); - // FIXME: rustdoc passes &[] instead of &krate.attrs here let outputs = util::build_output_filenames(&krate.attrs, sess); - let output_paths = generated_output_paths(tcx, &outputs, sess.io.output_file.is_some(), crate_name); From a60d6438dc483f9d999716f70ea153d7246971b1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 15:33:05 +1100 Subject: [PATCH 07/11] Wrap some overlong comments. --- compiler/rustc_interface/src/passes.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 1736a1f6d5375..9685b0777768b 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -880,16 +880,18 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> { let trait_ref = ty::Binder::dummy(ty::TraitRef::identity(tcx, tr)); - // A slightly edited version of the code in `rustc_trait_selection::traits::vtable::vtable_entries`, - // that works without self type and just counts number of entries. + // A slightly edited version of the code in + // `rustc_trait_selection::traits::vtable::vtable_entries`, that works without self + // type and just counts number of entries. // - // Note that this is technically wrong, for traits which have associated types in supertraits: + // Note that this is technically wrong, for traits which have associated types in + // supertraits: // // trait A: AsRef + AsRef<()> { type T; } // - // Without self type we can't normalize `Self::T`, so we can't know if `AsRef` and - // `AsRef<()>` are the same trait, thus we assume that those are different, and potentially - // over-estimate how many vtable entries there are. + // Without self type we can't normalize `Self::T`, so we can't know if `AsRef` + // and `AsRef<()>` are the same trait, thus we assume that those are different, and + // potentially over-estimate how many vtable entries there are. // // Similarly this is wrong for traits that have methods with possibly-impossible bounds. // For example: @@ -916,10 +918,10 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> { let own_existential_entries = tcx.own_existential_vtable_entries(trait_ref.def_id()); - // The original code here ignores the method if its predicates are impossible. - // We can't really do that as, for example, all not trivial bounds on generic - // parameters are impossible (since we don't know the parameters...), - // see the comment above. + // The original code here ignores the method if its predicates are + // impossible. We can't really do that as, for example, all not trivial + // bounds on generic parameters are impossible (since we don't know the + // parameters...), see the comment above. entries_ignoring_upcasting += own_existential_entries.len(); if emit_vptr { From 95b0088e7c7a781bcd77a274ec73d7fb4f9074a2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 15:42:03 +1100 Subject: [PATCH 08/11] Remove `check_output`. Using `find` and `any` from `std` makes the code shorter and clearer. --- compiler/rustc_interface/src/passes.rs | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 9685b0777768b..2099a6d59cbb5 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -392,34 +392,16 @@ fn generated_output_paths( out_filenames } -// Runs `f` on every output file path and returns the first non-None result, or None if `f` -// returns None for every file path. -fn check_output(output_paths: &[PathBuf], f: F) -> Option -where - F: Fn(&PathBuf) -> Option, -{ - for output_path in output_paths { - if let Some(result) = f(output_path) { - return Some(result); - } - } - None -} - fn output_contains_path(output_paths: &[PathBuf], input_path: &Path) -> bool { let input_path = try_canonicalize(input_path).ok(); if input_path.is_none() { return false; } - let check = |output_path: &PathBuf| { - if try_canonicalize(output_path).ok() == input_path { Some(()) } else { None } - }; - check_output(output_paths, check).is_some() + output_paths.iter().any(|output_path| try_canonicalize(output_path).ok() == input_path) } -fn output_conflicts_with_dir(output_paths: &[PathBuf]) -> Option { - let check = |output_path: &PathBuf| output_path.is_dir().then(|| output_path.clone()); - check_output(output_paths, check) +fn output_conflicts_with_dir(output_paths: &[PathBuf]) -> Option<&PathBuf> { + output_paths.iter().find(|output_path| output_path.is_dir()) } fn escape_dep_filename(filename: &str) -> String { From be8fd8b7d028c50c66b6300aa1bf692afe2e231c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 16:10:20 +1100 Subject: [PATCH 09/11] Streamline `collect_crate_types`. - The early return can be right at the top. - The control flow is simplified with `if let`. - The `collect` isn't necessary. - The "Unconditionally" comment is erroneously duplicated from `check_attr_crate_type`, and can be removed. --- compiler/rustc_interface/src/util.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 9bda685df0134..5fde98c7c062e 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -482,21 +482,6 @@ fn categorize_crate_type(s: Symbol) -> Option { } pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec { - // Unconditionally collect crate types from attributes to make them used - let attr_types: Vec = attrs - .iter() - .filter_map(|a| { - if a.has_name(sym::crate_type) { - match a.value_str() { - Some(s) => categorize_crate_type(s), - _ => None, - } - } else { - None - } - }) - .collect(); - // If we're generating a test executable, then ignore all other output // styles at all other locations if session.opts.test { @@ -510,6 +495,13 @@ pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec Date: Mon, 30 Oct 2023 18:19:40 +1100 Subject: [PATCH 10/11] Remove an unnecessary `drop`. --- compiler/rustc_interface/src/callbacks.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_interface/src/callbacks.rs b/compiler/rustc_interface/src/callbacks.rs index 6fa989bb96ce0..ef00ced67ffea 100644 --- a/compiler/rustc_interface/src/callbacks.rs +++ b/compiler/rustc_interface/src/callbacks.rs @@ -33,9 +33,7 @@ fn track_diagnostic(diagnostic: &mut Diagnostic, f: &mut dyn FnMut(&mut Diagnost tls::with_context_opt(|icx| { if let Some(icx) = icx { if let Some(diagnostics) = icx.diagnostics { - let mut diagnostics = diagnostics.lock(); - diagnostics.extend(Some(diagnostic.clone())); - std::mem::drop(diagnostics); + diagnostics.lock().extend(Some(diagnostic.clone())); } // Diagnostics are tracked, we can ignore the dependency. From 0c381ec05afd1f51f12c9727fc05eead2400d500 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 18:29:23 +1100 Subject: [PATCH 11/11] Streamline some `use` items. --- compiler/rustc_interface/src/tests.rs | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 41ccb25f7893a..594283168c98d 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -2,28 +2,18 @@ use crate::interface::parse_cfg; use rustc_data_structures::profiling::TimePassesFormat; use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig}; -use rustc_session::config::rustc_optgroups; -use rustc_session::config::Cfg; -use rustc_session::config::DebugInfo; -use rustc_session::config::Input; -use rustc_session::config::InstrumentXRay; -use rustc_session::config::LinkSelfContained; -use rustc_session::config::Polonius; -use rustc_session::config::TraitSolver; -use rustc_session::config::{build_configuration, build_session_options}; use rustc_session::config::{ - BranchProtection, Externs, OomStrategy, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet, - ProcMacroExecutionStrategy, SymbolManglingVersion, WasiExecModel, + build_configuration, build_session_options, rustc_optgroups, BranchProtection, CFGuard, Cfg, + DebugInfo, DumpMonoStatsFormat, ErrorOutputType, ExternEntry, ExternLocation, Externs, Input, + InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli, + MirSpanview, OomStrategy, Options, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet, + Passes, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath, SymbolManglingVersion, + TraitSolver, WasiExecModel, }; -use rustc_session::config::{CFGuard, ExternEntry, LinkerPluginLto, LtoCli, SwitchWithOptPath}; -use rustc_session::config::{DumpMonoStatsFormat, MirSpanview}; -use rustc_session::config::{ErrorOutputType, ExternLocation, LocationDetail, Options, Strip}; -use rustc_session::config::{InstrumentCoverage, Passes}; use rustc_session::lint::Level; use rustc_session::search_paths::SearchPath; use rustc_session::utils::{CanonicalizedPath, NativeLib, NativeLibKind}; -use rustc_session::{build_session, getopts, Session}; -use rustc_session::{CompilerIO, EarlyErrorHandler}; +use rustc_session::{build_session, getopts, CompilerIO, EarlyErrorHandler, Session}; use rustc_span::edition::{Edition, DEFAULT_EDITION}; use rustc_span::symbol::sym; use rustc_span::{FileName, SourceFileHashAlgorithm};