bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Improve internal representation of check-cfg

Browse source 
This is done to simplify to relationship between names() and values()
but also make thing clearer (having an Any to represent that any values
are allowed) but also to allow the (none) + values expected cases that
wasn't possible before.
This commit is contained in:
Urgau 2023-04-30 14:43:59 +02:00
parent ad6f4b73eb
commit d327d5b168
5 changed files with 173 additions and 153 deletions
Download patch file Download diff file Expand all files Collapse all files

34
compiler/rustc_attr/src/builtin.rs
View file

@ -5,6 +5,7 @@ use rustc_ast::{Attribute, LitKind, MetaItem, MetaItemKind, MetaItemLit, NestedM
use rustc_ast_pretty::pprust; use rustc_ast_pretty::pprust;
use rustc_feature::{find_gated_cfg, is_builtin_attr_name, Features, GatedCfg}; use rustc_feature::{find_gated_cfg, is_builtin_attr_name, Features, GatedCfg};
use rustc_macros::HashStable_Generic; use rustc_macros::HashStable_Generic;
use rustc_session::config::ExpectedValues;
use rustc_session::lint::builtin::UNEXPECTED_CFGS; use rustc_session::lint::builtin::UNEXPECTED_CFGS;
use rustc_session::lint::BuiltinLintDiagnostics; use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_session::parse::{feature_err, ParseSess}; use rustc_session::parse::{feature_err, ParseSess};
@ -581,8 +582,20 @@ pub fn cfg_matches(
) -> bool { ) -> bool {
eval_condition(cfg, sess, features, &mut |cfg| { eval_condition(cfg, sess, features, &mut |cfg| {
try_gate_cfg(cfg.name, cfg.span, sess, features); try_gate_cfg(cfg.name, cfg.span, sess, features);
if let Some(names_valid) = &sess.check_config.names_valid { match sess.check_config.expecteds.get(&cfg.name) {
if !names_valid.contains(&cfg.name) { Some(ExpectedValues::Some(values)) if !values.contains(&cfg.value) => {
sess.buffer_lint_with_diagnostic(
UNEXPECTED_CFGS,
cfg.span,
lint_node_id,
"unexpected `cfg` condition value",
BuiltinLintDiagnostics::UnexpectedCfg(
(cfg.name, cfg.name_span),
cfg.value_span.map(|vs| (cfg.value.unwrap(), vs)),
),
);
}
None if sess.check_config.exhaustive_names => {
sess.buffer_lint_with_diagnostic( sess.buffer_lint_with_diagnostic(
UNEXPECTED_CFGS, UNEXPECTED_CFGS,
cfg.span, cfg.span,
@ -591,22 +604,7 @@ pub fn cfg_matches(
BuiltinLintDiagnostics::UnexpectedCfg((cfg.name, cfg.name_span), None), BuiltinLintDiagnostics::UnexpectedCfg((cfg.name, cfg.name_span), None),
); );
} }
} _ => { /* not unexpected */ }
if let Some(value) = cfg.value {
if let Some(values) = &sess.check_config.values_valid.get(&cfg.name) {
if !values.contains(&value) {
sess.buffer_lint_with_diagnostic(
UNEXPECTED_CFGS,
cfg.span,
lint_node_id,
"unexpected `cfg` condition value",
BuiltinLintDiagnostics::UnexpectedCfg(
(cfg.name, cfg.name_span),
cfg.value_span.map(|vs| (value, vs)),
),
);
}
}
} }
sess.config.contains(&(cfg.name, cfg.value)) sess.config.contains(&(cfg.name, cfg.value))
}) })

25
compiler/rustc_interface/src/interface.rs
View file

@ -9,7 +9,7 @@ use rustc_data_structures::OnDrop;
use rustc_errors::registry::Registry; use rustc_errors::registry::Registry;
use rustc_errors::{ErrorGuaranteed, Handler}; use rustc_errors::{ErrorGuaranteed, Handler};
use rustc_lint::LintStore; use rustc_lint::LintStore;
use rustc_middle::ty; use rustc_middle::{bug, ty};
use rustc_parse::maybe_new_parser_from_source_str; use rustc_parse::maybe_new_parser_from_source_str;
use rustc_query_impl::QueryCtxt; use rustc_query_impl::QueryCtxt;
use rustc_query_system::query::print_query_stack; use rustc_query_system::query::print_query_stack;
@ -154,12 +154,14 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
Ok(meta_item) if parser.token == token::Eof => { Ok(meta_item) if parser.token == token::Eof => {
if let Some(args) = meta_item.meta_item_list() { if let Some(args) = meta_item.meta_item_list() {
if meta_item.has_name(sym::names) { if meta_item.has_name(sym::names) {
let names_valid = check_cfg.exhaustive_names = true;
check_cfg.names_valid.get_or_insert_with(|| FxHashSet::default());
for arg in args { for arg in args {
if arg.is_word() && arg.ident().is_some() { if arg.is_word() && arg.ident().is_some() {
let ident = arg.ident().expect("multi-segment cfg key"); let ident = arg.ident().expect("multi-segment cfg key");
names_valid.insert(ident.name.to_string()); check_cfg
.expecteds
.entry(ident.name.to_string())
.or_insert(ExpectedValues::Any);
} else { } else {
error!("`names()` arguments must be simple identifiers"); error!("`names()` arguments must be simple identifiers");
} }
@ -168,8 +170,8 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
if let Some((name, values)) = args.split_first() { if let Some((name, values)) = args.split_first() {
if name.is_word() && name.ident().is_some() { if name.is_word() && name.ident().is_some() {
let ident = name.ident().expect("multi-segment cfg key"); let ident = name.ident().expect("multi-segment cfg key");
let ident_values = check_cfg let expected_values = check_cfg
.values_valid .expecteds
.entry(ident.name.to_string()) .entry(ident.name.to_string())
.or_insert_with(|| { .or_insert_with(|| {
ExpectedValues::Some(FxHashSet::default()) ExpectedValues::Some(FxHashSet::default())
@ -183,20 +185,24 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
if let Some(LitKind::Str(s, _)) = if let Some(LitKind::Str(s, _)) =
val.lit().map(|lit| &lit.kind) val.lit().map(|lit| &lit.kind)
{ {
ident_values.insert(s.to_string()); expected_values.insert(Some(s.to_string()));
} else { } else {
error!( error!(
"`values()` arguments must be string literals" "`values()` arguments must be string literals"
); );
} }
} }
if values.is_empty() {
expected_values.insert(None);
}
} else { } else {
error!( error!(
"`values()` first argument must be a simple identifier" "`values()` first argument must be a simple identifier"
); );
} }
} else if args.is_empty() { } else if args.is_empty() {
check_cfg.well_known_values = true; check_cfg.exhaustive_values = true;
} else { } else {
expected_error(); expected_error();
} }
@ -220,9 +226,6 @@ pub fn parse_check_cfg(specs: Vec<String>) -> CheckCfg {
} }
} }
if let Some(names_valid) = &mut check_cfg.names_valid {
names_valid.extend(check_cfg.values_valid.keys().cloned());
}
check_cfg check_cfg
}) })
} }

20
compiler/rustc_lint/src/builtin.rs
View file

@ -63,6 +63,7 @@ use rustc_middle::ty::layout::{LayoutError, LayoutOf};
use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::{self, Instance, Ty, TyCtxt, VariantDef}; use rustc_middle::ty::{self, Instance, Ty, TyCtxt, VariantDef};
use rustc_session::config::ExpectedValues;
use rustc_session::lint::{BuiltinLintDiagnostics, FutureIncompatibilityReason}; use rustc_session::lint::{BuiltinLintDiagnostics, FutureIncompatibilityReason};
use rustc_span::edition::Edition; use rustc_span::edition::Edition;
use rustc_span::source_map::Spanned; use rustc_span::source_map::Spanned;
@ -3306,16 +3307,15 @@ impl EarlyLintPass for UnexpectedCfgs {
let cfg = &cx.sess().parse_sess.config; let cfg = &cx.sess().parse_sess.config;
let check_cfg = &cx.sess().parse_sess.check_config; let check_cfg = &cx.sess().parse_sess.check_config;
for &(name, value) in cfg { for &(name, value) in cfg {
if let Some(names_valid) = &check_cfg.names_valid && !names_valid.contains(&name){ match check_cfg.expecteds.get(&name) {
cx.emit_lint(UNEXPECTED_CFGS, BuiltinUnexpectedCliConfigName { Some(ExpectedValues::Some(values)) if !values.contains(&value) => {
name, let value = value.unwrap_or(kw::Empty);
}); cx.emit_lint(UNEXPECTED_CFGS, BuiltinUnexpectedCliConfigValue { name, value });
} }
if let Some(value) = value && let Some(values) = check_cfg.values_valid.get(&name) && !values.contains(&value) { None if check_cfg.exhaustive_names => {
cx.emit_lint( cx.emit_lint(UNEXPECTED_CFGS, BuiltinUnexpectedCliConfigName { name });
UNEXPECTED_CFGS, }
BuiltinUnexpectedCliConfigValue { name, value }, _ => { /* expected */ }
);
} }
} }
} }

14
compiler/rustc_lint/src/context.rs
View file

@ -769,10 +769,7 @@ pub trait LintContext: Sized {
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"); 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");
}, },
BuiltinLintDiagnostics::UnexpectedCfg((name, name_span), None) => { BuiltinLintDiagnostics::UnexpectedCfg((name, name_span), None) => {
let Some(names_valid) = &sess.parse_sess.check_config.names_valid else { let possibilities: Vec<Symbol> = sess.parse_sess.check_config.expecteds.keys().map(|s| *s).collect();
bug!("it shouldn't be possible to have a diagnostic on a name if name checking is not enabled");
};
let possibilities: Vec<Symbol> = names_valid.iter().map(|s| *s).collect();
// Suggest the most probable if we found one // Suggest the most probable if we found one
if let Some(best_match) = find_best_match_for_name(&possibilities, name, None) { if let Some(best_match) = find_best_match_for_name(&possibilities, name, None) {
@ -780,10 +777,15 @@ pub trait LintContext: Sized {
} }
}, },
BuiltinLintDiagnostics::UnexpectedCfg((name, name_span), Some((value, value_span))) => { BuiltinLintDiagnostics::UnexpectedCfg((name, name_span), Some((value, value_span))) => {
let Some(values) = &sess.parse_sess.check_config.values_valid.get(&name) else { let Some(rustc_session::config::ExpectedValues::Some(values)) = &sess.parse_sess.check_config.expecteds.get(&name) else {
bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values"); bug!("it shouldn't be possible to have a diagnostic on a value whose name is not in values");
}; };
let possibilities: Vec<Symbol> = values.iter().map(|&s| s).collect(); let mut have_none_possibility = false;
let possibilities: Vec<Symbol> = values.iter()
.inspect(|a| have_none_possibility |= a.is_none())
.copied()
.filter_map(std::convert::identity)
.collect();
// Show the full list if all possible values for a given name, but don't do it // 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 // for names as the possibilities could be very long

233
compiler/rustc_session/src/config.rs
View file

@ -1056,37 +1056,76 @@ pub fn to_crate_config(cfg: FxHashSet<(String, Option<String>)>) -> CrateConfig
/// The parsed `--check-cfg` options /// The parsed `--check-cfg` options
pub struct CheckCfg<T = String> { pub struct CheckCfg<T = String> {
/// The set of all `names()`, if None no name checking is performed /// Is well known names activated
pub names_valid: Option<FxHashSet<T>>, pub exhaustive_names: bool,
/// Is well known values activated /// Is well known values activated
pub well_known_values: bool, pub exhaustive_values: bool,
/// The set of all `values()` /// All the expected values for a config name
pub values_valid: FxHashMap<T, FxHashSet<T>>, pub expecteds: FxHashMap<T, ExpectedValues<T>>,
} }
impl<T> Default for CheckCfg<T> { impl<T> Default for CheckCfg<T> {
fn default() -> Self { fn default() -> Self {
CheckCfg { CheckCfg {
names_valid: Default::default(), exhaustive_names: false,
values_valid: Default::default(), exhaustive_values: false,
well_known_values: false, expecteds: FxHashMap::default(),
} }
} }
} }
impl<T> CheckCfg<T> { impl<T> CheckCfg<T> {
fn map_data<O: Eq + Hash>(&self, f: impl Fn(&T) -> O) -> CheckCfg<O> { fn map_data<O: Eq + Hash>(self, f: impl Fn(T) -> O) -> CheckCfg<O> {
CheckCfg { CheckCfg {
names_valid: self exhaustive_names: self.exhaustive_names,
.names_valid exhaustive_values: self.exhaustive_values,
.as_ref() expecteds: self
.map(|names_valid| names_valid.iter().map(|a| f(a)).collect()), .expecteds
values_valid: self .into_iter()
.values_valid .map(|(name, values)| {
.iter() (
.map(|(a, b)| (f(a), b.iter().map(|b| f(b)).collect())) f(name),
match values {
ExpectedValues::Some(values) => ExpectedValues::Some(
values.into_iter().map(|b| b.map(|b| f(b))).collect(),
),
ExpectedValues::Any => ExpectedValues::Any,
},
)
})
.collect(), .collect(),
well_known_values: self.well_known_values, }
}
}
pub enum ExpectedValues<T> {
Some(FxHashSet<Option<T>>),
Any,
}
impl<T: Eq + Hash> ExpectedValues<T> {
fn insert(&mut self, value: T) -> bool {
match self {
ExpectedValues::Some(expecteds) => expecteds.insert(Some(value)),
ExpectedValues::Any => false,
}
}
}
impl<T: Eq + Hash> Extend<T> for ExpectedValues<T> {
fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I) {
match self {
ExpectedValues::Some(expecteds) => expecteds.extend(iter.into_iter().map(Some)),
ExpectedValues::Any => {}
}
}
}
impl<'a, T: Eq + Hash + Copy + 'a> Extend<&'a T> for ExpectedValues<T> {
fn extend<I: IntoIterator<Item = &'a T>>(&mut self, iter: I) {
match self {
ExpectedValues::Some(expecteds) => expecteds.extend(iter.into_iter().map(|a| Some(*a))),
ExpectedValues::Any => {}
} }
} }
} }
@ -1095,58 +1134,27 @@ impl<T> CheckCfg<T> {
/// `rustc_interface::interface::Config` accepts this in the compiler configuration, /// `rustc_interface::interface::Config` accepts this in the compiler configuration,
/// but the symbol interner is not yet set up then, so we must convert it later. /// but the symbol interner is not yet set up then, so we must convert it later.
pub fn to_crate_check_config(cfg: CheckCfg) -> CrateCheckConfig { pub fn to_crate_check_config(cfg: CheckCfg) -> CrateCheckConfig {
cfg.map_data(|s| Symbol::intern(s)) cfg.map_data(|s| Symbol::intern(&s))
} }
impl CrateCheckConfig { impl CrateCheckConfig {
/// Fills a `CrateCheckConfig` with well-known configuration names. pub fn fill_well_known(&mut self, current_target: &Target) {
fn fill_well_known_names(&mut self) { if !self.exhaustive_values && !self.exhaustive_names {
// NOTE: This should be kept in sync with `default_configuration` and
// `fill_well_known_values`
const WELL_KNOWN_NAMES: &[Symbol] = &[
// rustc
sym::unix,
sym::windows,
sym::target_os,
sym::target_family,
sym::target_arch,
sym::target_endian,
sym::target_pointer_width,
sym::target_env,
sym::target_abi,
sym::target_vendor,
sym::target_thread_local,
sym::target_has_atomic_load_store,
sym::target_has_atomic,
sym::target_has_atomic_equal_alignment,
sym::target_feature,
sym::panic,
sym::sanitize,
sym::debug_assertions,
sym::proc_macro,
sym::test,
sym::feature,
// rustdoc
sym::doc,
sym::doctest,
// miri
sym::miri,
];
// We only insert well-known names if `names()` was activated
if let Some(names_valid) = &mut self.names_valid {
names_valid.extend(WELL_KNOWN_NAMES);
}
}
/// Fills a `CrateCheckConfig` with well-known configuration values.
fn fill_well_known_values(&mut self, current_target: &Target) {
if !self.well_known_values {
return; return;
} }
// NOTE: This should be kept in sync with `default_configuration` and let no_values = || {
// `fill_well_known_names` let mut values = FxHashSet::default();
values.insert(None);
ExpectedValues::Some(values)
};
let empty_values = || {
let values = FxHashSet::default();
ExpectedValues::Some(values)
};
// NOTE: This should be kept in sync with `default_configuration`
let panic_values = &PanicStrategy::all(); let panic_values = &PanicStrategy::all();
@ -1166,6 +1174,9 @@ impl CrateCheckConfig {
// Unknown possible values: // Unknown possible values:
// - `feature` // - `feature`
// - `target_feature` // - `target_feature`
for name in [sym::feature, sym::target_feature] {
self.expecteds.entry(name).or_insert(ExpectedValues::Any);
}
// No-values // No-values
for name in [ for name in [
@ -1179,20 +1190,23 @@ impl CrateCheckConfig {
sym::debug_assertions, sym::debug_assertions,
sym::target_thread_local, sym::target_thread_local,
] { ] {
self.values_valid.entry(name).or_default(); self.expecteds.entry(name).or_insert_with(no_values);
} }
// Pre-defined values // Pre-defined values
self.values_valid.entry(sym::panic).or_default().extend(panic_values); self.expecteds.entry(sym::panic).or_insert_with(empty_values).extend(panic_values);
self.values_valid.entry(sym::sanitize).or_default().extend(sanitize_values); self.expecteds.entry(sym::sanitize).or_insert_with(empty_values).extend(sanitize_values);
self.values_valid.entry(sym::target_has_atomic).or_default().extend(atomic_values); self.expecteds
self.values_valid .entry(sym::target_has_atomic)
.entry(sym::target_has_atomic_load_store) .or_insert_with(no_values)
.or_default()
.extend(atomic_values); .extend(atomic_values);
self.values_valid self.expecteds
.entry(sym::target_has_atomic_load_store)
.or_insert_with(no_values)
.extend(atomic_values);
self.expecteds
.entry(sym::target_has_atomic_equal_alignment) .entry(sym::target_has_atomic_equal_alignment)
.or_default() .or_insert_with(no_values)
.extend(atomic_values); .extend(atomic_values);
// Target specific values // Target specific values
@ -1210,47 +1224,50 @@ impl CrateCheckConfig {
// Initialize (if not already initialized) // Initialize (if not already initialized)
for &e in VALUES { for &e in VALUES {
self.values_valid.entry(e).or_default(); let entry = self.expecteds.entry(e);
if !self.exhaustive_values {
entry.or_insert(ExpectedValues::Any);
} else {
entry.or_insert_with(empty_values);
}
} }
// Get all values map at once otherwise it would be costly. if self.exhaustive_values {
// (8 values * 220 targets ~= 1760 times, at the time of writing this comment). // Get all values map at once otherwise it would be costly.
let [ // (8 values * 220 targets ~= 1760 times, at the time of writing this comment).
values_target_os, let [
values_target_family, values_target_os,
values_target_arch, values_target_family,
values_target_endian, values_target_arch,
values_target_env, values_target_endian,
values_target_abi, values_target_env,
values_target_vendor, values_target_abi,
values_target_pointer_width, values_target_vendor,
] = self values_target_pointer_width,
.values_valid ] = self
.get_many_mut(VALUES) .expecteds
.expect("unable to get all the check-cfg values buckets"); .get_many_mut(VALUES)
.expect("unable to get all the check-cfg values buckets");
for target in TARGETS for target in TARGETS
.iter() .iter()
.map(|target| Target::expect_builtin(&TargetTriple::from_triple(target))) .map(|target| Target::expect_builtin(&TargetTriple::from_triple(target)))
.chain(iter::once(current_target.clone())) .chain(iter::once(current_target.clone()))
{ {
values_target_os.insert(Symbol::intern(&target.options.os)); values_target_os.insert(Symbol::intern(&target.options.os));
values_target_family values_target_family.extend(
.extend(target.options.families.iter().map(|family| Symbol::intern(family))); target.options.families.iter().map(|family| Symbol::intern(family)),
values_target_arch.insert(Symbol::intern(&target.arch)); );
values_target_endian.insert(Symbol::intern(target.options.endian.as_str())); values_target_arch.insert(Symbol::intern(&target.arch));
values_target_env.insert(Symbol::intern(&target.options.env)); values_target_endian.insert(Symbol::intern(target.options.endian.as_str()));
values_target_abi.insert(Symbol::intern(&target.options.abi)); values_target_env.insert(Symbol::intern(&target.options.env));
values_target_vendor.insert(Symbol::intern(&target.options.vendor)); values_target_abi.insert(Symbol::intern(&target.options.abi));
values_target_pointer_width.insert(sym::integer(target.pointer_width)); values_target_vendor.insert(Symbol::intern(&target.options.vendor));
values_target_pointer_width.insert(sym::integer(target.pointer_width));
}
} }
} }
} }
pub fn fill_well_known(&mut self, current_target: &Target) {
self.fill_well_known_names();
self.fill_well_known_values(current_target);
}
} }
pub fn build_configuration(sess: &Session, mut user_cfg: CrateConfig) -> CrateConfig { pub fn build_configuration(sess: &Session, mut user_cfg: CrateConfig) -> CrateConfig {