Skip to content

Commit 99f5945

Browse files
committed
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.
1 parent ae5f67f commit 99f5945

21 files changed

+174
-88
lines changed

compiler/rustc_ast/src/ast.rs

+58-6
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};
@@ -1526,7 +1526,7 @@ impl MacCall {
15261526
}
15271527

15281528
/// Arguments passed to an attribute or a function-like macro.
1529-
#[derive(Clone, Encodable, Decodable, Debug, HashStable_Generic)]
1529+
#[derive(Clone, Encodable, Decodable, Debug)]
15301530
pub enum MacArgs {
15311531
/// No arguments - `#[attr]`.
15321532
Empty,
@@ -1536,11 +1536,20 @@ pub enum MacArgs {
15361536
Eq(
15371537
/// Span of the `=` token.
15381538
Span,
1539-
/// "value" as a nonterminal token.
1540-
Token,
1539+
/// The "value".
1540+
MacArgsEq,
15411541
),
15421542
}
15431543

1544+
// The RHS of a `MacArgs::Eq` starts out as an expression. Once macro expansion
1545+
// is completed, all cases end up either as a literal, which is the form used
1546+
// after lowering to HIR, or as an error.
1547+
#[derive(Clone, Encodable, Decodable, Debug)]
1548+
pub enum MacArgsEq {
1549+
Ast(P<Expr>),
1550+
Hir(Lit),
1551+
}
1552+
15441553
impl MacArgs {
15451554
pub fn delim(&self) -> Option<Delimiter> {
15461555
match self {
@@ -1553,7 +1562,10 @@ impl MacArgs {
15531562
match self {
15541563
MacArgs::Empty => None,
15551564
MacArgs::Delimited(dspan, ..) => Some(dspan.entire()),
1556-
MacArgs::Eq(eq_span, token) => Some(eq_span.to(token.span)),
1565+
MacArgs::Eq(eq_span, MacArgsEq::Ast(expr)) => Some(eq_span.to(expr.span)),
1566+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
1567+
unreachable!("in literal form when getting span: {:?}", lit);
1568+
}
15571569
}
15581570
}
15591571

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

@@ -1574,6 +1602,30 @@ impl MacArgs {
15741602
}
15751603
}
15761604

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

compiler/rustc_ast/src/attr/mod.rs

+19-4
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

+5-10
Original file line numberDiff line numberDiff line change
@@ -370,17 +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-
// The value in `#[key = VALUE]` must be visited as an expression for backward
376-
// compatibility, so that macros can be expanded in that position.
377-
match &mut token.kind {
378-
token::Interpolated(nt) => match Lrc::make_mut(nt) {
379-
token::NtExpr(expr) => vis.visit_expr(expr),
380-
t => panic!("unexpected token in key-value attribute: {:?}", t),
381-
},
382-
t => panic!("unexpected token in key-value attribute: {:?}", t),
383-
}
375+
vis.visit_expr(expr);
376+
}
377+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
378+
unreachable!("in literal form when visiting mac args eq: {:?}", lit)
384379
}
385380
}
386381
}

compiler/rustc_ast/src/visit.rs

+4-10
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;
@@ -937,14 +936,9 @@ pub fn walk_mac_args<'a, V: Visitor<'a>>(visitor: &mut V, args: &'a MacArgs) {
937936
match args {
938937
MacArgs::Empty => {}
939938
MacArgs::Delimited(_dspan, _delim, _tokens) => {}
940-
// The value in `#[key = VALUE]` must be visited as an expression for backward
941-
// compatibility, so that macros can be expanded in that position.
942-
MacArgs::Eq(_eq_span, token) => match &token.kind {
943-
token::Interpolated(nt) => match &**nt {
944-
token::NtExpr(expr) => visitor.visit_expr(expr),
945-
t => panic!("unexpected token in key-value attribute: {:?}", t),
946-
},
947-
t => panic!("unexpected token in key-value attribute: {:?}", t),
948-
},
939+
MacArgs::Eq(_eq_span, MacArgsEq::Ast(expr)) => visitor.visit_expr(expr),
940+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => {
941+
unreachable!("in literal form when walking mac args eq: {:?}", lit)
942+
}
949943
}
950944
}

compiler/rustc_ast_lowering/src/lib.rs

+16-16
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
#![recursion_limit = "256"]
3939
#![allow(rustc::potential_query_instability)]
4040

41-
use rustc_ast::token::{self, Token, TokenKind};
4241
use rustc_ast::tokenstream::{CanSynthesizeMissingTokens, TokenStream};
4342
use rustc_ast::visit;
4443
use rustc_ast::{self as ast, *};
@@ -874,23 +873,24 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
874873
)
875874
}
876875
// This is an inert key-value attribute - it will never be visible to macros
877-
// after it gets lowered to HIR. Therefore, we can synthesize tokens with fake
878-
// spans to handle nonterminals in `#[doc]` (e.g. `#[doc = $e]`).
879-
MacArgs::Eq(eq_span, ref token) => {
880-
// In valid code the value is always representable as a single literal token.
881-
// Otherwise, a dummy token suffices because the error is handled elsewhere.
882-
let token = if let token::Interpolated(nt) = &token.kind
883-
&& let token::NtExpr(expr) = &**nt
884-
{
885-
if let ExprKind::Lit(Lit { token, span, .. }) = expr.kind {
886-
Token::new(TokenKind::Literal(token), span)
887-
} else {
888-
Token::dummy()
889-
}
876+
// after it gets lowered to HIR. Therefore, we can extract literals to handle
877+
// nonterminals in `#[doc]` (e.g. `#[doc = $e]`).
878+
MacArgs::Eq(eq_span, MacArgsEq::Ast(ref expr)) => {
879+
// In valid code the value always ends up as a single literal. Otherwise, a dummy
880+
// literal suffices because the error is handled elsewhere.
881+
let lit = if let ExprKind::Lit(lit) = &expr.kind {
882+
lit.clone()
890883
} else {
891-
unreachable!()
884+
Lit {
885+
token: token::Lit::new(token::LitKind::Err, kw::Empty, None),
886+
kind: LitKind::Err(kw::Empty),
887+
span: DUMMY_SP,
888+
}
892889
};
893-
MacArgs::Eq(eq_span, token)
890+
MacArgs::Eq(eq_span, MacArgsEq::Hir(lit))
891+
}
892+
MacArgs::Eq(_, MacArgsEq::Hir(ref lit)) => {
893+
unreachable!("in literal form when lowering mac args eq: {:?}", lit)
894894
}
895895
}
896896
}

compiler/rustc_ast_pretty/src/pprust/state.rs

+14-3
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};
@@ -472,11 +472,18 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
472472
MacArgs::Empty => {
473473
self.print_path(&item.path, false, 0);
474474
}
475-
MacArgs::Eq(_, token) => {
475+
MacArgs::Eq(_, MacArgsEq::Ast(expr)) => {
476476
self.print_path(&item.path, false, 0);
477477
self.space();
478478
self.word_space("=");
479-
let token_str = self.token_to_string_ext(token, true);
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);
480487
self.word(token_str);
481488
}
482489
}
@@ -818,6 +825,10 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
818825
Self::to_string(|s| s.print_expr(e))
819826
}
820827

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

compiler/rustc_parse/src/parser/mod.rs

+3-10
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
}

compiler/rustc_parse/src/validate_attr.rs

+32-7
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
33
use crate::parse_in;
44

5-
use rustc_ast::tokenstream::{DelimSpan, TokenTree};
6-
use rustc_ast::{self as ast, Attribute, MacArgs, MacDelimiter, MetaItem, MetaItemKind};
5+
use rustc_ast::tokenstream::DelimSpan;
6+
use rustc_ast::{self as ast, Attribute, MacArgs, MacArgsEq, MacDelimiter, MetaItem, MetaItemKind};
7+
use rustc_ast_pretty::pprust;
78
use rustc_errors::{Applicability, FatalError, PResult};
89
use rustc_feature::{AttributeTemplate, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
910
use rustc_session::lint::builtin::ILL_FORMED_ATTRIBUTE_INPUT;
@@ -42,16 +43,40 @@ pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, Meta
4243
path: item.path.clone(),
4344
kind: match &item.args {
4445
MacArgs::Empty => MetaItemKind::Word,
45-
MacArgs::Eq(_, t) => {
46-
let t = TokenTree::Token(t.clone()).into();
47-
let v = parse_in(sess, t, "name value", |p| p.parse_unsuffixed_lit())?;
48-
MetaItemKind::NameValue(v)
49-
}
5046
MacArgs::Delimited(dspan, delim, t) => {
5147
check_meta_bad_delim(sess, *dspan, *delim, "wrong meta list delimiters");
5248
let nmis = parse_in(sess, t.clone(), "meta list", |p| p.parse_meta_seq_top())?;
5349
MetaItemKind::List(nmis)
5450
}
51+
MacArgs::Eq(_, MacArgsEq::Ast(expr)) => {
52+
if let ast::ExprKind::Lit(lit) = &expr.kind {
53+
if !lit.kind.is_unsuffixed() {
54+
let mut err = sess.span_diagnostic.struct_span_err(
55+
lit.span,
56+
"suffixed literals are not allowed in attributes",
57+
);
58+
err.help(
59+
"instead of using a suffixed literal (`1u8`, `1.0f32`, etc.), \
60+
use an unsuffixed version (`1`, `1.0`, etc.)",
61+
);
62+
return Err(err);
63+
} else {
64+
MetaItemKind::NameValue(lit.clone())
65+
}
66+
} else {
67+
// The non-error case can happen with e.g. `#[foo = 1+1]`. The error case can
68+
// happen with e.g. `#[foo = include_str!("non-existent-file.rs")]`; in that
69+
// case we delay the error because an earlier error will have already been
70+
// reported.
71+
let msg = format!("unexpected expression: `{}`", pprust::expr_to_string(expr));
72+
let mut err = sess.span_diagnostic.struct_span_err(expr.span, msg);
73+
if let ast::ExprKind::Err = expr.kind {
74+
err.downgrade_to_delayed_bug();
75+
}
76+
return Err(err);
77+
}
78+
}
79+
MacArgs::Eq(_, MacArgsEq::Hir(lit)) => MetaItemKind::NameValue(lit.clone()),
5580
},
5681
})
5782
}

0 commit comments

Comments
 (0)