Skip to content

Commit 4c60a0e

Browse files
borsbors
committed
Auto merge of #96546 - nnethercote:overhaul-MacArgs, r=petrochenkov
Overhaul `MacArgs` Motivation: - Clarify some code that I found hard to understand. - Eliminate one use of three places where `TokenKind::Interpolated` values are created. r? `@petrochenkov`
2 parents 343889b + baa18c0 commit 4c60a0e

File tree

22 files changed

+193
-115
lines changed

22 files changed

+193
-115
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub use GenericArgs::*;
2323
pub use UnsafeSource::*;
2424

2525
use crate::ptr::P;
26-
use crate::token::{self, CommentKind, Delimiter, Token};
26+
use crate::token::{self, CommentKind, Delimiter, Token, TokenKind};
2727
use crate::tokenstream::{DelimSpan, LazyTokenStream, TokenStream, TokenTree};
2828

2929
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
@@ -1532,7 +1532,7 @@ impl MacCall {
15321532
}
15331533

15341534
/// Arguments passed to an attribute or a function-like macro.
1535-
#[derive(Clone, Encodable, Decodable, Debug, HashStable_Generic)]
1535+
#[derive(Clone, Encodable, Decodable, Debug)]
15361536
pub enum MacArgs {
15371537
/// No arguments - `#[attr]`.
15381538
Empty,
@@ -1542,11 +1542,20 @@ pub enum MacArgs {
15421542
Eq(
15431543
/// Span of the `=` token.
15441544
Span,
1545-
/// "value" as a nonterminal token.
1546-
Token,
1545+
/// The "value".
1546+
MacArgsEq,
15471547
),
15481548
}
15491549

1550+
// The RHS of a `MacArgs::Eq` starts out as an expression. Once macro expansion
1551+
// is completed, all cases end up either as a literal, which is the form used
1552+
// after lowering to HIR, or as an error.
1553+
#[derive(Clone, Encodable, Decodable, Debug)]
1554+
pub enum MacArgsEq {
1555+
Ast(P<Expr>),
1556+
Hir(Lit),
1557+
}
1558+
15501559
impl MacArgs {
15511560
pub fn delim(&self) -> Option<Delimiter> {
15521561
match self {
@@ -1559,7 +1568,10 @@ impl MacArgs {
15591568
match self {
15601569
MacArgs::Empty => None,
15611570
MacArgs::Delimited(dspan, ..) => Some(dspan.entire()),
1562-
MacArgs::Eq(eq_span, token) => Some(eq_span.to(token.span)),
1571+
MacArgs::Eq(eq_span, MacArgsEq::Ast(expr)) => Some(eq_span.to(expr.span)),
1572+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
1573+
unreachable!("in literal form when getting span: {:?}", lit);
1574+
}
15631575
}
15641576
}
15651577

@@ -1569,7 +1581,23 @@ impl MacArgs {
15691581
match self {
15701582
MacArgs::Empty => TokenStream::default(),
15711583
MacArgs::Delimited(.., tokens) => tokens.clone(),
1572-
MacArgs::Eq(.., token) => TokenTree::Token(token.clone()).into(),
1584+
MacArgs::Eq(_, MacArgsEq::Ast(expr)) => {
1585+
// Currently only literals are allowed here. If more complex expression kinds are
1586+
// allowed in the future, then `nt_to_tokenstream` should be used to extract the
1587+
// token stream. This will require some cleverness, perhaps with a function
1588+
// pointer, because `nt_to_tokenstream` is not directly usable from this crate.
1589+
// It will also require changing the `parse_expr` call in `parse_mac_args_common`
1590+
// to `parse_expr_force_collect`.
1591+
if let ExprKind::Lit(lit) = &expr.kind {
1592+
let token = Token::new(TokenKind::Literal(lit.token), lit.span);
1593+
TokenTree::Token(token).into()
1594+
} else {
1595+
unreachable!("couldn't extract literal when getting inner tokens: {:?}", expr)
1596+
}
1597+
}
1598+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
1599+
unreachable!("in literal form when getting inner tokens: {:?}", lit)
1600+
}
15731601
}
15741602
}
15751603

@@ -1580,6 +1608,30 @@ impl MacArgs {
15801608
}
15811609
}
15821610

1611+
impl<CTX> HashStable<CTX> for MacArgs
1612+
where
1613+
CTX: crate::HashStableContext,
1614+
{
1615+
fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) {
1616+
mem::discriminant(self).hash_stable(ctx, hasher);
1617+
match self {
1618+
MacArgs::Empty => {}
1619+
MacArgs::Delimited(dspan, delim, tokens) => {
1620+
dspan.hash_stable(ctx, hasher);
1621+
delim.hash_stable(ctx, hasher);
1622+
tokens.hash_stable(ctx, hasher);
1623+
}
1624+
MacArgs::Eq(_eq_span, MacArgsEq::Ast(expr)) => {
1625+
unreachable!("hash_stable {:?}", expr);
1626+
}
1627+
MacArgs::Eq(eq_span, MacArgsEq::Hir(lit)) => {
1628+
eq_span.hash_stable(ctx, hasher);
1629+
lit.hash_stable(ctx, hasher);
1630+
}
1631+
}
1632+
}
1633+
}
1634+
15831635
#[derive(Copy, Clone, PartialEq, Eq, Encodable, Decodable, Debug, HashStable_Generic)]
15841636
pub enum MacDelimiter {
15851637
Parenthesis,

‎compiler/rustc_ast/src/attr/mod.rs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,16 @@
33
use crate::ast;
44
use crate::ast::{AttrId, AttrItem, AttrKind, AttrStyle, Attribute};
55
use crate::ast::{Lit, LitKind};
6-
use crate::ast::{MacArgs, MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem};
6+
use crate::ast::{MacArgs, MacArgsEq, MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem};
77
use crate::ast::{Path, PathSegment};
8+
use crate::ptr::P;
89
use crate::token::{self, CommentKind, Delimiter, Token};
910
use crate::tokenstream::{AttrAnnotatedTokenStream, AttrAnnotatedTokenTree};
1011
use crate::tokenstream::{DelimSpan, Spacing, TokenTree, TreeAndSpacing};
1112
use crate::tokenstream::{LazyTokenStream, TokenStream};
1213
use crate::util::comments;
1314

15+
use rustc_data_structures::thin_vec::ThinVec;
1416
use rustc_index::bit_set::GrowableBitSet;
1517
use rustc_span::source_map::BytePos;
1618
use rustc_span::symbol::{sym, Ident, Symbol};
@@ -475,7 +477,16 @@ impl MetaItemKind {
475477
pub fn mac_args(&self, span: Span) -> MacArgs {
476478
match self {
477479
MetaItemKind::Word => MacArgs::Empty,
478-
MetaItemKind::NameValue(lit) => MacArgs::Eq(span, lit.to_token()),
480+
MetaItemKind::NameValue(lit) => {
481+
let expr = P(ast::Expr {
482+
id: ast::DUMMY_NODE_ID,
483+
kind: ast::ExprKind::Lit(lit.clone()),
484+
span: lit.span,
485+
attrs: ThinVec::new(),
486+
tokens: None,
487+
});
488+
MacArgs::Eq(span, MacArgsEq::Ast(expr))
489+
}
479490
MetaItemKind::List(list) => {
480491
let mut tts = Vec::new();
481492
for (i, item) in list.iter().enumerate() {
@@ -552,12 +563,16 @@ impl MetaItemKind {
552563

553564
fn from_mac_args(args: &MacArgs) -> Option<MetaItemKind> {
554565
match args {
566+
MacArgs::Empty => Some(MetaItemKind::Word),
555567
MacArgs::Delimited(_, MacDelimiter::Parenthesis, tokens) => {
556568
MetaItemKind::list_from_tokens(tokens.clone())
557569
}
558570
MacArgs::Delimited(..) => None,
559-
MacArgs::Eq(_, token) => Lit::from_token(token).ok().map(MetaItemKind::NameValue),
560-
MacArgs::Empty => Some(MetaItemKind::Word),
571+
MacArgs::Eq(_, MacArgsEq::Ast(expr)) => match &expr.kind {
572+
ast::ExprKind::Lit(lit) => Some(MetaItemKind::NameValue(lit.clone())),
573+
_ => None,
574+
},
575+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => Some(MetaItemKind::NameValue(lit.clone())),
561576
}
562577
}
563578

‎compiler/rustc_ast/src/mut_visit.rs

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -370,21 +370,12 @@ pub fn visit_mac_args<T: MutVisitor>(args: &mut MacArgs, vis: &mut T) {
370370
visit_delim_span(dspan, vis);
371371
visit_tts(tokens, vis);
372372
}
373-
MacArgs::Eq(eq_span, token) => {
373+
MacArgs::Eq(eq_span, MacArgsEq::Ast(expr)) => {
374374
vis.visit_span(eq_span);
375-
if T::VISIT_TOKENS {
376-
visit_token(token, vis);
377-
} else {
378-
// The value in `#[key = VALUE]` must be visited as an expression for backward
379-
// compatibility, so that macros can be expanded in that position.
380-
match &mut token.kind {
381-
token::Interpolated(nt) => match Lrc::make_mut(nt) {
382-
token::NtExpr(expr) => vis.visit_expr(expr),
383-
t => panic!("unexpected token in key-value attribute: {:?}", t),
384-
},
385-
t => panic!("unexpected token in key-value attribute: {:?}", t),
386-
}
387-
}
375+
vis.visit_expr(expr);
376+
}
377+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
378+
unreachable!("in literal form when visiting mac args eq: {:?}", lit)
388379
}
389380
}
390381
}
@@ -738,7 +729,7 @@ pub fn visit_token<T: MutVisitor>(t: &mut Token, vis: &mut T) {
738729
}
739730
token::Interpolated(nt) => {
740731
let mut nt = Lrc::make_mut(nt);
741-
visit_interpolated(&mut nt, vis);
732+
visit_nonterminal(&mut nt, vis);
742733
}
743734
_ => {}
744735
}
@@ -769,7 +760,7 @@ pub fn visit_token<T: MutVisitor>(t: &mut Token, vis: &mut T) {
769760
// contain multiple items, but decided against it when I looked at
770761
// `parse_item_or_view_item` and tried to figure out what I would do with
771762
// multiple items there....
772-
pub fn visit_interpolated<T: MutVisitor>(nt: &mut token::Nonterminal, vis: &mut T) {
763+
pub fn visit_nonterminal<T: MutVisitor>(nt: &mut token::Nonterminal, vis: &mut T) {
773764
match nt {
774765
token::NtItem(item) => visit_clobber(item, |item| {
775766
// This is probably okay, because the only visitors likely to

‎compiler/rustc_ast/src/token.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,15 @@ pub enum TokenKind {
237237
/// treat regular and interpolated lifetime identifiers in the same way.
238238
Lifetime(Symbol),
239239

240+
/// An embedded AST node, as produced by a macro. This only exists for
241+
/// historical reasons. We'd like to get rid of it, for multiple reasons.
242+
/// - It's conceptually very strange. Saying a token can contain an AST
243+
/// node is like saying, in natural language, that a word can contain a
244+
/// sentence.
245+
/// - It requires special handling in a bunch of places in the parser.
246+
/// - It prevents `Token` from implementing `Copy`.
247+
/// It adds complexity and likely slows things down. Please don't add new
248+
/// occurrences of this token kind!
240249
Interpolated(Lrc<Nonterminal>),
241250

242251
/// A doc comment token.

‎compiler/rustc_ast/src/visit.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
//! those that are created by the expansion of a macro.
1515
1616
use crate::ast::*;
17-
use crate::token;
1817

1918
use rustc_span::symbol::{Ident, Symbol};
2019
use rustc_span::Span;
@@ -940,14 +939,9 @@ pub fn walk_mac_args<'a, V: Visitor<'a>>(visitor: &mut V, args: &'a MacArgs) {
940939
match args {
941940
MacArgs::Empty => {}
942941
MacArgs::Delimited(_dspan, _delim, _tokens) => {}
943-
// The value in `#[key = VALUE]` must be visited as an expression for backward
944-
// compatibility, so that macros can be expanded in that position.
945-
MacArgs::Eq(_eq_span, token) => match &token.kind {
946-
token::Interpolated(nt) => match &**nt {
947-
token::NtExpr(expr) => visitor.visit_expr(expr),
948-
t => panic!("unexpected token in key-value attribute: {:?}", t),
949-
},
950-
t => panic!("unexpected token in key-value attribute: {:?}", t),
951-
},
942+
MacArgs::Eq(_eq_span, MacArgsEq::Ast(expr)) => visitor.visit_expr(expr),
943+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
944+
unreachable!("in literal form when walking mac args eq: {:?}", lit)
945+
}
952946
}
953947
}

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@
3838
#![recursion_limit = "256"]
3939
#![allow(rustc::potential_query_instability)]
4040

41-
use rustc_ast::token::{Delimiter, Token};
42-
use rustc_ast::tokenstream::{CanSynthesizeMissingTokens, TokenStream, TokenTree};
41+
use rustc_ast::tokenstream::{CanSynthesizeMissingTokens, TokenStream};
4342
use rustc_ast::visit;
4443
use rustc_ast::{self as ast, *};
4544
use rustc_ast_pretty::pprust;
@@ -884,37 +883,24 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
884883
)
885884
}
886885
// This is an inert key-value attribute - it will never be visible to macros
887-
// after it gets lowered to HIR. Therefore, we can synthesize tokens with fake
888-
// spans to handle nonterminals in `#[doc]` (e.g. `#[doc = $e]`).
889-
MacArgs::Eq(eq_span, ref token) => {
890-
// In valid code the value is always representable as a single literal token.
891-
fn unwrap_single_token(sess: &Session, tokens: TokenStream, span: Span) -> Token {
892-
if tokens.len() != 1 {
893-
sess.diagnostic()
894-
.delay_span_bug(span, "multiple tokens in key-value attribute's value");
895-
}
896-
match tokens.into_trees().next() {
897-
Some(TokenTree::Token(token)) => token,
898-
Some(TokenTree::Delimited(_, delim, tokens)) => {
899-
if delim != Delimiter::Invisible {
900-
sess.diagnostic().delay_span_bug(
901-
span,
902-
"unexpected delimiter in key-value attribute's value",
903-
);
904-
}
905-
unwrap_single_token(sess, tokens, span)
906-
}
907-
None => Token::dummy(),
886+
// after it gets lowered to HIR. Therefore, we can extract literals to handle
887+
// nonterminals in `#[doc]` (e.g. `#[doc = $e]`).
888+
MacArgs::Eq(eq_span, MacArgsEq::Ast(ref expr)) => {
889+
// In valid code the value always ends up as a single literal. Otherwise, a dummy
890+
// literal suffices because the error is handled elsewhere.
891+
let lit = if let ExprKind::Lit(lit) = &expr.kind {
892+
lit.clone()
893+
} else {
894+
Lit {
895+
token: token::Lit::new(token::LitKind::Err, kw::Empty, None),
896+
kind: LitKind::Err(kw::Empty),
897+
span: DUMMY_SP,
908898
}
909-
}
910-
911-
let tokens = FlattenNonterminals {
912-
parse_sess: &self.sess.parse_sess,
913-
synthesize_tokens: CanSynthesizeMissingTokens::Yes,
914-
nt_to_tokenstream: self.nt_to_tokenstream,
915-
}
916-
.process_token(token.clone());
917-
MacArgs::Eq(eq_span, unwrap_single_token(self.sess, tokens, token.span))
899+
};
900+
MacArgs::Eq(eq_span, MacArgsEq::Hir(lit))
901+
}
902+
MacArgs::Eq(_, MacArgsEq::Hir(ref lit)) => {
903+
unreachable!("in literal form when lowering mac args eq: {:?}", lit)
918904
}
919905
}
920906
}

‎compiler/rustc_ast_pretty/src/pprust/state.rs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_ast::util::comments::{gather_comments, Comment, CommentStyle};
1313
use rustc_ast::util::parser;
1414
use rustc_ast::{self as ast, BlockCheckMode, PatKind, RangeEnd, RangeSyntax};
1515
use rustc_ast::{attr, Term};
16-
use rustc_ast::{GenericArg, MacArgs};
16+
use rustc_ast::{GenericArg, MacArgs, MacArgsEq};
1717
use rustc_ast::{GenericBound, SelfKind, TraitBoundModifier};
1818
use rustc_ast::{InlineAsmOperand, InlineAsmRegOrRegClass};
1919
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
@@ -469,14 +469,22 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
469469
true,
470470
span,
471471
),
472-
MacArgs::Empty | MacArgs::Eq(..) => {
472+
MacArgs::Empty => {
473473
self.print_path(&item.path, false, 0);
474-
if let MacArgs::Eq(_, token) = &item.args {
475-
self.space();
476-
self.word_space("=");
477-
let token_str = self.token_to_string_ext(token, true);
478-
self.word(token_str);
479-
}
474+
}
475+
MacArgs::Eq(_, MacArgsEq::Ast(expr)) => {
476+
self.print_path(&item.path, false, 0);
477+
self.space();
478+
self.word_space("=");
479+
let token_str = self.expr_to_string(expr);
480+
self.word(token_str);
481+
}
482+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
483+
self.print_path(&item.path, false, 0);
484+
self.space();
485+
self.word_space("=");
486+
let token_str = self.literal_to_string(lit);
487+
self.word(token_str);
480488
}
481489
}
482490
self.end();
@@ -817,6 +825,10 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
817825
Self::to_string(|s| s.print_expr(e))
818826
}
819827

828+
fn literal_to_string(&self, lit: &ast::Lit) -> String {
829+
Self::to_string(|s| s.print_literal(lit))
830+
}
831+
820832
fn tt_to_string(&self, tt: &TokenTree) -> String {
821833
Self::to_string(|s| s.print_tt(tt, false))
822834
}

‎compiler/rustc_parse/src/parser/mod.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,10 @@ use rustc_ast::tokenstream::{TokenStream, TokenTree};
2626
use rustc_ast::AttrId;
2727
use rustc_ast::DUMMY_NODE_ID;
2828
use rustc_ast::{self as ast, AnonConst, AstLike, AttrStyle, AttrVec, Const, CrateSugar, Extern};
29-
use rustc_ast::{Async, Expr, ExprKind, MacArgs, MacDelimiter, Mutability, StrLit, Unsafe};
30-
use rustc_ast::{Visibility, VisibilityKind};
29+
use rustc_ast::{Async, Expr, ExprKind, MacArgs, MacArgsEq, MacDelimiter, Mutability, StrLit};
30+
use rustc_ast::{Unsafe, Visibility, VisibilityKind};
3131
use rustc_ast_pretty::pprust;
3232
use rustc_data_structures::fx::FxHashMap;
33-
use rustc_data_structures::sync::Lrc;
3433
use rustc_errors::PResult;
3534
use rustc_errors::{
3635
struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed, FatalError, MultiSpan,
@@ -1157,13 +1156,7 @@ impl<'a> Parser<'a> {
11571156
} else if !delimited_only {
11581157
if self.eat(&token::Eq) {
11591158
let eq_span = self.prev_token.span;
1160-
1161-
// Collect tokens because they are used during lowering to HIR.
1162-
let expr = self.parse_expr_force_collect()?;
1163-
let span = expr.span;
1164-
1165-
let token_kind = token::Interpolated(Lrc::new(token::NtExpr(expr)));
1166-
MacArgs::Eq(eq_span, Token::new(token_kind, span))
1159+
MacArgs::Eq(eq_span, MacArgsEq::Ast(self.parse_expr_force_collect()?))
11671160
} else {
11681161
MacArgs::Empty
11691162
}

0 commit comments

Comments
 (0)