From 828f069c12b356631ef2f7b707d51708861a788d Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 20 Oct 2023 19:33:22 +0200 Subject: [PATCH 1/3] Remove most indentation in check-cfg impl --- compiler/rustc_interface/src/interface.rs | 381 ++++++++++------------ 1 file changed, 181 insertions(+), 200 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index d2ce77ad53511..2218892538212 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -139,7 +139,7 @@ pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> Check 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, ")"), s @@ -147,217 +147,198 @@ pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> Check }; } - 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 let Some(args) = meta_item.meta_item_list() { - 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 { - 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()`" - ); - } - } - } else { - error!( - "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`" - ); - } + 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") + }; - if values.is_empty() && !values_any_specified && !any_specified { - values.insert(None); - } else if !values.is_empty() && values_any_specified { + 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 { + 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!( - "`values()` arguments cannot specify string literals and `any()` at the same time" + "`any()` in `values()` cannot be specified multiple times" ); } - - if any_specified { - if !names.is_empty() - || !values.is_empty() - || values_any_specified - { - error!("`cfg(any())` can only be provided in isolation"); - } - - 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()) - } - }); - } + values_any_specified = true; + if !args.is_empty() { + error!("`any()` must be empty"); } } else { - expected_error(); + error!( + "`values()` arguments must be string literals or `any()`" + ); } - } else { - expected_error(); } + } else { + error!( + "`cfg()` arguments must be simple identifiers, `any()` or `values(...)`" + ); } - Ok(..) => expected_error(), - Err(err) => { - err.cancel(); - expected_error(); + } + + 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_any_specified { + error!("`cfg(any())` can only be provided in isolation"); + } + + 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()) + } + }); } - }, - Err(errs) => { - drop(errs); - expected_error(); } + } else { + expected_error(); } } From 1ef96a9e06124ca3ce95f9ceb3be6f24d0e7154a Mon Sep 17 00:00:00 2001 From: Urgau Date: Fri, 20 Oct 2023 19:34:45 +0200 Subject: [PATCH 2/3] Fix residual (never merged) check-cfg syntax in doc --- src/doc/unstable-book/src/compiler-flags/check-cfg.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/doc/unstable-book/src/compiler-flags/check-cfg.md b/src/doc/unstable-book/src/compiler-flags/check-cfg.md index 7a3ef5e9e2be6..0e15c79076fa4 100644 --- a/src/doc/unstable-book/src/compiler-flags/check-cfg.md +++ b/src/doc/unstable-book/src/compiler-flags/check-cfg.md @@ -139,7 +139,7 @@ fn do_mumble_frotz() {} ```bash # This turns on checking for feature values, but not for condition names. -rustc --check-cfg 'configure(feature, values("zapping", "lasers"))' \ +rustc --check-cfg 'cfg(feature, values("zapping", "lasers"))' \ --check-cfg 'cfg(any())' \ --cfg 'feature="zapping"' -Z unstable-options ``` From 84a1a689ccc8eb6b8be48f3385bc9b0aeb5c71a6 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 21 Oct 2023 19:11:24 +0200 Subject: [PATCH 3/3] Better guard against wrong input with check-cfg any() --- compiler/rustc_interface/src/interface.rs | 10 +++++++--- tests/ui/check-cfg/invalid-arguments.any_values.stderr | 2 ++ tests/ui/check-cfg/invalid-arguments.rs | 4 +++- .../ui/check-cfg/invalid-arguments.unterminated.stderr | 2 ++ 4 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 tests/ui/check-cfg/invalid-arguments.any_values.stderr create mode 100644 tests/ui/check-cfg/invalid-arguments.unterminated.stderr diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 2218892538212..422c2fc65ad6a 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -311,11 +311,15 @@ pub fn parse_check_cfg(handler: &EarlyErrorHandler, specs: Vec) -> Check } if any_specified { - if !names.is_empty() || !values.is_empty() || values_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"); } - - check_cfg.exhaustive_names = false; } else { for name in names { check_cfg diff --git a/tests/ui/check-cfg/invalid-arguments.any_values.stderr b/tests/ui/check-cfg/invalid-arguments.any_values.stderr new file mode 100644 index 0000000000000..f9a9c4a6e1328 --- /dev/null +++ b/tests/ui/check-cfg/invalid-arguments.any_values.stderr @@ -0,0 +1,2 @@ +error: invalid `--check-cfg` argument: `cfg(any(),values())` (`values()` cannot be specified before the names) + diff --git a/tests/ui/check-cfg/invalid-arguments.rs b/tests/ui/check-cfg/invalid-arguments.rs index 79bef89c95740..a56f48e0af93d 100644 --- a/tests/ui/check-cfg/invalid-arguments.rs +++ b/tests/ui/check-cfg/invalid-arguments.rs @@ -6,7 +6,7 @@ // revisions: multiple_values_any not_empty_any not_empty_values_any // revisions: values_any_missing_values values_any_before_ident ident_in_values_1 // revisions: ident_in_values_2 unknown_meta_item_1 unknown_meta_item_2 unknown_meta_item_3 -// revisions: mixed_values_any mixed_any giberich +// revisions: mixed_values_any mixed_any any_values giberich unterminated // // compile-flags: -Z unstable-options // [anything_else]compile-flags: --check-cfg=anything_else(...) @@ -29,6 +29,8 @@ // [unknown_meta_item_3]compile-flags: --check-cfg=cfg(foo,values(test())) // [mixed_values_any]compile-flags: --check-cfg=cfg(foo,values("bar",any())) // [mixed_any]compile-flags: --check-cfg=cfg(any(),values(any())) +// [any_values]compile-flags: --check-cfg=cfg(any(),values()) // [giberich]compile-flags: --check-cfg=cfg(...) +// [unterminated]compile-flags: --check-cfg=cfg( fn main() {} diff --git a/tests/ui/check-cfg/invalid-arguments.unterminated.stderr b/tests/ui/check-cfg/invalid-arguments.unterminated.stderr new file mode 100644 index 0000000000000..80161a6aa0fc8 --- /dev/null +++ b/tests/ui/check-cfg/invalid-arguments.unterminated.stderr @@ -0,0 +1,2 @@ +error: invalid `--check-cfg` argument: `cfg(` (expected `cfg(name, values("value1", "value2", ... "valueN"))`) +