Skip to content

Fix ice for feature-gated cfg attributes applied to the crate #143984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 30 additions & 23 deletions compiler/rustc_attr_parsing/src/attributes/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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
Expand All @@ -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))) {
Expand Down
27 changes: 23 additions & 4 deletions compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_attr_parsing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
22 changes: 16 additions & 6 deletions compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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`
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/check-cfg/cfg-crate-features.rs
Original file line number Diff line number Diff line change
@@ -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() {}
33 changes: 33 additions & 0 deletions tests/ui/check-cfg/cfg-crate-features.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading