From ab36d506d21287dfe1b8d3f1c70024b6e6af18cf Mon Sep 17 00:00:00 2001 From: Jules Bertholet Date: Fri, 20 Mar 2026 18:48:38 -0400 Subject: [PATCH] Address review comments --- .../rustc_attr_parsing/src/attributes/cfg.rs | 6 ++-- .../src/attributes/cfg_select.rs | 4 +-- compiler/rustc_attr_parsing/src/interface.rs | 6 ++-- compiler/rustc_attr_parsing/src/parser.rs | 35 ++++++++++++------- compiler/rustc_builtin_macros/src/cfg.rs | 7 ++-- compiler/rustc_expand/src/config.rs | 7 ++-- compiler/rustc_expand/src/expand.rs | 3 +- tests/ui/macros/attr-expr.rs | 12 ++++++- tests/ui/macros/attr-expr.stderr | 13 ++++++- 9 files changed, 63 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_attr_parsing/src/attributes/cfg.rs b/compiler/rustc_attr_parsing/src/attributes/cfg.rs index c88dff40b2a0..badf696606e9 100644 --- a/compiler/rustc_attr_parsing/src/attributes/cfg.rs +++ b/compiler/rustc_attr_parsing/src/attributes/cfg.rs @@ -20,7 +20,9 @@ use thin_vec::ThinVec; use crate::context::{AcceptContext, ShouldEmit, Stage}; -use crate::parser::{ArgParser, MetaItemListParser, MetaItemOrLitParser, NameValueParser}; +use crate::parser::{ + AllowExprMetavar, ArgParser, MetaItemListParser, MetaItemOrLitParser, NameValueParser, +}; use crate::session_diagnostics::{ AttributeParseError, AttributeParseErrorReason, CfgAttrBadDelim, MetaBadDelimSugg, ParsedDescription, @@ -363,7 +365,7 @@ fn parse_cfg_attr_internal<'a>( let meta = MetaItemOrLitParser::parse_single( parser, ShouldEmit::ErrorsAndLints { recovery: Recovery::Allowed }, - true, + AllowExprMetavar::Yes, )?; let pred_span = pred_start.with_hi(parser.token.span.hi()); diff --git a/compiler/rustc_attr_parsing/src/attributes/cfg_select.rs b/compiler/rustc_attr_parsing/src/attributes/cfg_select.rs index 96a4437c33a7..f3612afe69e5 100644 --- a/compiler/rustc_attr_parsing/src/attributes/cfg_select.rs +++ b/compiler/rustc_attr_parsing/src/attributes/cfg_select.rs @@ -12,7 +12,7 @@ use rustc_session::lint::builtin::UNREACHABLE_CFG_SELECT_PREDICATES; use rustc_span::{ErrorGuaranteed, Span, Symbol, sym}; -use crate::parser::MetaItemOrLitParser; +use crate::parser::{AllowExprMetavar, MetaItemOrLitParser}; use crate::{AttributeParser, ParsedDescription, ShouldEmit, parse_cfg_entry}; #[derive(Clone)] @@ -94,7 +94,7 @@ pub fn parse_cfg_select( let meta = MetaItemOrLitParser::parse_single( p, ShouldEmit::ErrorsAndLints { recovery: Recovery::Allowed }, - true, + AllowExprMetavar::Yes, ) .map_err(|diag| diag.emit())?; let cfg_span = meta.span(); diff --git a/compiler/rustc_attr_parsing/src/interface.rs b/compiler/rustc_attr_parsing/src/interface.rs index a9e2bd3aa184..fa66dec6a156 100644 --- a/compiler/rustc_attr_parsing/src/interface.rs +++ b/compiler/rustc_attr_parsing/src/interface.rs @@ -14,7 +14,7 @@ use crate::context::{AcceptContext, FinalizeContext, FinalizeFn, SharedContext, Stage}; use crate::early_parsed::{EARLY_PARSED_ATTRIBUTES, EarlyParsedState}; -use crate::parser::{ArgParser, PathParser, RefPathParser}; +use crate::parser::{AllowExprMetavar, ArgParser, PathParser, RefPathParser}; use crate::session_diagnostics::ParsedDescription; use crate::{Early, Late, OmitDoc, ShouldEmit}; @@ -139,7 +139,7 @@ pub fn parse_single( emit_errors: ShouldEmit, parse_fn: fn(cx: &mut AcceptContext<'_, '_, Early>, item: &ArgParser) -> Option, template: &AttributeTemplate, - allow_expr_metavar: bool, + allow_expr_metavar: AllowExprMetavar, ) -> Option { let ast::AttrKind::Normal(normal_attr) = &attr.kind else { panic!("parse_single called on a doc attr") @@ -335,7 +335,7 @@ pub fn parse_attribute_list( &parts, &self.sess.psess, self.stage.should_emit(), - false, + AllowExprMetavar::No, ) else { continue; }; diff --git a/compiler/rustc_attr_parsing/src/parser.rs b/compiler/rustc_attr_parsing/src/parser.rs index ee77a1a9f9cd..79cb98916ed6 100644 --- a/compiler/rustc_attr_parsing/src/parser.rs +++ b/compiler/rustc_attr_parsing/src/parser.rs @@ -109,7 +109,7 @@ pub fn from_attr_args<'sess>( parts: &[Symbol], psess: &'sess ParseSess, should_emit: ShouldEmit, - allow_expr_metavar: bool, + allow_expr_metavar: AllowExprMetavar, ) -> Option { Some(match value { AttrArgs::Empty => Self::NoArgs, @@ -225,7 +225,7 @@ impl MetaItemOrLitParser { pub fn parse_single<'sess>( parser: &mut Parser<'sess>, should_emit: ShouldEmit, - allow_expr_metavar: bool, + allow_expr_metavar: AllowExprMetavar, ) -> PResult<'sess, MetaItemOrLitParser> { let mut this = MetaItemListParserContext { parser, should_emit, allow_expr_metavar }; this.parse_meta_item_inner() @@ -413,10 +413,19 @@ fn expr_to_lit<'sess>( } } +/// Whether expansions of `expr` metavariables from decrarative macros +/// are permitted. Used when parsing meta items; currently, only `cfg` predicates +/// enable this option +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum AllowExprMetavar { + No, + Yes, +} + struct MetaItemListParserContext<'a, 'sess> { parser: &'a mut Parser<'sess>, should_emit: ShouldEmit, - allow_expr_metavar: bool, + allow_expr_metavar: AllowExprMetavar, } impl<'a, 'sess> MetaItemListParserContext<'a, 'sess> { @@ -457,25 +466,25 @@ fn unsuffixed_meta_item_from_lit( Ok(lit) } - fn parse_attr_item(&mut self) -> PResult<'sess, MetaItemParser> { + fn parse_meta_item(&mut self) -> PResult<'sess, MetaItemParser> { if let Some(metavar) = self.parser.token.is_metavar_seq() { - match metavar { - kind @ MetaVarKind::Expr { .. } if self.allow_expr_metavar => { + match (metavar, self.allow_expr_metavar) { + (kind @ MetaVarKind::Expr { .. }, AllowExprMetavar::Yes) => { return self .parser .eat_metavar_seq(kind, |this| { MetaItemListParserContext { parser: this, should_emit: self.should_emit, - allow_expr_metavar: true, + allow_expr_metavar: AllowExprMetavar::Yes, } - .parse_attr_item() + .parse_meta_item() }) .ok_or_else(|| { self.parser.unexpected_any::().unwrap_err() }); } - MetaVarKind::Meta { has_meta_form } => { + (MetaVarKind::Meta { has_meta_form }, _) => { return if has_meta_form { let attr_item = self .parser @@ -485,7 +494,7 @@ fn parse_attr_item(&mut self) -> PResult<'sess, MetaItemParser> { should_emit: self.should_emit, allow_expr_metavar: self.allow_expr_metavar, } - .parse_attr_item() + .parse_meta_item() }) .unwrap(); Ok(attr_item) @@ -530,7 +539,7 @@ fn parse_meta_item_inner(&mut self) -> PResult<'sess, MetaItemOrLitParser> { Ok(MetaItemOrLitParser::Lit(self.unsuffixed_meta_item_from_lit(token_lit)?)) } else { let prev_pros = self.parser.approx_token_stream_pos(); - match self.parse_attr_item() { + match self.parse_meta_item() { Ok(item) => Ok(MetaItemOrLitParser::MetaItemParser(item)), Err(err) => { // If `parse_attr_item` made any progress, it likely has a more precise error we should prefer @@ -618,7 +627,7 @@ fn parse( psess: &'sess ParseSess, span: Span, should_emit: ShouldEmit, - allow_expr_metavar: bool, + allow_expr_metavar: AllowExprMetavar, ) -> PResult<'sess, MetaItemListParser> { let mut parser = Parser::new(psess, tokens, None); if let ShouldEmit::ErrorsAndLints { recovery } = should_emit { @@ -658,7 +667,7 @@ pub(crate) fn new<'sess>( span: Span, psess: &'sess ParseSess, should_emit: ShouldEmit, - allow_expr_metavar: bool, + allow_expr_metavar: AllowExprMetavar, ) -> Result> { MetaItemListParserContext::parse( tokens.clone(), diff --git a/compiler/rustc_builtin_macros/src/cfg.rs b/compiler/rustc_builtin_macros/src/cfg.rs index cdd556388064..c4a458089f2d 100644 --- a/compiler/rustc_builtin_macros/src/cfg.rs +++ b/compiler/rustc_builtin_macros/src/cfg.rs @@ -4,10 +4,9 @@ use rustc_ast::tokenstream::TokenStream; use rustc_ast::{AttrStyle, token}; -use rustc_attr_parsing as attr; -use rustc_attr_parsing::parser::MetaItemOrLitParser; +use rustc_attr_parsing::parser::{AllowExprMetavar, MetaItemOrLitParser}; use rustc_attr_parsing::{ - AttributeParser, CFG_TEMPLATE, ParsedDescription, ShouldEmit, parse_cfg_entry, + self as attr, AttributeParser, CFG_TEMPLATE, ParsedDescription, ShouldEmit, parse_cfg_entry, }; use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacEager, MacroExpanderResult}; use rustc_hir::attrs::CfgEntry; @@ -44,7 +43,7 @@ fn parse_cfg(cx: &ExtCtxt<'_>, span: Span, tts: TokenStream) -> Result Eval emit_errors, parse_cfg, &CFG_TEMPLATE, - true, + AllowExprMetavar::Yes, ) else { // Cfg attribute was not parsable, give up return EvalConfigResult::True; diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index db08790364c6..640d0746fe1a 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -13,6 +13,7 @@ TyKind, token, }; use rustc_ast_pretty::pprust; +use rustc_attr_parsing::parser::AllowExprMetavar; use rustc_attr_parsing::{ AttributeParser, CFG_TEMPLATE, Early, EvalConfigResult, ShouldEmit, eval_config_entry, parse_cfg, validate_attr, @@ -2224,7 +2225,7 @@ fn expand_cfg_true( ShouldEmit::ErrorsAndLints { recovery: Recovery::Allowed }, parse_cfg, &CFG_TEMPLATE, - true, + AllowExprMetavar::Yes, ) else { // Cfg attribute was not parsable, give up return EvalConfigResult::True; diff --git a/tests/ui/macros/attr-expr.rs b/tests/ui/macros/attr-expr.rs index d2f5f5d184d6..a2bee8728ac5 100644 --- a/tests/ui/macros/attr-expr.rs +++ b/tests/ui/macros/attr-expr.rs @@ -3,7 +3,17 @@ macro_rules! foo { #[$e] //~^ ERROR expected identifier, found metavariable fn foo() {} - } + }; } foo!(inline); + +macro_rules! bar { + ($e:expr) => { + #[inline($e)] + //~^ ERROR expected a literal (`1u8`, `1.0f32`, `"string"`, etc.) here, found `expr` metavariable + fn bar() {} + }; +} +bar!(always); + fn main() {} diff --git a/tests/ui/macros/attr-expr.stderr b/tests/ui/macros/attr-expr.stderr index 034f116d017d..eaad41c44fb9 100644 --- a/tests/ui/macros/attr-expr.stderr +++ b/tests/ui/macros/attr-expr.stderr @@ -9,5 +9,16 @@ LL | foo!(inline); | = note: this error originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 1 previous error +error: expected a literal (`1u8`, `1.0f32`, `"string"`, etc.) here, found `expr` metavariable + --> $DIR/attr-expr.rs:12:18 + | +LL | #[inline($e)] + | ^^ +... +LL | bar!(always); + | ------------ in this macro invocation + | + = note: this error originates in the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 2 previous errors