Rollup merge of #143984 - JonathanBrouwer:fix-feature-gate-ice, r=Urgau

matthiaskrgr · web-flow · commit 52868368dd2b · 2025-07-17T10:41:49.000+02:00
Fix ice for feature-gated `cfg` attributes applied to the crate This PR fixes two fixes: 1. When a feature gated option of the `cfg` attribute is applied to the crate, an ICE would occur because features are not yet available at that stage. This is fixed by ignoring the feature gate at that point, the attribute will later be re-checked (this was already done) when the feature gate is available. Fixes #143977 2. Errors and lints on the `cfg` attribute applied to the crate would be produced twice, because of the re-checking. This is fixed by not producing any errors and lints during the first run. The added regression test checks both problems. r? ``@jdonszelmann``
diff --git 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<S: Stage>(
     list: &MetaItemListParser<'_>,
     meta_span: Span,
 ) -> Option<CfgEntry> {
-    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<S: Stage>(
     list: &MetaItemListParser<'_>,
     meta_span: Span,
 ) -> Option<CfgEntry> {
-    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
@@ -226,7 +226,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()
@@ -257,7 +257,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;
@@ -558,6 +558,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> {
@@ -600,7 +619,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,
@@ -623,7 +642,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
@@ -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
@@ -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,15 +438,15 @@ impl<'a> StripUnconfigured<'a> {
             attr.span,
             node,
             self.features,
-            true,
+            emit_errors,
             parse_cfg_attr,
             &CFG_TEMPLATE,
         ) else {
             // Cfg attribute was not parsable, give up
             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
@@ -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
@@ -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,
diff --git a/tests/ui/check-cfg/cfg-crate-features.rs 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
@@ -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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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 <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> 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`.
