From 678e01a3fc1d21eb8ea4b281b3905c6d3340ce54 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 30 Oct 2023 13:37:34 +1100 Subject: [PATCH] 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 | 531 +++++++++++----------- src/librustdoc/core.rs | 7 +- src/librustdoc/doctest.rs | 11 +- src/librustdoc/lib.rs | 5 +- 5 files changed, 271 insertions(+), 289 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 37897b8d702..e4cb3fd25cd 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 e703fe228c3..6b51dfea9ec 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\"))`") - }; - - let Ok(mut parser) = maybe_new_parser_from_source_str(&sess, filename, s.to_string()) - else { - expected_error(); - }; - - 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(); - } - }; - - 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"); - } - } - } 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") - }; - - 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"); + 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"); } - } - - if values.is_empty() { - expected_values.insert(None); - } - } else { - error!("`values()` first argument must be a simple identifier"); - } - } 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 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 { - error!( - "`values()` arguments must be string literals or `any()`" + 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()), ); } } - } else { - error!( - "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`" - ); } - } + Ok(..) => {} + Err(err) => err.cancel(), + }, + Err(errs) => drop(errs), + } - 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 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::>() +} - 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"); +/// 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 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(); + } + }; + + 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"); + } + } + } 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") + }; + + 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"); + } + } + + if values.is_empty() { + expected_values.insert(None); } } 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!("`values()` first argument must be a simple identifier"); } + } 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); - check_cfg - }) + 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; + + 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 { + 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 { + 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 { + error!("`cfg(any())` can only be provided in isolation"); + } + } 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()) + } + }); + } + } + } else { + expected_error(); + } + } + + 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 bcc61df7c4d..17b117de5d4 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 c61c98e4de9..2412865801d 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 41aee06c8d2..dda06d4c9c1 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();