From 8e400f97e59211732ba67518199432ae22685cb5 Mon Sep 17 00:00:00 2001 From: Jonathan Brouwer Date: Tue, 15 Jul 2025 19:07:37 +0200 Subject: [PATCH 1/2] Fix ice for feature-gated cfg attributes applied to the crate Signed-off-by: Jonathan Brouwer --- .../rustc_attr_parsing/src/attributes/cfg.rs | 53 +++++++++++-------- compiler/rustc_attr_parsing/src/context.rs | 27 ++++++++-- compiler/rustc_attr_parsing/src/lib.rs | 2 +- compiler/rustc_expand/src/config.rs | 22 +++++--- compiler/rustc_expand/src/expand.rs | 4 +- compiler/rustc_resolve/src/def_collector.rs | 4 +- 6 files changed, 74 insertions(+), 38 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/attributes/cfg.rs b/compiler/rustc_attr_parsing/src/attributes/cfg.rs index a56855b3bd3d1..6373cf6e08adb 100644 --- a/compiler/rustc_attr_parsing/src/attributes/cfg.rs +++ b/compiler/rustc_attr_parsing/src/attributes/cfg.rs @@ -9,7 +9,7 @@ use rustc_session::parse::feature_err; use rustc_span::{Span, Symbol, sym}; use thin_vec::ThinVec; -use crate::context::{AcceptContext, Stage}; +use crate::context::{AcceptContext, ShouldEmit, Stage}; use crate::parser::{ArgParser, MetaItemListParser, MetaItemOrLitParser, NameValueParser}; use crate::{ CfgMatchesLintEmitter, fluent_generated, parse_version, session_diagnostics, try_gate_cfg, @@ -90,7 +90,7 @@ fn parse_cfg_entry_version( list: &MetaItemListParser<'_>, meta_span: Span, ) -> Option { - try_gate_cfg(sym::version, meta_span, cx.sess(), Some(cx.features())); + try_gate_cfg(sym::version, meta_span, cx.sess(), cx.features_option()); let Some(version) = list.single() else { cx.emit_err(session_diagnostics::ExpectedSingleVersionLiteral { span: list.span }); return None; @@ -119,7 +119,9 @@ fn parse_cfg_entry_target( list: &MetaItemListParser<'_>, meta_span: Span, ) -> Option { - if !cx.features().cfg_target_compact() { + if let Some(features) = cx.features_option() + && !features.cfg_target_compact() + { feature_err( cx.sess(), sym::cfg_target_compact, @@ -186,12 +188,13 @@ pub fn eval_config_entry( cfg_entry: &CfgEntry, id: NodeId, features: Option<&Features>, + emit_lints: ShouldEmit, ) -> EvalConfigResult { match cfg_entry { CfgEntry::All(subs, ..) => { let mut all = None; for sub in subs { - let res = eval_config_entry(sess, sub, id, features); + let res = eval_config_entry(sess, sub, id, features, emit_lints); // We cannot short-circuit because `eval_config_entry` emits some lints if !res.as_bool() { all.get_or_insert(res); @@ -202,7 +205,7 @@ pub fn eval_config_entry( CfgEntry::Any(subs, span) => { let mut any = None; for sub in subs { - let res = eval_config_entry(sess, sub, id, features); + let res = eval_config_entry(sess, sub, id, features, emit_lints); // We cannot short-circuit because `eval_config_entry` emits some lints if res.as_bool() { any.get_or_insert(res); @@ -214,7 +217,7 @@ pub fn eval_config_entry( }) } CfgEntry::Not(sub, span) => { - if eval_config_entry(sess, sub, id, features).as_bool() { + if eval_config_entry(sess, sub, id, features, emit_lints).as_bool() { EvalConfigResult::False { reason: cfg_entry.clone(), reason_span: *span } } else { EvalConfigResult::True @@ -228,24 +231,28 @@ pub fn eval_config_entry( } } CfgEntry::NameValue { name, name_span, value, span } => { - match sess.psess.check_config.expecteds.get(name) { - Some(ExpectedValues::Some(values)) if !values.contains(&value.map(|(v, _)| v)) => { - id.emit_span_lint( - sess, - UNEXPECTED_CFGS, - *span, - BuiltinLintDiag::UnexpectedCfgValue((*name, *name_span), *value), - ); + if let ShouldEmit::ErrorsAndLints = emit_lints { + match sess.psess.check_config.expecteds.get(name) { + Some(ExpectedValues::Some(values)) + if !values.contains(&value.map(|(v, _)| v)) => + { + id.emit_span_lint( + sess, + UNEXPECTED_CFGS, + *span, + BuiltinLintDiag::UnexpectedCfgValue((*name, *name_span), *value), + ); + } + None if sess.psess.check_config.exhaustive_names => { + id.emit_span_lint( + sess, + UNEXPECTED_CFGS, + *span, + BuiltinLintDiag::UnexpectedCfgName((*name, *name_span), *value), + ); + } + _ => { /* not unexpected */ } } - None if sess.psess.check_config.exhaustive_names => { - id.emit_span_lint( - sess, - UNEXPECTED_CFGS, - *span, - BuiltinLintDiag::UnexpectedCfgName((*name, *name_span), *value), - ); - } - _ => { /* not unexpected */ } } if sess.psess.config.contains(&(*name, value.map(|(v, _)| v))) { diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index 567341d1517dd..dc80d408b2737 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -223,7 +223,7 @@ impl Stage for Early { sess: &'sess Session, diag: impl for<'x> Diagnostic<'x>, ) -> ErrorGuaranteed { - if self.emit_errors { + if self.emit_errors.should_emit() { sess.dcx().emit_err(diag) } else { sess.dcx().create_err(diag).delay_as_bug() @@ -254,7 +254,7 @@ pub struct Early { /// Whether to emit errors or delay them as a bug /// For most attributes, the attribute will be parsed again in the `Late` stage and in this case the errors should be delayed /// But for some, such as `cfg`, the attribute will be removed before the `Late` stage so errors must be emitted - pub emit_errors: bool, + pub emit_errors: ShouldEmit, } /// used when parsing attributes during ast lowering pub struct Late; @@ -555,6 +555,25 @@ pub enum OmitDoc { Skip, } +#[derive(Copy, Clone)] +pub enum ShouldEmit { + /// The operation will emit errors and lints. + /// This is usually what you need. + ErrorsAndLints, + /// The operation will emit *not* errors and lints. + /// Use this if you are *sure* that this operation will be called at a different time with `ShouldEmit::Emit`. + Nothing, +} + +impl ShouldEmit { + pub fn should_emit(&self) -> bool { + match self { + ShouldEmit::ErrorsAndLints => true, + ShouldEmit::Nothing => false, + } + } +} + /// Context created once, for example as part of the ast lowering /// context, through which all attributes can be lowered. pub struct AttributeParser<'sess, S: Stage = Late> { @@ -597,7 +616,7 @@ impl<'sess> AttributeParser<'sess, Early> { tools: Vec::new(), parse_only: Some(sym), sess, - stage: Early { emit_errors: false }, + stage: Early { emit_errors: ShouldEmit::Nothing }, }; let mut parsed = p.parse_attribute_list( attrs, @@ -620,7 +639,7 @@ impl<'sess> AttributeParser<'sess, Early> { target_span: Span, target_node_id: NodeId, features: Option<&'sess Features>, - emit_errors: bool, + emit_errors: ShouldEmit, parse_fn: fn(cx: &mut AcceptContext<'_, '_, Early>, item: &ArgParser<'_>) -> T, template: &AttributeTemplate, ) -> T { diff --git a/compiler/rustc_attr_parsing/src/lib.rs b/compiler/rustc_attr_parsing/src/lib.rs index 2102a26108bc4..dc54cb6b840cf 100644 --- a/compiler/rustc_attr_parsing/src/lib.rs +++ b/compiler/rustc_attr_parsing/src/lib.rs @@ -95,7 +95,7 @@ pub use attributes::cfg_old::*; pub use attributes::util::{ find_crate_name, is_builtin_attr, is_doc_alias_attrs_contain_symbol, parse_version, }; -pub use context::{AttributeParser, Early, Late, OmitDoc}; +pub use context::{AttributeParser, Early, Late, OmitDoc, ShouldEmit}; pub use lints::emit_attribute_lint; rustc_fluent_macro::fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 6922ddfd6bddc..83a8d601afeae 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -12,7 +12,7 @@ use rustc_ast::{ }; use rustc_attr_parsing as attr; use rustc_attr_parsing::{ - AttributeParser, CFG_TEMPLATE, EvalConfigResult, eval_config_entry, parse_cfg_attr, + AttributeParser, CFG_TEMPLATE, EvalConfigResult, ShouldEmit, eval_config_entry, parse_cfg_attr, }; use rustc_data_structures::flat_map_in_place::FlatMapInPlace; use rustc_feature::{ @@ -167,7 +167,9 @@ pub fn pre_configure_attrs(sess: &Session, attrs: &[Attribute]) -> ast::AttrVec .flat_map(|attr| strip_unconfigured.process_cfg_attr(attr)) .take_while(|attr| { !is_cfg(attr) - || strip_unconfigured.cfg_true(attr, strip_unconfigured.lint_node_id).as_bool() + || strip_unconfigured + .cfg_true(attr, strip_unconfigured.lint_node_id, ShouldEmit::Nothing) + .as_bool() }) .collect() } @@ -401,10 +403,18 @@ impl<'a> StripUnconfigured<'a> { /// Determines if a node with the given attributes should be included in this configuration. fn in_cfg(&self, attrs: &[Attribute]) -> bool { - attrs.iter().all(|attr| !is_cfg(attr) || self.cfg_true(attr, self.lint_node_id).as_bool()) + attrs.iter().all(|attr| { + !is_cfg(attr) + || self.cfg_true(attr, self.lint_node_id, ShouldEmit::ErrorsAndLints).as_bool() + }) } - pub(crate) fn cfg_true(&self, attr: &Attribute, node: NodeId) -> EvalConfigResult { + pub(crate) fn cfg_true( + &self, + attr: &Attribute, + node: NodeId, + emit_errors: ShouldEmit, + ) -> EvalConfigResult { // We need to run this to do basic validation of the attribute, such as that lits are valid, etc // FIXME(jdonszelmann) this should not be necessary in the future match validate_attr::parse_meta(&self.sess.psess, attr) { @@ -428,7 +438,7 @@ impl<'a> StripUnconfigured<'a> { attr.span, node, self.features, - true, + emit_errors, parse_cfg_attr, &CFG_TEMPLATE, ) else { @@ -436,7 +446,7 @@ impl<'a> StripUnconfigured<'a> { return EvalConfigResult::True; }; - eval_config_entry(self.sess, &cfg, self.lint_node_id, self.features) + eval_config_entry(self.sess, &cfg, self.lint_node_id, self.features, emit_errors) } /// If attributes are not allowed on expressions, emit an error for `attr` diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index f99060e9a2165..79ec79a2fdf42 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -13,7 +13,7 @@ use rustc_ast::{ MetaItemKind, ModKind, NodeId, PatKind, StmtKind, TyKind, token, }; use rustc_ast_pretty::pprust; -use rustc_attr_parsing::EvalConfigResult; +use rustc_attr_parsing::{EvalConfigResult, ShouldEmit}; use rustc_data_structures::flat_map_in_place::FlatMapInPlace; use rustc_errors::PResult; use rustc_feature::Features; @@ -2171,7 +2171,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> { attr: ast::Attribute, pos: usize, ) -> EvalConfigResult { - let res = self.cfg().cfg_true(&attr, node.node_id()); + let res = self.cfg().cfg_true(&attr, node.node_id(), ShouldEmit::ErrorsAndLints); if res.as_bool() { // A trace attribute left in AST in place of the original `cfg` attribute. // It can later be used by lints or other diagnostics. diff --git a/compiler/rustc_resolve/src/def_collector.rs b/compiler/rustc_resolve/src/def_collector.rs index 781e2ce970403..7d51fef28d3ba 100644 --- a/compiler/rustc_resolve/src/def_collector.rs +++ b/compiler/rustc_resolve/src/def_collector.rs @@ -2,7 +2,7 @@ use std::mem; use rustc_ast::visit::FnKind; use rustc_ast::*; -use rustc_attr_parsing::{AttributeParser, Early, OmitDoc}; +use rustc_attr_parsing::{AttributeParser, Early, OmitDoc, ShouldEmit}; use rustc_expand::expand::AstFragment; use rustc_hir as hir; use rustc_hir::def::{CtorKind, CtorOf, DefKind}; @@ -132,7 +132,7 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> { &self.resolver.tcx.sess, self.resolver.tcx.features(), Vec::new(), - Early { emit_errors: false }, + Early { emit_errors: ShouldEmit::Nothing }, ); let attrs = parser.parse_attribute_list( &i.attrs, From bfa45ac78b4cb018ece7daa56116e50820d2fd92 Mon Sep 17 00:00:00 2001 From: Jonathan Brouwer Date: Tue, 15 Jul 2025 19:07:49 +0200 Subject: [PATCH 2/2] Add regression test Signed-off-by: Jonathan Brouwer --- tests/ui/check-cfg/cfg-crate-features.rs | 13 ++++++++ tests/ui/check-cfg/cfg-crate-features.stderr | 33 ++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 tests/ui/check-cfg/cfg-crate-features.rs create mode 100644 tests/ui/check-cfg/cfg-crate-features.stderr diff --git a/tests/ui/check-cfg/cfg-crate-features.rs b/tests/ui/check-cfg/cfg-crate-features.rs new file mode 100644 index 0000000000000..16be8aaeeaa43 --- /dev/null +++ b/tests/ui/check-cfg/cfg-crate-features.rs @@ -0,0 +1,13 @@ +// https://github.com/rust-lang/rust/issues/143977 +// Check that features are available when an attribute is applied to a crate + +#![cfg(version("1.0"))] +//~^ ERROR `cfg(version)` is experimental and subject to change + +// Using invalid value `does_not_exist`, +// so we don't accidentally configure out the crate for any certain OS +#![cfg(not(target(os = "does_not_exist")))] +//~^ ERROR compact `cfg(target(..))` is experimental and subject to change +//~| WARN unexpected `cfg` condition value: `does_not_exist` + +fn main() {} diff --git a/tests/ui/check-cfg/cfg-crate-features.stderr b/tests/ui/check-cfg/cfg-crate-features.stderr new file mode 100644 index 0000000000000..60a5404c07391 --- /dev/null +++ b/tests/ui/check-cfg/cfg-crate-features.stderr @@ -0,0 +1,33 @@ +error[E0658]: `cfg(version)` is experimental and subject to change + --> $DIR/cfg-crate-features.rs:4:8 + | +LL | #![cfg(version("1.0"))] + | ^^^^^^^^^^^^^^ + | + = note: see issue #64796 for more information + = help: add `#![feature(cfg_version)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error[E0658]: compact `cfg(target(..))` is experimental and subject to change + --> $DIR/cfg-crate-features.rs:9:12 + | +LL | #![cfg(not(target(os = "does_not_exist")))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #96901 for more information + = help: add `#![feature(cfg_target_compact)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +warning: unexpected `cfg` condition value: `does_not_exist` + --> $DIR/cfg-crate-features.rs:9:19 + | +LL | #![cfg(not(target(os = "does_not_exist")))] + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: expected values for `target_os` are: `aix`, `amdhsa`, `android`, `cuda`, `cygwin`, `dragonfly`, `emscripten`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `lynxos178`, `macos`, `netbsd`, `none`, `nto`, `nuttx`, `openbsd`, `psp`, `psx`, `redox`, `rtems`, `solaris`, `solid_asp3`, `teeos`, `trusty`, `tvos`, and `uefi` and 9 more + = note: see for more information about checking conditional configuration + = note: `#[warn(unexpected_cfgs)]` on by default + +error: aborting due to 2 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`.