From da896d35f471a27eb7b9385d6eadf50a5f761265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20BRANSTETT?= Date: Sat, 19 Feb 2022 23:06:11 +0100 Subject: [PATCH 1/5] Improve CheckCfg internal representation --- compiler/rustc_attr/src/builtin.rs | 31 ++++++++------- compiler/rustc_interface/src/interface.rs | 22 ++++++++--- compiler/rustc_session/src/config.rs | 48 ++++++++++++----------- 3 files changed, 58 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 49043e9f5f9d6..cd2e150a1907d 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -463,27 +463,30 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat MetaItemKind::NameValue(..) | MetaItemKind::Word => { let name = cfg.ident().expect("multi-segment cfg predicate").name; let value = cfg.value_str(); - if sess.check_config.names_checked && !sess.check_config.names_valid.contains(&name) - { - sess.buffer_lint( - UNEXPECTED_CFGS, - cfg.span, - CRATE_NODE_ID, - "unexpected `cfg` condition name", - ); - } - if let Some(val) = value { - if sess.check_config.values_checked.contains(&name) - && !sess.check_config.values_valid.contains(&(name, val)) - { + if let Some(names_valid) = &sess.check_config.names_valid { + if !names_valid.contains(&name) { sess.buffer_lint( UNEXPECTED_CFGS, cfg.span, CRATE_NODE_ID, - "unexpected `cfg` condition value", + "unexpected `cfg` condition name", ); } } + if let Some(val) = value { + if let Some(values_valid) = &sess.check_config.values_valid { + if let Some(values) = values_valid.get(&name) { + if !values.contains(&val) { + sess.buffer_lint( + UNEXPECTED_CFGS, + cfg.span, + CRATE_NODE_ID, + "unexpected `cfg` condition value", + ); + } + } + } + } sess.config.contains(&(name, value)) } } diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 609fc4b78c0de..5e0d59bf1b136 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -169,11 +169,12 @@ pub fn parse_check_cfg(specs: Vec) -> CheckCfg { Ok(meta_item) if parser.token == token::Eof => { if let Some(args) = meta_item.meta_item_list() { if meta_item.has_name(sym::names) { - cfg.names_checked = true; + let names_valid = + cfg.names_valid.get_or_insert_with(|| FxHashSet::default()); for arg in args { if arg.is_word() && arg.ident().is_some() { let ident = arg.ident().expect("multi-segment cfg key"); - cfg.names_valid.insert(ident.name.to_string()); + names_valid.insert(ident.name.to_string()); } else { error!("`names()` arguments must be simple identifers"); } @@ -182,14 +183,19 @@ pub fn parse_check_cfg(specs: Vec) -> CheckCfg { } else if meta_item.has_name(sym::values) { if let Some((name, values)) = args.split_first() { if name.is_word() && name.ident().is_some() { + let values_valid = cfg + .values_valid + .get_or_insert_with(|| FxHashMap::default()); let ident = name.ident().expect("multi-segment cfg key"); - cfg.values_checked.insert(ident.to_string()); + let ident_values = values_valid + .entry(ident.to_string()) + .or_insert_with(|| FxHashSet::default()); + for val in values { if let Some(LitKind::Str(s, _)) = val.literal().map(|lit| &lit.kind) { - cfg.values_valid - .insert((ident.to_string(), s.to_string())); + ident_values.insert(s.to_string()); } else { error!( "`values()` arguments must be string literals" @@ -219,7 +225,11 @@ pub fn parse_check_cfg(specs: Vec) -> CheckCfg { ); } - cfg.names_valid.extend(cfg.values_checked.iter().cloned()); + if let Some(values_valid) = &cfg.values_valid { + if let Some(names_valid) = &mut cfg.names_valid { + names_valid.extend(values_valid.keys().cloned()); + } + } cfg }) } diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 7a0d9a212c9d9..e0a3cc78b1772 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -8,7 +8,7 @@ use crate::search_paths::SearchPath; use crate::utils::{CanonicalizedPath, NativeLib, NativeLibKind}; use crate::{early_error, early_warn, Session}; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::impl_stable_hash_via_hash; use rustc_target::abi::{Align, TargetDataLayout}; @@ -1023,34 +1023,28 @@ pub fn to_crate_config(cfg: FxHashSet<(String, Option)>) -> CrateConfig /// The parsed `--check-cfg` options pub struct CheckCfg { - /// Set if `names()` checking is enabled - pub names_checked: bool, - /// The union of all `names()` - pub names_valid: FxHashSet, - /// The set of names for which `values()` was used - pub values_checked: FxHashSet, - /// The set of all (name, value) pairs passed in `values()` - pub values_valid: FxHashSet<(T, T)>, + /// The set of all `names()`, if none no names checking is performed + pub names_valid: Option>, + /// The set of all `values()`, if none no values chcking is performed + pub values_valid: Option>>, } impl Default for CheckCfg { fn default() -> Self { - CheckCfg { - names_checked: false, - names_valid: FxHashSet::default(), - values_checked: FxHashSet::default(), - values_valid: FxHashSet::default(), - } + CheckCfg { names_valid: Default::default(), values_valid: Default::default() } } } impl CheckCfg { fn map_data(&self, f: impl Fn(&T) -> O) -> CheckCfg { CheckCfg { - names_checked: self.names_checked, - names_valid: self.names_valid.iter().map(|a| f(a)).collect(), - values_checked: self.values_checked.iter().map(|a| f(a)).collect(), - values_valid: self.values_valid.iter().map(|(a, b)| (f(a), f(b))).collect(), + names_valid: self + .names_valid + .as_ref() + .map(|names_valid| names_valid.iter().map(|a| f(a)).collect()), + values_valid: self.values_valid.as_ref().map(|values_valid| { + values_valid.iter().map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect())).collect() + }), } } } @@ -1090,17 +1084,25 @@ impl CrateCheckConfig { sym::doctest, sym::feature, ]; - for &name in WELL_KNOWN_NAMES { - self.names_valid.insert(name); + if let Some(names_valid) = &mut self.names_valid { + for &name in WELL_KNOWN_NAMES { + names_valid.insert(name); + } } } /// Fills a `CrateCheckConfig` with configuration names and values that are actually active. pub fn fill_actual(&mut self, cfg: &CrateConfig) { for &(k, v) in cfg { - self.names_valid.insert(k); + if let Some(names_valid) = &mut self.names_valid { + names_valid.insert(k); + } if let Some(v) = v { - self.values_valid.insert((k, v)); + if let Some(values_valid) = &mut self.values_valid { + values_valid.entry(k).and_modify(|values| { + values.insert(v); + }); + } } } } From fbe1c153ecc88651a147517a6c1aea3da6aca8fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20BRANSTETT?= Date: Sun, 20 Feb 2022 00:04:10 +0100 Subject: [PATCH 2/5] Add test for well known names defined by rustc --- src/test/ui/check-cfg/well-known-names.rs | 27 +++++++++++++++++++ src/test/ui/check-cfg/well-known-names.stderr | 22 +++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 src/test/ui/check-cfg/well-known-names.rs create mode 100644 src/test/ui/check-cfg/well-known-names.stderr diff --git a/src/test/ui/check-cfg/well-known-names.rs b/src/test/ui/check-cfg/well-known-names.rs new file mode 100644 index 0000000000000..a66568a2ffdc9 --- /dev/null +++ b/src/test/ui/check-cfg/well-known-names.rs @@ -0,0 +1,27 @@ +// This test checks that we lint on non well known names and that we don't lint on well known names +// +// check-pass +// compile-flags: --check-cfg=names() -Z unstable-options + +#[cfg(target_oz = "linux")] +//~^ WARNING unexpected `cfg` condition name +fn target_os_misspell() {} + +#[cfg(target_os = "linux")] +fn target_os() {} + +#[cfg(features = "foo")] +//~^ WARNING unexpected `cfg` condition name +fn feature_misspell() {} + +#[cfg(feature = "foo")] +fn feature() {} + +#[cfg(uniw)] +//~^ WARNING unexpected `cfg` condition name +fn unix_misspell() {} + +#[cfg(unix)] +fn unix() {} + +fn main() {} diff --git a/src/test/ui/check-cfg/well-known-names.stderr b/src/test/ui/check-cfg/well-known-names.stderr new file mode 100644 index 0000000000000..a6b9a77dc8d9a --- /dev/null +++ b/src/test/ui/check-cfg/well-known-names.stderr @@ -0,0 +1,22 @@ +warning: unexpected `cfg` condition name + --> $DIR/well-known-names.rs:6:7 + | +LL | #[cfg(target_oz = "linux")] + | ^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(unexpected_cfgs)]` on by default + +warning: unexpected `cfg` condition name + --> $DIR/well-known-names.rs:13:7 + | +LL | #[cfg(features = "foo")] + | ^^^^^^^^^^^^^^^^ + +warning: unexpected `cfg` condition name + --> $DIR/well-known-names.rs:20:7 + | +LL | #[cfg(uniw)] + | ^^^^ + +warning: 3 warnings emitted + From 3d234770b1c83840d18305e8625da38e97da3174 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20BRANSTETT?= Date: Sun, 20 Feb 2022 00:48:10 +0100 Subject: [PATCH 3/5] Improve diagnostic of the unexpected_cfgs lint --- compiler/rustc_attr/src/builtin.rs | 15 ++++++-- compiler/rustc_lint/src/context.rs | 34 ++++++++++++++++++- compiler/rustc_lint/src/lib.rs | 1 + compiler/rustc_lint_defs/src/lib.rs | 1 + src/test/ui/check-cfg/invalid-cfg-name.stderr | 2 +- .../ui/check-cfg/invalid-cfg-value.stderr | 1 + src/test/ui/check-cfg/well-known-names.stderr | 10 ++++-- 7 files changed, 57 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index cd2e150a1907d..503e172b6873f 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -8,6 +8,7 @@ use rustc_errors::{struct_span_err, Applicability}; use rustc_feature::{find_gated_cfg, is_builtin_attr_name, Features, GatedCfg}; use rustc_macros::HashStable_Generic; use rustc_session::lint::builtin::UNEXPECTED_CFGS; +use rustc_session::lint::BuiltinLintDiagnostics; use rustc_session::parse::{feature_err, ParseSess}; use rustc_session::Session; use rustc_span::hygiene::Transparency; @@ -465,11 +466,16 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat let value = cfg.value_str(); if let Some(names_valid) = &sess.check_config.names_valid { if !names_valid.contains(&name) { - sess.buffer_lint( + sess.buffer_lint_with_diagnostic( UNEXPECTED_CFGS, cfg.span, CRATE_NODE_ID, "unexpected `cfg` condition name", + BuiltinLintDiagnostics::UnexpectedCfg( + cfg.ident().unwrap().span, + name, + None, + ), ); } } @@ -477,11 +483,16 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat if let Some(values_valid) = &sess.check_config.values_valid { if let Some(values) = values_valid.get(&name) { if !values.contains(&val) { - sess.buffer_lint( + sess.buffer_lint_with_diagnostic( UNEXPECTED_CFGS, cfg.span, CRATE_NODE_ID, "unexpected `cfg` condition value", + BuiltinLintDiagnostics::UnexpectedCfg( + cfg.name_value_literal_span().unwrap(), + name, + Some(val), + ), ); } } diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index d2d853efda2d2..72a3f5d5fc9b8 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -766,7 +766,39 @@ pub trait LintContext: Sized { BuiltinLintDiagnostics::NamedAsmLabel(help) => { db.help(&help); db.note("see the asm section of Rust By Example for more information"); - } + }, + BuiltinLintDiagnostics::UnexpectedCfg(span, name, value) => { + let mut possibilities: Vec = if value.is_some() { + let Some(values_valid) = &sess.parse_sess.check_config.values_valid else { + bug!("it shouldn't be possible to have a diagnostic on a value if values checking is not enable"); + }; + let Some(values) = values_valid.get(&name) else { + bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values"); + }; + values.iter().map(|&s| s).collect() + } else { + let Some(names_valid) = &sess.parse_sess.check_config.names_valid else { + bug!("it shouldn't be possible to have a diagnostic on a value if values checking is not enable"); + }; + names_valid.iter().map(|s| *s).collect() + }; + + // Show the full list if all possible values for a given name, but don't do it + // for names as the possibilities could be very long + if value.is_some() { + // Sorting can take some time, so we only do it if required + possibilities.sort(); + + let possibilities = possibilities.iter().map(Symbol::as_str).intersperse(", ").collect::(); + db.note(&format!("possible values for `{name}` are: {possibilities}")); + } + + // Suggest the most probable if we found one + if let Some(best_match) = find_best_match_for_name(&possibilities, value.unwrap_or(name), None) { + let ponctuation = if value.is_some() { "\"" } else { "" }; + db.span_suggestion(span, "did you mean", format!("{ponctuation}{best_match}{ponctuation}"), Applicability::MaybeIncorrect); + } + }, } // Rewrap `db`, and pass control to the user. decorate(LintDiagnosticBuilder::new(db)); diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 69863b5ff827f..7182022d25298 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -31,6 +31,7 @@ #![feature(box_patterns)] #![feature(crate_visibility_modifier)] #![feature(if_let_guard)] +#![feature(iter_intersperse)] #![feature(iter_order_by)] #![feature(let_else)] #![feature(never_type)] diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 1f834b7212fe5..e9c62fc400651 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -310,6 +310,7 @@ pub enum BuiltinLintDiagnostics { BreakWithLabelAndLoop(Span), NamedAsmLabel(String), UnicodeTextFlow(Span, String), + UnexpectedCfg(Span, Symbol, Option), } /// Lints that are buffered up early on in the `Session` before the diff --git a/src/test/ui/check-cfg/invalid-cfg-name.stderr b/src/test/ui/check-cfg/invalid-cfg-name.stderr index 2587685afa048..2bd1821c9422b 100644 --- a/src/test/ui/check-cfg/invalid-cfg-name.stderr +++ b/src/test/ui/check-cfg/invalid-cfg-name.stderr @@ -2,7 +2,7 @@ warning: unexpected `cfg` condition name --> $DIR/invalid-cfg-name.rs:7:7 | LL | #[cfg(widnows)] - | ^^^^^^^ + | ^^^^^^^ help: did you mean: `windows` | = note: `#[warn(unexpected_cfgs)]` on by default diff --git a/src/test/ui/check-cfg/invalid-cfg-value.stderr b/src/test/ui/check-cfg/invalid-cfg-value.stderr index c591d8474a261..23fd5c8c759cf 100644 --- a/src/test/ui/check-cfg/invalid-cfg-value.stderr +++ b/src/test/ui/check-cfg/invalid-cfg-value.stderr @@ -5,6 +5,7 @@ LL | #[cfg(feature = "sedre")] | ^^^^^^^^^^^^^^^^^ | = note: `#[warn(unexpected_cfgs)]` on by default + = note: possible values for `feature` are: rand, serde, full warning: 1 warning emitted diff --git a/src/test/ui/check-cfg/well-known-names.stderr b/src/test/ui/check-cfg/well-known-names.stderr index a6b9a77dc8d9a..bdbe4d29d30fe 100644 --- a/src/test/ui/check-cfg/well-known-names.stderr +++ b/src/test/ui/check-cfg/well-known-names.stderr @@ -2,7 +2,9 @@ warning: unexpected `cfg` condition name --> $DIR/well-known-names.rs:6:7 | LL | #[cfg(target_oz = "linux")] - | ^^^^^^^^^^^^^^^^^^^ + | ---------^^^^^^^^^^ + | | + | help: did you mean: `target_os` | = note: `#[warn(unexpected_cfgs)]` on by default @@ -10,13 +12,15 @@ warning: unexpected `cfg` condition name --> $DIR/well-known-names.rs:13:7 | LL | #[cfg(features = "foo")] - | ^^^^^^^^^^^^^^^^ + | --------^^^^^^^^ + | | + | help: did you mean: `feature` warning: unexpected `cfg` condition name --> $DIR/well-known-names.rs:20:7 | LL | #[cfg(uniw)] - | ^^^^ + | ^^^^ help: did you mean: `unix` warning: 3 warnings emitted From 8d3de56da1eba68d012977d4c743d5eaaa1baee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20BRANSTETT?= Date: Sun, 20 Feb 2022 01:26:52 +0100 Subject: [PATCH 4/5] Continue improvements on the --check-cfg implementation - Test the combinations of --check-cfg with partial values() and --cfg - Test that we detect unexpected value when none are expected --- compiler/rustc_attr/src/builtin.rs | 39 +++++------ compiler/rustc_interface/src/interface.rs | 14 ++-- compiler/rustc_lint/src/context.rs | 25 +++---- compiler/rustc_session/src/config.rs | 22 +++---- .../ui/check-cfg/invalid-cfg-value.stderr | 2 +- src/test/ui/check-cfg/mix.rs | 50 ++++++++++++++ src/test/ui/check-cfg/mix.stderr | 66 +++++++++++++++++++ src/test/ui/check-cfg/no-values.rs | 10 +++ src/test/ui/check-cfg/no-values.stderr | 11 ++++ 9 files changed, 184 insertions(+), 55 deletions(-) create mode 100644 src/test/ui/check-cfg/mix.rs create mode 100644 src/test/ui/check-cfg/mix.stderr create mode 100644 src/test/ui/check-cfg/no-values.rs create mode 100644 src/test/ui/check-cfg/no-values.stderr diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 503e172b6873f..50eb6b6e5da52 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -462,7 +462,8 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat true } MetaItemKind::NameValue(..) | MetaItemKind::Word => { - let name = cfg.ident().expect("multi-segment cfg predicate").name; + let ident = cfg.ident().expect("multi-segment cfg predicate"); + let name = ident.name; let value = cfg.value_str(); if let Some(names_valid) = &sess.check_config.names_valid { if !names_valid.contains(&name) { @@ -471,30 +472,24 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat cfg.span, CRATE_NODE_ID, "unexpected `cfg` condition name", - BuiltinLintDiagnostics::UnexpectedCfg( - cfg.ident().unwrap().span, - name, - None, - ), + BuiltinLintDiagnostics::UnexpectedCfg(ident.span, name, None), ); } } - if let Some(val) = value { - if let Some(values_valid) = &sess.check_config.values_valid { - if let Some(values) = values_valid.get(&name) { - if !values.contains(&val) { - sess.buffer_lint_with_diagnostic( - UNEXPECTED_CFGS, - cfg.span, - CRATE_NODE_ID, - "unexpected `cfg` condition value", - BuiltinLintDiagnostics::UnexpectedCfg( - cfg.name_value_literal_span().unwrap(), - name, - Some(val), - ), - ); - } + if let Some(value) = value { + if let Some(values) = &sess.check_config.values_valid.get(&name) { + if !values.contains(&value) { + sess.buffer_lint_with_diagnostic( + UNEXPECTED_CFGS, + cfg.span, + CRATE_NODE_ID, + "unexpected `cfg` condition value", + BuiltinLintDiagnostics::UnexpectedCfg( + cfg.name_value_literal_span().unwrap(), + name, + Some(value), + ), + ); } } } diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 5e0d59bf1b136..91ced2a2d90e2 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -183,12 +183,10 @@ pub fn parse_check_cfg(specs: Vec) -> CheckCfg { } else if meta_item.has_name(sym::values) { if let Some((name, values)) = args.split_first() { if name.is_word() && name.ident().is_some() { - let values_valid = cfg - .values_valid - .get_or_insert_with(|| FxHashMap::default()); let ident = name.ident().expect("multi-segment cfg key"); - let ident_values = values_valid - .entry(ident.to_string()) + let ident_values = cfg + .values_valid + .entry(ident.name.to_string()) .or_insert_with(|| FxHashSet::default()); for val in values { @@ -225,10 +223,8 @@ pub fn parse_check_cfg(specs: Vec) -> CheckCfg { ); } - if let Some(values_valid) = &cfg.values_valid { - if let Some(names_valid) = &mut cfg.names_valid { - names_valid.extend(values_valid.keys().cloned()); - } + if let Some(names_valid) = &mut cfg.names_valid { + names_valid.extend(cfg.values_valid.keys().cloned()); } cfg }) diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 72a3f5d5fc9b8..5f07cf08c2ea3 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -768,17 +768,14 @@ pub trait LintContext: Sized { db.note("see the asm section of Rust By Example for more information"); }, BuiltinLintDiagnostics::UnexpectedCfg(span, name, value) => { - let mut possibilities: Vec = if value.is_some() { - let Some(values_valid) = &sess.parse_sess.check_config.values_valid else { - bug!("it shouldn't be possible to have a diagnostic on a value if values checking is not enable"); - }; - let Some(values) = values_valid.get(&name) else { + let possibilities: Vec = if value.is_some() { + let Some(values) = &sess.parse_sess.check_config.values_valid.get(&name) else { bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values"); }; values.iter().map(|&s| s).collect() } else { let Some(names_valid) = &sess.parse_sess.check_config.names_valid else { - bug!("it shouldn't be possible to have a diagnostic on a value if values checking is not enable"); + bug!("it shouldn't be possible to have a diagnostic on a name if name checking is not enabled"); }; names_valid.iter().map(|s| *s).collect() }; @@ -786,17 +783,21 @@ pub trait LintContext: Sized { // Show the full list if all possible values for a given name, but don't do it // for names as the possibilities could be very long if value.is_some() { - // Sorting can take some time, so we only do it if required - possibilities.sort(); + if !possibilities.is_empty() { + let mut possibilities = possibilities.iter().map(Symbol::as_str).collect::>(); + possibilities.sort(); - let possibilities = possibilities.iter().map(Symbol::as_str).intersperse(", ").collect::(); - db.note(&format!("possible values for `{name}` are: {possibilities}")); + let possibilities = possibilities.join(", "); + db.note(&format!("expected values for `{name}` are: {possibilities}")); + } else { + db.note(&format!("no expected value for `{name}`")); + } } // Suggest the most probable if we found one if let Some(best_match) = find_best_match_for_name(&possibilities, value.unwrap_or(name), None) { - let ponctuation = if value.is_some() { "\"" } else { "" }; - db.span_suggestion(span, "did you mean", format!("{ponctuation}{best_match}{ponctuation}"), Applicability::MaybeIncorrect); + let punctuation = if value.is_some() { "\"" } else { "" }; + db.span_suggestion(span, "did you mean", format!("{punctuation}{best_match}{punctuation}"), Applicability::MaybeIncorrect); } }, } diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index e0a3cc78b1772..f9b75690e375f 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1023,10 +1023,10 @@ pub fn to_crate_config(cfg: FxHashSet<(String, Option)>) -> CrateConfig /// The parsed `--check-cfg` options pub struct CheckCfg { - /// The set of all `names()`, if none no names checking is performed + /// The set of all `names()`, if None no name checking is performed pub names_valid: Option>, - /// The set of all `values()`, if none no values chcking is performed - pub values_valid: Option>>, + /// The set of all `values()` + pub values_valid: FxHashMap>, } impl Default for CheckCfg { @@ -1042,9 +1042,11 @@ impl CheckCfg { .names_valid .as_ref() .map(|names_valid| names_valid.iter().map(|a| f(a)).collect()), - values_valid: self.values_valid.as_ref().map(|values_valid| { - values_valid.iter().map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect())).collect() - }), + values_valid: self + .values_valid + .iter() + .map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect())) + .collect(), } } } @@ -1098,11 +1100,9 @@ impl CrateCheckConfig { names_valid.insert(k); } if let Some(v) = v { - if let Some(values_valid) = &mut self.values_valid { - values_valid.entry(k).and_modify(|values| { - values.insert(v); - }); - } + self.values_valid.entry(k).and_modify(|values| { + values.insert(v); + }); } } } diff --git a/src/test/ui/check-cfg/invalid-cfg-value.stderr b/src/test/ui/check-cfg/invalid-cfg-value.stderr index 23fd5c8c759cf..bc2c053fed65a 100644 --- a/src/test/ui/check-cfg/invalid-cfg-value.stderr +++ b/src/test/ui/check-cfg/invalid-cfg-value.stderr @@ -5,7 +5,7 @@ LL | #[cfg(feature = "sedre")] | ^^^^^^^^^^^^^^^^^ | = note: `#[warn(unexpected_cfgs)]` on by default - = note: possible values for `feature` are: rand, serde, full + = note: expected values for `feature` are: full, rand, serde warning: 1 warning emitted diff --git a/src/test/ui/check-cfg/mix.rs b/src/test/ui/check-cfg/mix.rs new file mode 100644 index 0000000000000..26c735c4a10bd --- /dev/null +++ b/src/test/ui/check-cfg/mix.rs @@ -0,0 +1,50 @@ +// This test checks the combination of well known names, their activation via names(), the usage of +// partial values() with a --cfg and test that we also correctly lint on the `cfg!` macro and +// `cfg_attr` attribute. +// +// check-pass +// compile-flags: --check-cfg=names() --check-cfg=values(feature,"foo") --cfg feature="bar" -Z unstable-options + +#[cfg(windows)] +fn do_windows_stuff() {} + +#[cfg(widnows)] +//~^ WARNING unexpected `cfg` condition name +fn do_windows_stuff() {} + +#[cfg(feature = "foo")] +fn use_foo() {} + +#[cfg(feature = "bar")] +fn use_bar() {} + +#[cfg(feature = "zebra")] +//~^ WARNING unexpected `cfg` condition value +fn use_zebra() {} + +#[cfg_attr(uu, test)] +//~^ WARNING unexpected `cfg` condition name +fn do_test() {} + +#[cfg_attr(feature = "foo", no_mangle)] +fn do_test_foo() {} + +fn test_cfg_macro() { + cfg!(windows); + cfg!(widnows); + //~^ WARNING unexpected `cfg` condition name + cfg!(feature = "foo"); + cfg!(feature = "bar"); + cfg!(feature = "zebra"); + //~^ WARNING unexpected `cfg` condition value + cfg!(xxx = "foo"); + //~^ WARNING unexpected `cfg` condition name + cfg!(xxx); + //~^ WARNING unexpected `cfg` condition name + cfg!(any(xxx, windows)); + //~^ WARNING unexpected `cfg` condition name + cfg!(any(feature = "bad", windows)); + //~^ WARNING unexpected `cfg` condition value +} + +fn main() {} diff --git a/src/test/ui/check-cfg/mix.stderr b/src/test/ui/check-cfg/mix.stderr new file mode 100644 index 0000000000000..b273be774224d --- /dev/null +++ b/src/test/ui/check-cfg/mix.stderr @@ -0,0 +1,66 @@ +warning: unexpected `cfg` condition name + --> $DIR/mix.rs:11:7 + | +LL | #[cfg(widnows)] + | ^^^^^^^ help: did you mean: `windows` + | + = note: `#[warn(unexpected_cfgs)]` on by default + +warning: unexpected `cfg` condition value + --> $DIR/mix.rs:21:7 + | +LL | #[cfg(feature = "zebra")] + | ^^^^^^^^^^^^^^^^^ + | + = note: expected values for `feature` are: bar, foo + +warning: unexpected `cfg` condition name + --> $DIR/mix.rs:25:12 + | +LL | #[cfg_attr(uu, test)] + | ^^ + +warning: unexpected `cfg` condition name + --> $DIR/mix.rs:34:10 + | +LL | cfg!(widnows); + | ^^^^^^^ help: did you mean: `windows` + +warning: unexpected `cfg` condition value + --> $DIR/mix.rs:38:10 + | +LL | cfg!(feature = "zebra"); + | ^^^^^^^^^^^^^^^^^ + | + = note: expected values for `feature` are: bar, foo + +warning: unexpected `cfg` condition name + --> $DIR/mix.rs:40:10 + | +LL | cfg!(xxx = "foo"); + | ^^^^^^^^^^^ + +warning: unexpected `cfg` condition name + --> $DIR/mix.rs:42:10 + | +LL | cfg!(xxx); + | ^^^ + +warning: unexpected `cfg` condition name + --> $DIR/mix.rs:44:14 + | +LL | cfg!(any(xxx, windows)); + | ^^^ + +warning: unexpected `cfg` condition value + --> $DIR/mix.rs:46:14 + | +LL | cfg!(any(feature = "bad", windows)); + | ^^^^^^^^^^----- + | | + | help: did you mean: `"bar"` + | + = note: expected values for `feature` are: bar, foo + +warning: 9 warnings emitted + diff --git a/src/test/ui/check-cfg/no-values.rs b/src/test/ui/check-cfg/no-values.rs new file mode 100644 index 0000000000000..2440757e52da9 --- /dev/null +++ b/src/test/ui/check-cfg/no-values.rs @@ -0,0 +1,10 @@ +// Check that we detect unexpected value when none are allowed +// +// check-pass +// compile-flags: --check-cfg=values(feature) -Z unstable-options + +#[cfg(feature = "foo")] +//~^ WARNING unexpected `cfg` condition value +fn do_foo() {} + +fn main() {} diff --git a/src/test/ui/check-cfg/no-values.stderr b/src/test/ui/check-cfg/no-values.stderr new file mode 100644 index 0000000000000..ea1c9107d4c2f --- /dev/null +++ b/src/test/ui/check-cfg/no-values.stderr @@ -0,0 +1,11 @@ +warning: unexpected `cfg` condition value + --> $DIR/no-values.rs:6:7 + | +LL | #[cfg(feature = "foo")] + | ^^^^^^^^^^^^^^^ + | + = note: `#[warn(unexpected_cfgs)]` on by default + = note: no expected value for `feature` + +warning: 1 warning emitted + From a556a2a8e60501203f310407b27cf739618c0000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20BRANSTETT?= Date: Mon, 21 Feb 2022 15:29:04 +0100 Subject: [PATCH 5/5] Add compiler flag `--check-cfg` to the unstable book --- .../src/compiler-flags/check-cfg.md | 221 ++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 src/doc/unstable-book/src/compiler-flags/check-cfg.md diff --git a/src/doc/unstable-book/src/compiler-flags/check-cfg.md b/src/doc/unstable-book/src/compiler-flags/check-cfg.md new file mode 100644 index 0000000000000..d7345ad0c33f2 --- /dev/null +++ b/src/doc/unstable-book/src/compiler-flags/check-cfg.md @@ -0,0 +1,221 @@ +# `check-cfg` + +The tracking issue for this feature is: [#82450](https://github.com/rust-lang/rust/issues/82450). + +------------------------ + +This feature allows you to enable complete or partial checking of configuration. + +`rustc` accepts the `--check-cfg` option, which specifies whether to check conditions and how to +check them. The `--check-cfg` option takes a value, called the _check cfg specification_. The +check cfg specification is parsed using the Rust metadata syntax, just as the `--cfg` option is. + +`--check-cfg` option can take one of two forms: + +1. `--check-cfg names(...)` enables checking condition names. +2. `--check-cfg values(...)` enables checking the values within list-valued conditions. + +These two options are independent. `names` checks only the namespace of condition names +while `values` checks only the namespace of the values of list-valued conditions. + +## The `names(...)` form + +The `names(...)` form enables checking the names. This form uses a named list: + +```bash +rustc --check-cfg 'names(name1, name2, ... nameN)' +``` + +where each `name` is a bare identifier (has no quotes). The order of the names is not significant. + +If `--check-cfg names(...)` is specified at least once, then `rustc` will check all references to +condition names. `rustc` will check every `#[cfg]` attribute, `#[cfg_attr]` attribute, `cfg` clause +inside `#[link]` attribute and `cfg!(...)` call against the provided list of expected condition +names. If a name is not present in this list, then `rustc` will report an `unexpected_cfgs` lint +diagnostic. The default diagnostic level for this lint is `Warn`. + +If `--check-cfg names(...)` is not specified, then `rustc` will not check references to condition +names. + +`--check-cfg names(...)` may be specified more than once. The result is that the list of valid +condition names is merged across all options. It is legal for a condition name to be specified +more than once; redundantly specifying a condition name has no effect. + +To enable checking condition names with an empty set of valid condition names, use the following +form. The parentheses are required. + +```bash +rustc --check-cfg 'names()' +``` + +Note that `--check-cfg 'names()'` is _not_ equivalent to omitting the option entirely. +The first form enables checking condition names, while specifying that there are no valid +condition names (outside of the set of well-known names defined by `rustc`). Omitting the +`--check-cfg 'names(...)'` option does not enable checking condition names. + +Conditions that are enabled are implicitly valid; it is unnecessary (but legal) to specify a +condition name as both enabled and valid. For example, the following invocations are equivalent: + +```bash +# condition names will be checked, and 'has_time_travel' is valid +rustc --cfg 'has_time_travel' --check-cfg 'names()' + +# condition names will be checked, and 'has_time_travel' is valid +rustc --cfg 'has_time_travel' --check-cfg 'names(has_time_travel)' +``` + +In contrast, the following two invocations are _not_ equivalent: + +```bash +# condition names will not be checked (because there is no --check-cfg names(...)) +rustc --cfg 'has_time_travel' + +# condition names will be checked, and 'has_time_travel' is both valid and enabled. +rustc --cfg 'has_time_travel' --check-cfg 'names(has_time_travel)' +``` + +## The `values(...)` form + +The `values(...)` form enables checking the values within list-valued conditions. It has this +form: + +```bash +rustc --check-cfg `values(name, "value1", "value2", ... "valueN")' +``` + +where `name` is a bare identifier (has no quotes) and each `"value"` term is a quoted literal +string. `name` specifies the name of the condition, such as `feature` or `target_os`. + +When the `values(...)` option is specified, `rustc` will check every `#[cfg(name = "value")]` +attribute, `#[cfg_attr(name = "value")]` attribute, `#[link(name = "a", cfg(name = "value"))]` +and `cfg!(name = "value")` call. It will check that the `"value"` specified is present in the +list of expected values. If `"value"` is not in it, then `rustc` will report an `unexpected_cfgs` +lint diagnostic. The default diagnostic level for this lint is `Warn`. + +The form `values()` is an error, because it does not specify a condition name. + +To enable checking of values, but to provide an empty set of valid values, use this form: + +```bash +rustc --check-cfg `values(name)` +``` + +The `--check-cfg values(...)` option can be repeated, both for the same condition name and for +different names. If it is repeated for the same condition name, then the sets of values for that +condition are merged together. + +## Examples + +Consider this command line: + +```bash +rustc --check-cfg 'names(feature)' \ + --check-cfg 'values(feature,"lion","zebra")' \ + --cfg 'feature="lion"' -Z unstable-options \ + example.rs +``` + +This command line indicates that this crate has two features: `lion` and `zebra`. The `lion` +feature is enabled, while the `zebra` feature is disabled. Consider compiling this code: + +```rust +// This is expected, and tame_lion() will be compiled +#[cfg(feature = "lion")] +fn tame_lion(lion: Lion) {} + +// This is expected, and ride_zebra() will NOT be compiled. +#[cfg(feature = "zebra")] +fn ride_zebra(zebra: Zebra) {} + +// This is UNEXPECTED, and will cause a compiler warning (by default). +#[cfg(feature = "platypus")] +fn poke_platypus() {} + +// This is UNEXPECTED, because 'feechure' is not a known condition name, +// and will cause a compiler warning (by default). +#[cfg(feechure = "lion")] +fn tame_lion() {} +``` + +> Note: The `--check-cfg names(feature)` option is necessary only to enable checking the condition +> name, as in the last example. `feature` is a well-known (always-expected) condition name, and so +> it is not necessary to specify it in a `--check-cfg 'names(...)'` option. That option can be +> shortened to > `--check-cfg names()` in order to enable checking well-known condition names. + +### Example: Checking condition names, but not values + +```bash +# This turns on checking for condition names, but not values, such as 'feature' values. +rustc --check-cfg 'names(is_embedded, has_feathers)' \ + --cfg has_feathers --cfg 'feature = "zapping"' -Z unstable-options +``` + +```rust +#[cfg(is_embedded)] // This is expected as "is_embedded" was provided in names() +fn do_embedded() {} + +#[cfg(has_feathers)] // This is expected as "has_feathers" was provided in names() +fn do_features() {} + +#[cfg(has_mumble_frotz)] // This is UNEXPECTED because names checking is enable and + // "has_mumble_frotz" was not provided in names() +fn do_mumble_frotz() {} + +#[cfg(feature = "lasers")] // This doesn't raise a warning, because values checking for "feature" + // was never used +fn shoot_lasers() {} +``` + +### Example: Checking feature values, but not condition names + +```bash +# This turns on checking for feature values, but not for condition names. +rustc --check-cfg 'values(feature, "zapping", "lasers")' \ + --cfg 'feature="zapping"' -Z unstable-options +``` + +```rust +#[cfg(is_embedded)] // This is doesn't raise a warning, because names checking was not + // enable (ie not names()) +fn do_embedded() {} + +#[cfg(has_feathers)] // Same as above, --check-cfg names(...) was never used so no name + // checking is performed +fn do_features() {} + + +#[cfg(feature = "lasers")] // This is expected, "lasers" is in the values(feature) list +fn shoot_lasers() {} + +#[cfg(feature = "monkeys")] // This is UNEXPECTED, because "monkeys" is not in the + // --check-cfg values(feature) list +fn write_shakespeare() {} +``` + +### Example: Checking both condition names and feature values + +```bash +# This turns on checking for feature values and for condition names. +rustc --check-cfg 'names(is_embedded, has_feathers)' \ + --check-cfg 'values(feature, "zapping", "lasers")' \ + --cfg has_feathers --cfg 'feature="zapping"' -Z unstable-options +``` + +```rust +#[cfg(is_embedded)] // This is expected because "is_embedded" was provided in names() +fn do_embedded() {} + +#[cfg(has_feathers)] // This is expected because "has_feathers" was provided in names() +fn do_features() {} + +#[cfg(has_mumble_frotz)] // This is UNEXPECTED, because has_mumble_frotz is not in the + // --check-cfg names(...) list +fn do_mumble_frotz() {} + +#[cfg(feature = "lasers")] // This is expected, "lasers" is in the values(feature) list +fn shoot_lasers() {} + +#[cfg(feature = "monkeys")] // This is UNEXPECTED, because "monkeys" is not in + // the values(feature) list +fn write_shakespear() {} +```