Skip to content

Commit 92b5d1d

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#94175 - Urgau:check-cfg-improvements, r=petrochenkov
Improve `--check-cfg` implementation This pull-request is a mix of improvements regarding the `--check-cfg` implementation: - Simpler internal representation (usage of `Option` instead of separate bool) - Add --check-cfg to the unstable book (based on the RFC) - Improved diagnostics: * List possible values when the value is unexpected * Suggest if possible a name or value that is similar - Add more tests (well known names, mix of combinations, ...) r? `@petrochenkov`
2 parents b33098f + a556a2a commit 92b5d1d

File tree

15 files changed

+511
-47
lines changed

15 files changed

+511
-47
lines changed

‎compiler/rustc_attr/src/builtin.rs‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use rustc_errors::{struct_span_err, Applicability};
88
use rustc_feature::{find_gated_cfg, is_builtin_attr_name, Features, GatedCfg};
99
use rustc_macros::HashStable_Generic;
1010
use rustc_session::lint::builtin::UNEXPECTED_CFGS;
11+
use rustc_session::lint::BuiltinLintDiagnostics;
1112
use rustc_session::parse::{feature_err, ParseSess};
1213
use rustc_session::Session;
1314
use rustc_span::hygiene::Transparency;
@@ -461,29 +462,37 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
461462
true
462463
}
463464
MetaItemKind::NameValue(..) | MetaItemKind::Word => {
464-
let name = cfg.ident().expect("multi-segment cfg predicate").name;
465+
let ident = cfg.ident().expect("multi-segment cfg predicate");
466+
let name = ident.name;
465467
let value = cfg.value_str();
466-
if sess.check_config.names_checked && !sess.check_config.names_valid.contains(&name)
467-
{
468-
sess.buffer_lint(
469-
UNEXPECTED_CFGS,
470-
cfg.span,
471-
CRATE_NODE_ID,
472-
"unexpected `cfg` condition name",
473-
);
474-
}
475-
if let Some(val) = value {
476-
if sess.check_config.values_checked.contains(&name)
477-
&& !sess.check_config.values_valid.contains(&(name, val))
478-
{
479-
sess.buffer_lint(
468+
if let Some(names_valid) = &sess.check_config.names_valid {
469+
if !names_valid.contains(&name) {
470+
sess.buffer_lint_with_diagnostic(
480471
UNEXPECTED_CFGS,
481472
cfg.span,
482473
CRATE_NODE_ID,
483-
"unexpected `cfg` condition value",
474+
"unexpected `cfg` condition name",
475+
BuiltinLintDiagnostics::UnexpectedCfg(ident.span, name, None),
484476
);
485477
}
486478
}
479+
if let Some(value) = value {
480+
if let Some(values) = &sess.check_config.values_valid.get(&name) {
481+
if !values.contains(&value) {
482+
sess.buffer_lint_with_diagnostic(
483+
UNEXPECTED_CFGS,
484+
cfg.span,
485+
CRATE_NODE_ID,
486+
"unexpected `cfg` condition value",
487+
BuiltinLintDiagnostics::UnexpectedCfg(
488+
cfg.name_value_literal_span().unwrap(),
489+
name,
490+
Some(value),
491+
),
492+
);
493+
}
494+
}
495+
}
487496
sess.config.contains(&(name, value))
488497
}
489498
}

‎compiler/rustc_interface/src/interface.rs‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,12 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
169169
Ok(meta_item) if parser.token == token::Eof => {
170170
if let Some(args) = meta_item.meta_item_list() {
171171
if meta_item.has_name(sym::names) {
172-
cfg.names_checked = true;
172+
let names_valid =
173+
cfg.names_valid.get_or_insert_with(|| FxHashSet::default());
173174
for arg in args {
174175
if arg.is_word() && arg.ident().is_some() {
175176
let ident = arg.ident().expect("multi-segment cfg key");
176-
cfg.names_valid.insert(ident.name.to_string());
177+
names_valid.insert(ident.name.to_string());
177178
} else {
178179
error!("`names()` arguments must be simple identifers");
179180
}
@@ -183,13 +184,16 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
183184
if let Some((name, values)) = args.split_first() {
184185
if name.is_word() && name.ident().is_some() {
185186
let ident = name.ident().expect("multi-segment cfg key");
186-
cfg.values_checked.insert(ident.to_string());
187+
let ident_values = cfg
188+
.values_valid
189+
.entry(ident.name.to_string())
190+
.or_insert_with(|| FxHashSet::default());
191+
187192
for val in values {
188193
if let Some(LitKind::Str(s, _)) =
189194
val.literal().map(|lit| &lit.kind)
190195
{
191-
cfg.values_valid
192-
.insert((ident.to_string(), s.to_string()));
196+
ident_values.insert(s.to_string());
193197
} else {
194198
error!(
195199
"`values()` arguments must be string literals"
@@ -219,7 +223,9 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
219223
);
220224
}
221225

222-
cfg.names_valid.extend(cfg.values_checked.iter().cloned());
226+
if let Some(names_valid) = &mut cfg.names_valid {
227+
names_valid.extend(cfg.values_valid.keys().cloned());
228+
}
223229
cfg
224230
})
225231
}

‎compiler/rustc_lint/src/context.rs‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,40 @@ pub trait LintContext: Sized {
765765
BuiltinLintDiagnostics::NamedAsmLabel(help) => {
766766
db.help(&help);
767767
db.note("see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information");
768-
}
768+
},
769+
BuiltinLintDiagnostics::UnexpectedCfg(span, name, value) => {
770+
let possibilities: Vec<Symbol> = if value.is_some() {
771+
let Some(values) = &sess.parse_sess.check_config.values_valid.get(&name) else {
772+
bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values");
773+
};
774+
values.iter().map(|&s| s).collect()
775+
} else {
776+
let Some(names_valid) = &sess.parse_sess.check_config.names_valid else {
777+
bug!("it shouldn't be possible to have a diagnostic on a name if name checking is not enabled");
778+
};
779+
names_valid.iter().map(|s| *s).collect()
780+
};
781+
782+
// Show the full list if all possible values for a given name, but don't do it
783+
// for names as the possibilities could be very long
784+
if value.is_some() {
785+
if !possibilities.is_empty() {
786+
let mut possibilities = possibilities.iter().map(Symbol::as_str).collect::<Vec<_>>();
787+
possibilities.sort();
788+
789+
let possibilities = possibilities.join(", ");
790+
db.note(&format!("expected values for `{name}` are: {possibilities}"));
791+
} else {
792+
db.note(&format!("no expected value for `{name}`"));
793+
}
794+
}
795+
796+
// Suggest the most probable if we found one
797+
if let Some(best_match) = find_best_match_for_name(&possibilities, value.unwrap_or(name), None) {
798+
let punctuation = if value.is_some() { "\"" } else { "" };
799+
db.span_suggestion(span, "did you mean", format!("{punctuation}{best_match}{punctuation}"), Applicability::MaybeIncorrect);
800+
}
801+
},
769802
}
770803
// Rewrap `db`, and pass control to the user.
771804
decorate(LintDiagnosticBuilder::new(db));

‎compiler/rustc_lint/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#![feature(box_patterns)]
3232
#![feature(crate_visibility_modifier)]
3333
#![feature(if_let_guard)]
34+
#![feature(iter_intersperse)]
3435
#![feature(iter_order_by)]
3536
#![feature(let_else)]
3637
#![feature(never_type)]

‎compiler/rustc_lint_defs/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ pub enum BuiltinLintDiagnostics {
310310
BreakWithLabelAndLoop(Span),
311311
NamedAsmLabel(String),
312312
UnicodeTextFlow(Span, String),
313+
UnexpectedCfg(Span, Symbol, Option<Symbol>),
313314
}
314315

315316
/// Lints that are buffered up early on in the `Session` before the

‎compiler/rustc_session/src/config.rs‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::search_paths::SearchPath;
88
use crate::utils::{CanonicalizedPath, NativeLib, NativeLibKind};
99
use crate::{early_error, early_warn, Session};
1010

11-
use rustc_data_structures::fx::FxHashSet;
11+
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1212
use rustc_data_structures::impl_stable_hash_via_hash;
1313

1414
use rustc_target::abi::{Align, TargetDataLayout};
@@ -1023,34 +1023,30 @@ pub fn to_crate_config(cfg: FxHashSet<(String, Option<String>)>) -> CrateConfig
10231023

10241024
/// The parsed `--check-cfg` options
10251025
pub struct CheckCfg<T = String> {
1026-
/// Set if `names()` checking is enabled
1027-
pub names_checked: bool,
1028-
/// The union of all `names()`
1029-
pub names_valid: FxHashSet<T>,
1030-
/// The set of names for which `values()` was used
1031-
pub values_checked: FxHashSet<T>,
1032-
/// The set of all (name, value) pairs passed in `values()`
1033-
pub values_valid: FxHashSet<(T, T)>,
1026+
/// The set of all `names()`, if None no name checking is performed
1027+
pub names_valid: Option<FxHashSet<T>>,
1028+
/// The set of all `values()`
1029+
pub values_valid: FxHashMap<T, FxHashSet<T>>,
10341030
}
10351031

10361032
impl<T> Default for CheckCfg<T> {
10371033
fn default() -> Self {
1038-
CheckCfg {
1039-
names_checked: false,
1040-
names_valid: FxHashSet::default(),
1041-
values_checked: FxHashSet::default(),
1042-
values_valid: FxHashSet::default(),
1043-
}
1034+
CheckCfg { names_valid: Default::default(), values_valid: Default::default() }
10441035
}
10451036
}
10461037

10471038
impl<T> CheckCfg<T> {
10481039
fn map_data<O: Eq + Hash>(&self, f: impl Fn(&T) -> O) -> CheckCfg<O> {
10491040
CheckCfg {
1050-
names_checked: self.names_checked,
1051-
names_valid: self.names_valid.iter().map(|a| f(a)).collect(),
1052-
values_checked: self.values_checked.iter().map(|a| f(a)).collect(),
1053-
values_valid: self.values_valid.iter().map(|(a, b)| (f(a), f(b))).collect(),
1041+
names_valid: self
1042+
.names_valid
1043+
.as_ref()
1044+
.map(|names_valid| names_valid.iter().map(|a| f(a)).collect()),
1045+
values_valid: self
1046+
.values_valid
1047+
.iter()
1048+
.map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect()))
1049+
.collect(),
10541050
}
10551051
}
10561052
}
@@ -1090,17 +1086,23 @@ impl CrateCheckConfig {
10901086
sym::doctest,
10911087
sym::feature,
10921088
];
1093-
for &name in WELL_KNOWN_NAMES {
1094-
self.names_valid.insert(name);
1089+
if let Some(names_valid) = &mut self.names_valid {
1090+
for &name in WELL_KNOWN_NAMES {
1091+
names_valid.insert(name);
1092+
}
10951093
}
10961094
}
10971095

10981096
/// Fills a `CrateCheckConfig` with configuration names and values that are actually active.
10991097
pub fn fill_actual(&mut self, cfg: &CrateConfig) {
11001098
for &(k, v) in cfg {
1101-
self.names_valid.insert(k);
1099+
if let Some(names_valid) = &mut self.names_valid {
1100+
names_valid.insert(k);
1101+
}
11021102
if let Some(v) = v {
1103-
self.values_valid.insert((k, v));
1103+
self.values_valid.entry(k).and_modify(|values| {
1104+
values.insert(v);
1105+
});
11041106
}
11051107
}
11061108
}

0 commit comments

Comments
 (0)