Rollup merge of #73102 - petrochenkov:flatgroup, r=Aaron1011

proc_macro: Stop flattening groups with dummy spans Reduce the scope of the hack described in #72545 (comment). We still pass AST fragments to attribute and derive macros as single nonterminal tokens rather than as tokens streams, but now use a precise flag instead of the span-based heuristic that could do lead to incorrect behavior in unrelated cases. #73345 attempts to fully resolve this issue, but there are some compatibility issues to be addressed.
rust-lang · Jun 26, 2020 · 2aee608 · 2aee608
2 parents a25fbb9 + 77b0ed7
commit 2aee608
Show file tree

Hide file tree

Showing 18 changed files with 76 additions and 50 deletions.
diff --git a/src/librustc_ast/attr/mod.rs b/src/librustc_ast/attr/mod.rs
@@ -475,7 +475,7 @@ impl MetaItem {
                 let span = span.with_hi(segments.last().unwrap().ident.span.hi());
                 Path { span, segments }
             }
-            Some(TokenTree::Token(Token { kind: token::Interpolated(nt), .. })) => match *nt {
+            Some(TokenTree::Token(Token { kind: token::Interpolated(nt, _), .. })) => match *nt {
                 token::Nonterminal::NtMeta(ref item) => return item.meta(item.path.span),
                 token::Nonterminal::NtPath(ref path) => path.clone(),
                 _ => return None,

diff --git a/src/librustc_ast/mut_visit.rs b/src/librustc_ast/mut_visit.rs
@@ -656,7 +656,7 @@ pub fn noop_visit_token<T: MutVisitor>(t: &mut Token, vis: &mut T) {
             *span = ident.span;
             return; // Avoid visiting the span for the second time.
         }
-        token::Interpolated(nt) => {
+        token::Interpolated(nt, _) => {
             let mut nt = Lrc::make_mut(nt);
             vis.visit_interpolated(&mut nt);
         }

diff --git a/src/librustc_ast/token.rs b/src/librustc_ast/token.rs
@@ -182,6 +182,15 @@ fn ident_can_begin_type(name: Symbol, span: Span, is_raw: bool) -> bool {
             .contains(&name)
 }
 
+/// A hack used to pass AST fragments to attribute and derive macros
+/// as a single nonterminal token instead of a token stream.
+/// FIXME: It needs to be removed, but there are some compatibility issues (see #73345).
+#[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable_Generic)]
+pub enum FlattenGroup {
+    Yes,
+    No,
+}
+
 #[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable_Generic)]
 pub enum TokenKind {
     /* Expression-operator symbols. */
@@ -236,7 +245,7 @@ pub enum TokenKind {
     /// treat regular and interpolated lifetime identifiers in the same way.
     Lifetime(Symbol),
 
-    Interpolated(Lrc<Nonterminal>),
+    Interpolated(Lrc<Nonterminal>, FlattenGroup),
 
     // Can be expanded into several tokens.
     /// A doc comment.
@@ -343,7 +352,7 @@ impl Token {
     /// if they keep spans or perform edition checks.
     pub fn uninterpolated_span(&self) -> Span {
         match &self.kind {
-            Interpolated(nt) => nt.span(),
+            Interpolated(nt, _) => nt.span(),
             _ => self.span,
         }
     }
@@ -382,7 +391,7 @@ impl Token {
             ModSep                            | // global path
             Lifetime(..)                      | // labeled loop
             Pound                             => true, // expression attributes
-            Interpolated(ref nt) => match **nt {
+            Interpolated(ref nt, _) => match **nt {
                 NtLiteral(..) |
                 NtExpr(..)    |
                 NtBlock(..)   |
@@ -408,7 +417,7 @@ impl Token {
             Lifetime(..)                | // lifetime bound in trait object
             Lt | BinOp(Shl)             | // associated path
             ModSep                      => true, // global path
-            Interpolated(ref nt) => match **nt {
+            Interpolated(ref nt, _) => match **nt {
                 NtTy(..) | NtPath(..) => true,
                 _ => false,
             },
@@ -420,7 +429,7 @@ impl Token {
     pub fn can_begin_const_arg(&self) -> bool {
         match self.kind {
             OpenDelim(Brace) => true,
-            Interpolated(ref nt) => match **nt {
+            Interpolated(ref nt, _) => match **nt {
                 NtExpr(..) | NtBlock(..) | NtLiteral(..) => true,
                 _ => false,
             },
@@ -455,7 +464,7 @@ impl Token {
         match self.uninterpolate().kind {
             Literal(..) | BinOp(Minus) => true,
             Ident(name, false) if name.is_bool_lit() => true,
-            Interpolated(ref nt) => match &**nt {
+            Interpolated(ref nt, _) => match &**nt {
                 NtLiteral(_) => true,
                 NtExpr(e) => match &e.kind {
                     ast::ExprKind::Lit(_) => true,
@@ -476,7 +485,7 @@ impl Token {
     // otherwise returns the original token.
     pub fn uninterpolate(&self) -> Cow<'_, Token> {
         match &self.kind {
-            Interpolated(nt) => match **nt {
+            Interpolated(nt, _) => match **nt {
                 NtIdent(ident, is_raw) => {
                     Cow::Owned(Token::new(Ident(ident.name, is_raw), ident.span))
                 }
@@ -523,7 +532,7 @@ impl Token {
 
     /// Returns `true` if the token is an interpolated path.
     fn is_path(&self) -> bool {
-        if let Interpolated(ref nt) = self.kind {
+        if let Interpolated(ref nt, _) = self.kind {
             if let NtPath(..) = **nt {
                 return true;
             }
@@ -535,7 +544,7 @@ impl Token {
     /// That is, is this a pre-parsed expression dropped into the token stream
     /// (which happens while parsing the result of macro expansion)?
     pub fn is_whole_expr(&self) -> bool {
-        if let Interpolated(ref nt) = self.kind {
+        if let Interpolated(ref nt, _) = self.kind {
             if let NtExpr(_) | NtLiteral(_) | NtPath(_) | NtIdent(..) | NtBlock(_) = **nt {
                 return true;
             }
@@ -546,7 +555,7 @@ impl Token {
 
     // Is the token an interpolated block (`$b:block`)?
     pub fn is_whole_block(&self) -> bool {
-        if let Interpolated(ref nt) = self.kind {
+        if let Interpolated(ref nt, _) = self.kind {
             if let NtBlock(..) = **nt {
                 return true;
             }
@@ -724,7 +733,7 @@ impl Token {
                 b == d && (a == c || a == kw::DollarCrate || c == kw::DollarCrate)
             }
 
-            (&Interpolated(_), &Interpolated(_)) => false,
+            (&Interpolated(..), &Interpolated(..)) => false,
 
             _ => panic!("forgot to add a token?"),
         }

diff --git a/src/librustc_ast/util/literal.rs b/src/librustc_ast/util/literal.rs
@@ -205,7 +205,7 @@ impl Lit {
                 token::Lit::new(token::Bool, name, None)
             }
             token::Literal(lit) => lit,
-            token::Interpolated(ref nt) => {
+            token::Interpolated(ref nt, _) => {
                 if let token::NtExpr(expr) | token::NtLiteral(expr) = &**nt {
                     if let ast::ExprKind::Lit(lit) = &expr.kind {
                         return Ok(lit.clone());

diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs
@@ -1027,7 +1027,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
     fn lower_token(&mut self, token: Token) -> TokenStream {
         match token.kind {
-            token::Interpolated(nt) => {
+            token::Interpolated(nt, _) => {
                 let tts = (self.nt_to_tokenstream)(&nt, &self.sess.parse_sess, token.span);
                 self.lower_token_stream(tts)
             }

diff --git a/src/librustc_ast_pretty/pprust.rs b/src/librustc_ast_pretty/pprust.rs
@@ -266,7 +266,7 @@ fn token_kind_to_string_ext(tok: &TokenKind, convert_dollar_crate: Option<Span>)
         token::Shebang(s) => format!("/* shebang: {}*/", s),
         token::Unknown(s) => s.to_string(),
 
-        token::Interpolated(ref nt) => nonterminal_to_string(nt),
+        token::Interpolated(ref nt, _) => nonterminal_to_string(nt),
     }
 }
 

diff --git a/src/librustc_expand/base.rs b/src/librustc_expand/base.rs
@@ -4,7 +4,7 @@ use crate::module::DirectoryOwnership;
 use rustc_ast::ast::{self, Attribute, NodeId, PatKind};
 use rustc_ast::mut_visit::{self, MutVisitor};
 use rustc_ast::ptr::P;
-use rustc_ast::token;
+use rustc_ast::token::{self, FlattenGroup};
 use rustc_ast::tokenstream::{self, TokenStream, TokenTree};
 use rustc_ast::visit::{AssocCtxt, Visitor};
 use rustc_attr::{self as attr, Deprecation, HasAttrs, Stability};
@@ -142,7 +142,7 @@ impl Annotatable {
             | Annotatable::StructField(..)
             | Annotatable::Variant(..) => panic!("unexpected annotatable"),
         };
-        TokenTree::token(token::Interpolated(Lrc::new(nt)), DUMMY_SP).into()
+        TokenTree::token(token::Interpolated(Lrc::new(nt), FlattenGroup::Yes), DUMMY_SP).into()
     }
 
     pub fn expect_item(self) -> P<ast::Item> {
@@ -374,7 +374,7 @@ where
         impl MutVisitor for AvoidInterpolatedIdents {
             fn visit_tt(&mut self, tt: &mut tokenstream::TokenTree) {
                 if let tokenstream::TokenTree::Token(token) = tt {
-                    if let token::Interpolated(nt) = &token.kind {
+                    if let token::Interpolated(nt, _) = &token.kind {
                         if let token::NtIdent(ident, is_raw) = **nt {
                             *tt = tokenstream::TokenTree::token(
                                 token::Ident(ident.name, is_raw),

diff --git a/src/librustc_expand/mbe/macro_parser.rs b/src/librustc_expand/mbe/macro_parser.rs
@@ -785,12 +785,12 @@ fn may_begin_with(token: &Token, name: Symbol) -> bool {
         sym::literal => token.can_begin_literal_maybe_minus(),
         sym::vis => match token.kind {
             // The follow-set of :vis + "priv" keyword + interpolated
-            token::Comma | token::Ident(..) | token::Interpolated(_) => true,
+            token::Comma | token::Ident(..) | token::Interpolated(..) => true,
             _ => token.can_begin_type(),
         },
         sym::block => match token.kind {
             token::OpenDelim(token::Brace) => true,
-            token::Interpolated(ref nt) => match **nt {
+            token::Interpolated(ref nt, _) => match **nt {
                 token::NtItem(_)
                 | token::NtPat(_)
                 | token::NtTy(_)
@@ -804,7 +804,7 @@ fn may_begin_with(token: &Token, name: Symbol) -> bool {
         },
         sym::path | sym::meta => match token.kind {
             token::ModSep | token::Ident(..) => true,
-            token::Interpolated(ref nt) => match **nt {
+            token::Interpolated(ref nt, _) => match **nt {
                 token::NtPath(_) | token::NtMeta(_) => true,
                 _ => may_be_ident(&nt),
             },
@@ -823,12 +823,12 @@ fn may_begin_with(token: &Token, name: Symbol) -> bool {
             token::ModSep |                     // path
             token::Lt |                         // path (UFCS constant)
             token::BinOp(token::Shl) => true,   // path (double UFCS)
-            token::Interpolated(ref nt) => may_be_ident(nt),
+            token::Interpolated(ref nt, _) => may_be_ident(nt),
             _ => false,
         },
         sym::lifetime => match token.kind {
             token::Lifetime(_) => true,
-            token::Interpolated(ref nt) => match **nt {
+            token::Interpolated(ref nt, _) => match **nt {
                 token::NtLifetime(_) | token::NtTT(_) => true,
                 _ => false,
             },

diff --git a/src/librustc_expand/mbe/transcribe.rs b/src/librustc_expand/mbe/transcribe.rs
@@ -4,7 +4,7 @@ use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq, NamedMatch};
 
 use rustc_ast::ast::MacCall;
 use rustc_ast::mut_visit::{self, MutVisitor};
-use rustc_ast::token::{self, NtTT, Token};
+use rustc_ast::token::{self, FlattenGroup, NtTT, Token};
 use rustc_ast::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync::Lrc;
@@ -240,7 +240,10 @@ pub(super) fn transcribe<'a>(
                             result.push(tt.clone().into());
                         } else {
                             marker.visit_span(&mut sp);
-                            let token = TokenTree::token(token::Interpolated(nt.clone()), sp);
+                            let token = TokenTree::token(
+                                token::Interpolated(nt.clone(), FlattenGroup::No),
+                                sp,
+                            );
                             result.push(token.into());
                         }
                     } else {

diff --git a/src/librustc_expand/proc_macro.rs b/src/librustc_expand/proc_macro.rs
@@ -2,7 +2,7 @@ use crate::base::{self, *};
 use crate::proc_macro_server;
 
 use rustc_ast::ast::{self, ItemKind, MetaItemKind, NestedMetaItem};
-use rustc_ast::token;
+use rustc_ast::token::{self, FlattenGroup};
 use rustc_ast::tokenstream::{self, TokenStream};
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::{Applicability, ErrorReported};
@@ -102,7 +102,7 @@ impl MultiItemModifier for ProcMacroDerive {
             }
         }
 
-        let token = token::Interpolated(Lrc::new(token::NtItem(item)));
+        let token = token::Interpolated(Lrc::new(token::NtItem(item)), FlattenGroup::Yes);
         let input = tokenstream::TokenTree::token(token, DUMMY_SP).into();
 
         let server = proc_macro_server::Rustc::new(ecx);

diff --git a/src/librustc_expand/proc_macro_server.rs b/src/librustc_expand/proc_macro_server.rs
@@ -1,7 +1,7 @@
 use crate::base::ExtCtxt;
 
 use rustc_ast::ast;
-use rustc_ast::token;
+use rustc_ast::token::{self, FlattenGroup};
 use rustc_ast::tokenstream::{self, DelimSpan, IsJoint::*, TokenStream, TreeAndJoint};
 use rustc_ast::util::comments;
 use rustc_ast_pretty::pprust;
@@ -60,7 +60,12 @@ impl FromInternal<(TreeAndJoint, &'_ ParseSess, &'_ mut Vec<Self>)>
         let Token { kind, span } = match tree {
             tokenstream::TokenTree::Delimited(span, delim, tts) => {
                 let delimiter = Delimiter::from_internal(delim);
-                return TokenTree::Group(Group { delimiter, stream: tts, span });
+                return TokenTree::Group(Group {
+                    delimiter,
+                    stream: tts,
+                    span,
+                    flatten: FlattenGroup::No,
+                });
             }
             tokenstream::TokenTree::Token(token) => token,
         };
@@ -167,19 +172,21 @@ impl FromInternal<(TreeAndJoint, &'_ ParseSess, &'_ mut Vec<Self>)>
                     delimiter: Delimiter::Bracket,
                     stream,
                     span: DelimSpan::from_single(span),
+                    flatten: FlattenGroup::No,
                 }));
                 if style == ast::AttrStyle::Inner {
                     stack.push(tt!(Punct::new('!', false)));
                 }
                 tt!(Punct::new('#', false))
             }
 
-            Interpolated(nt) => {
+            Interpolated(nt, flatten) => {
                 let stream = nt_to_tokenstream(&nt, sess, span);
                 TokenTree::Group(Group {
                     delimiter: Delimiter::None,
                     stream,
                     span: DelimSpan::from_single(span),
+                    flatten,
                 })
             }
 
@@ -195,7 +202,7 @@ impl ToInternal<TokenStream> for TokenTree<Group, Punct, Ident, Literal> {
 
         let (ch, joint, span) = match self {
             TokenTree::Punct(Punct { ch, joint, span }) => (ch, joint, span),
-            TokenTree::Group(Group { delimiter, stream, span }) => {
+            TokenTree::Group(Group { delimiter, stream, span, .. }) => {
                 return tokenstream::TokenTree::Delimited(span, delimiter.to_internal(), stream)
                     .into();
             }
@@ -283,6 +290,10 @@ pub struct Group {
     delimiter: Delimiter,
     stream: TokenStream,
     span: DelimSpan,
+    /// A hack used to pass AST fragments to attribute and derive macros
+    /// as a single nonterminal token instead of a token stream.
+    /// FIXME: It needs to be removed, but there are some compatibility issues (see #73345).
+    flatten: FlattenGroup,
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
@@ -437,14 +448,12 @@ impl server::TokenStreamIter for Rustc<'_> {
                 let next = iter.cursor.next_with_joint()?;
                 Some(TokenTree::from_internal((next, self.sess, &mut iter.stack)))
             })?;
-            // HACK: The condition "dummy span + group with empty delimiter" represents an AST
-            // fragment approximately converted into a token stream. This may happen, for
-            // example, with inputs to proc macro attributes, including derives. Such "groups"
-            // need to flattened during iteration over stream's token trees.
-            // Eventually this needs to be removed in favor of keeping original token trees
-            // and not doing the roundtrip through AST.
+            // A hack used to pass AST fragments to attribute and derive macros
+            // as a single nonterminal token instead of a token stream.
+            // Such token needs to be "unwrapped" and not represented as a delimited group.
+            // FIXME: It needs to be removed, but there are some compatibility issues (see #73345).
             if let TokenTree::Group(ref group) = tree {
-                if group.delimiter == Delimiter::None && group.span.entire().is_dummy() {
+                if matches!(group.flatten, FlattenGroup::Yes) {
                     iter.cursor.append(group.stream.clone());
                     continue;
                 }
@@ -456,7 +465,12 @@ impl server::TokenStreamIter for Rustc<'_> {
 
 impl server::Group for Rustc<'_> {
     fn new(&mut self, delimiter: Delimiter, stream: Self::TokenStream) -> Self::Group {
-        Group { delimiter, stream, span: DelimSpan::from_single(server::Span::call_site(self)) }
+        Group {
+            delimiter,
+            stream,
+            span: DelimSpan::from_single(server::Span::call_site(self)),
+            flatten: FlattenGroup::No,
+        }
     }
     fn delimiter(&mut self, group: &Self::Group) -> Delimiter {
         group.delimiter

diff --git a/src/librustc_parse/parser/attr.rs b/src/librustc_parse/parser/attr.rs
@@ -155,7 +155,7 @@ impl<'a> Parser<'a> {
     /// The delimiters or `=` are still put into the resulting token stream.
     pub fn parse_attr_item(&mut self) -> PResult<'a, ast::AttrItem> {
         let item = match self.token.kind {
-            token::Interpolated(ref nt) => match **nt {
+            token::Interpolated(ref nt, _) => match **nt {
                 Nonterminal::NtMeta(ref item) => Some(item.clone().into_inner()),
                 _ => None,
             },
@@ -254,7 +254,7 @@ impl<'a> Parser<'a> {
     ///     meta_item_inner : (meta_item | UNSUFFIXED_LIT) (',' meta_item_inner)? ;
     pub fn parse_meta_item(&mut self) -> PResult<'a, ast::MetaItem> {
         let nt_meta = match self.token.kind {
-            token::Interpolated(ref nt) => match **nt {
+            token::Interpolated(ref nt, _) => match **nt {
                 token::NtMeta(ref e) => Some(e.clone()),
                 _ => None,
             },

diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs
@@ -26,7 +26,7 @@ use std::mem;
 /// `token::Interpolated` tokens.
 macro_rules! maybe_whole_expr {
     ($p:expr) => {
-        if let token::Interpolated(nt) = &$p.token.kind {
+        if let token::Interpolated(nt, _) = &$p.token.kind {
             match &**nt {
                 token::NtExpr(e) | token::NtLiteral(e) => {
                     let e = e.clone();