From 99f5945f85342e1eff8d31507410ddd66ea94d64 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 29 Apr 2022 06:52:01 +1000 Subject: Overhaul `MacArgs::Eq`. The value in `MacArgs::Eq` is currently represented as a `Token`. Because of `TokenKind::Interpolated`, `Token` can be either a token or an arbitrary AST fragment. In practice, a `MacArgs::Eq` starts out as a literal or macro call AST fragment, and then is later lowered to a literal token. But this is very non-obvious. `Token` is a much more general type than what is needed. This commit restricts things, by introducing a new type `MacArgsEqKind` that is either an AST expression (pre-lowering) or an AST literal (post-lowering). The downside is that the code is a bit more verbose in a few places. The benefit is that makes it much clearer what the possibilities are (though also shorter in some other places). Also, it removes one use of `TokenKind::Interpolated`, taking us a step closer to removing that variant, which will let us make `Token` impl `Copy` and remove many "handle Interpolated" code paths in the parser. Things to note: - Error messages have improved. Messages like this: ``` unexpected token: `"bug" + "found"` ``` now say "unexpected expression", which makes more sense. Although arbitrary expressions can exist within tokens thanks to `TokenKind::Interpolated`, that's not obvious to anyone who doesn't know compiler internals. - In `parse_mac_args_common`, we no longer need to collect tokens for the value expression. --- compiler/rustc_ast/src/ast.rs | 64 +++++++++++++++++++++++++++++++++---- compiler/rustc_ast/src/attr/mod.rs | 23 ++++++++++--- compiler/rustc_ast/src/mut_visit.rs | 15 +++------ compiler/rustc_ast/src/visit.rs | 14 +++----- 4 files changed, 86 insertions(+), 30 deletions(-) (limited to 'compiler/rustc_ast/src') diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index a310828b9fb..bea0482bf73 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -23,7 +23,7 @@ pub use GenericArgs::*; pub use UnsafeSource::*; use crate::ptr::P; -use crate::token::{self, CommentKind, Delimiter, Token}; +use crate::token::{self, CommentKind, Delimiter, Token, TokenKind}; use crate::tokenstream::{DelimSpan, LazyTokenStream, TokenStream, TokenTree}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; @@ -1526,7 +1526,7 @@ impl MacCall { } /// Arguments passed to an attribute or a function-like macro. -#[derive(Clone, Encodable, Decodable, Debug, HashStable_Generic)] +#[derive(Clone, Encodable, Decodable, Debug)] pub enum MacArgs { /// No arguments - `#[attr]`. Empty, @@ -1536,11 +1536,20 @@ pub enum MacArgs { Eq( /// Span of the `=` token. Span, - /// "value" as a nonterminal token. - Token, + /// The "value". + MacArgsEq, ), } +// The RHS of a `MacArgs::Eq` starts out as an expression. Once macro expansion +// is completed, all cases end up either as a literal, which is the form used +// after lowering to HIR, or as an error. +#[derive(Clone, Encodable, Decodable, Debug)] +pub enum MacArgsEq { + Ast(P), + Hir(Lit), +} + impl MacArgs { pub fn delim(&self) -> Option { match self { @@ -1553,7 +1562,10 @@ impl MacArgs { match self { MacArgs::Empty => None, MacArgs::Delimited(dspan, ..) => Some(dspan.entire()), - MacArgs::Eq(eq_span, token) => Some(eq_span.to(token.span)), + MacArgs::Eq(eq_span, MacArgsEq::Ast(expr)) => Some(eq_span.to(expr.span)), + MacArgs::Eq(_, MacArgsEq::Hir(lit)) => { + unreachable!("in literal form when getting span: {:?}", lit); + } } } @@ -1563,7 +1575,23 @@ impl MacArgs { match self { MacArgs::Empty => TokenStream::default(), MacArgs::Delimited(.., tokens) => tokens.clone(), - MacArgs::Eq(.., token) => TokenTree::Token(token.clone()).into(), + MacArgs::Eq(_, MacArgsEq::Ast(expr)) => { + // Currently only literals are allowed here. If more complex expression kinds are + // allowed in the future, then `nt_to_tokenstream` should be used to extract the + // token stream. This will require some cleverness, perhaps with a function + // pointer, because `nt_to_tokenstream` is not directly usable from this crate. + // It will also require changing the `parse_expr` call in `parse_mac_args_common` + // to `parse_expr_force_collect`. + if let ExprKind::Lit(lit) = &expr.kind { + let token = Token::new(TokenKind::Literal(lit.token), lit.span); + TokenTree::Token(token).into() + } else { + unreachable!("couldn't extract literal when getting inner tokens: {:?}", expr) + } + } + MacArgs::Eq(_, MacArgsEq::Hir(lit)) => { + unreachable!("in literal form when getting inner tokens: {:?}", lit) + } } } @@ -1574,6 +1602,30 @@ impl MacArgs { } } +impl HashStable for MacArgs +where + CTX: crate::HashStableContext, +{ + fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { + mem::discriminant(self).hash_stable(ctx, hasher); + match self { + MacArgs::Empty => {} + MacArgs::Delimited(dspan, delim, tokens) => { + dspan.hash_stable(ctx, hasher); + delim.hash_stable(ctx, hasher); + tokens.hash_stable(ctx, hasher); + } + MacArgs::Eq(_eq_span, MacArgsEq::Ast(expr)) => { + unreachable!("hash_stable {:?}", expr); + } + MacArgs::Eq(eq_span, MacArgsEq::Hir(lit)) => { + eq_span.hash_stable(ctx, hasher); + lit.hash_stable(ctx, hasher); + } + } + } +} + #[derive(Copy, Clone, PartialEq, Eq, Encodable, Decodable, Debug, HashStable_Generic)] pub enum MacDelimiter { Parenthesis, diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs index b14367aa1c2..84654a9f737 100644 --- a/compiler/rustc_ast/src/attr/mod.rs +++ b/compiler/rustc_ast/src/attr/mod.rs @@ -3,14 +3,16 @@ use crate::ast; use crate::ast::{AttrId, AttrItem, AttrKind, AttrStyle, Attribute}; use crate::ast::{Lit, LitKind}; -use crate::ast::{MacArgs, MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem}; +use crate::ast::{MacArgs, MacArgsEq, MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem}; use crate::ast::{Path, PathSegment}; +use crate::ptr::P; use crate::token::{self, CommentKind, Delimiter, Token}; use crate::tokenstream::{AttrAnnotatedTokenStream, AttrAnnotatedTokenTree}; use crate::tokenstream::{DelimSpan, Spacing, TokenTree, TreeAndSpacing}; use crate::tokenstream::{LazyTokenStream, TokenStream}; use crate::util::comments; +use rustc_data_structures::thin_vec::ThinVec; use rustc_index::bit_set::GrowableBitSet; use rustc_span::source_map::BytePos; use rustc_span::symbol::{sym, Ident, Symbol}; @@ -475,7 +477,16 @@ impl MetaItemKind { pub fn mac_args(&self, span: Span) -> MacArgs { match self { MetaItemKind::Word => MacArgs::Empty, - MetaItemKind::NameValue(lit) => MacArgs::Eq(span, lit.to_token()), + MetaItemKind::NameValue(lit) => { + let expr = P(ast::Expr { + id: ast::DUMMY_NODE_ID, + kind: ast::ExprKind::Lit(lit.clone()), + span: lit.span, + attrs: ThinVec::new(), + tokens: None, + }); + MacArgs::Eq(span, MacArgsEq::Ast(expr)) + } MetaItemKind::List(list) => { let mut tts = Vec::new(); for (i, item) in list.iter().enumerate() { @@ -552,12 +563,16 @@ impl MetaItemKind { fn from_mac_args(args: &MacArgs) -> Option { match args { + MacArgs::Empty => Some(MetaItemKind::Word), MacArgs::Delimited(_, MacDelimiter::Parenthesis, tokens) => { MetaItemKind::list_from_tokens(tokens.clone()) } MacArgs::Delimited(..) => None, - MacArgs::Eq(_, token) => Lit::from_token(token).ok().map(MetaItemKind::NameValue), - MacArgs::Empty => Some(MetaItemKind::Word), + MacArgs::Eq(_, MacArgsEq::Ast(expr)) => match &expr.kind { + ast::ExprKind::Lit(lit) => Some(MetaItemKind::NameValue(lit.clone())), + _ => None, + }, + MacArgs::Eq(_, MacArgsEq::Hir(lit)) => Some(MetaItemKind::NameValue(lit.clone())), } } diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index c85b763a820..333ffce774d 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -370,17 +370,12 @@ pub fn visit_mac_args(args: &mut MacArgs, vis: &mut T) { visit_delim_span(dspan, vis); visit_tts(tokens, vis); } - MacArgs::Eq(eq_span, token) => { + MacArgs::Eq(eq_span, MacArgsEq::Ast(expr)) => { vis.visit_span(eq_span); - // The value in `#[key = VALUE]` must be visited as an expression for backward - // compatibility, so that macros can be expanded in that position. - match &mut token.kind { - token::Interpolated(nt) => match Lrc::make_mut(nt) { - token::NtExpr(expr) => vis.visit_expr(expr), - t => panic!("unexpected token in key-value attribute: {:?}", t), - }, - t => panic!("unexpected token in key-value attribute: {:?}", t), - } + vis.visit_expr(expr); + } + MacArgs::Eq(_, MacArgsEq::Hir(lit)) => { + unreachable!("in literal form when visiting mac args eq: {:?}", lit) } } } diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index e08ba73e0ae..3a2d957194c 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -14,7 +14,6 @@ //! those that are created by the expansion of a macro. use crate::ast::*; -use crate::token; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::Span; @@ -937,14 +936,9 @@ pub fn walk_mac_args<'a, V: Visitor<'a>>(visitor: &mut V, args: &'a MacArgs) { match args { MacArgs::Empty => {} MacArgs::Delimited(_dspan, _delim, _tokens) => {} - // The value in `#[key = VALUE]` must be visited as an expression for backward - // compatibility, so that macros can be expanded in that position. - MacArgs::Eq(_eq_span, token) => match &token.kind { - token::Interpolated(nt) => match &**nt { - token::NtExpr(expr) => visitor.visit_expr(expr), - t => panic!("unexpected token in key-value attribute: {:?}", t), - }, - t => panic!("unexpected token in key-value attribute: {:?}", t), - }, + MacArgs::Eq(_eq_span, MacArgsEq::Ast(expr)) => visitor.visit_expr(expr), + MacArgs::Eq(_, MacArgsEq::Hir(lit)) => { + unreachable!("in literal form when walking mac args eq: {:?}", lit) + } } } -- cgit 1.4.1-3-g733a5