WIP

Author: petrochenkov

compiler/rustc_ast/src/ast_traits.rs

@@ -240,7 +240,7 @@ impl HasTokens for Nonterminal {
             Nonterminal::NtPath(path) => path.tokens(),
             Nonterminal::NtVis(vis) => vis.tokens(),
             Nonterminal::NtBlock(block) => block.tokens(),
-            Nonterminal::NtIdent(..) | Nonterminal::NtLifetime(..) => None,
+            Nonterminal::NtLifetime(..) => None,
         }
     }
     fn tokens_mut(&mut self) -> Option<&mut Option<LazyAttrTokenStream>> {
@@ -254,7 +254,7 @@ impl HasTokens for Nonterminal {
             Nonterminal::NtPath(path) => path.tokens_mut(),
             Nonterminal::NtVis(vis) => vis.tokens_mut(),
             Nonterminal::NtBlock(block) => block.tokens_mut(),
-            Nonterminal::NtIdent(..) | Nonterminal::NtLifetime(..) => None,
+            Nonterminal::NtLifetime(..) => None,
         }
     }
 }

compiler/rustc_ast/src/mut_visit.rs

@@ -816,7 +816,6 @@ pub fn visit_nonterminal<T: MutVisitor>(nt: &mut token::Nonterminal, vis: &mut T
         token::NtPat(pat) => vis.visit_pat(pat),
         token::NtExpr(expr) => vis.visit_expr(expr),
         token::NtTy(ty) => vis.visit_ty(ty),
-        token::NtIdent(ident, _is_raw) => vis.visit_ident(ident),
         token::NtLifetime(ident) => vis.visit_ident(ident),
         token::NtLiteral(expr) => vis.visit_expr(expr),
         token::NtMeta(item) => {

compiler/rustc_ast/src/token.rs

@@ -295,9 +295,6 @@ pub enum TokenKind {
     Literal(Lit),
 
     /// Identifier token.
-    /// Do not forget about `NtIdent` when you want to match on identifiers.
-    /// It's recommended to use `Token::(ident,uninterpolate,uninterpolated_span)` to
-    /// treat regular and interpolated identifiers in the same way.
     Ident(Symbol, /* is_raw */ bool),
     /// Lifetime identifier token.
     /// Do not forget about `NtLifetime` when you want to match on lifetime identifiers.
@@ -590,9 +587,6 @@ impl Token {
     pub fn uninterpolate(&self) -> Cow<'_, Token> {
         match &self.kind {
             Interpolated(nt) => match &nt.0 {
-                NtIdent(ident, is_raw) => {
-                    Cow::Owned(Token::new(Ident(ident.name, *is_raw), ident.span))
-                }
                 NtLifetime(ident) => Cow::Owned(Token::new(Lifetime(ident.name), ident.span)),
                 _ => Cow::Borrowed(self),
             },
@@ -606,10 +600,6 @@ impl Token {
         // We avoid using `Token::uninterpolate` here because it's slow.
         match &self.kind {
             &Ident(name, is_raw) => Some((Ident::new(name, self.span), is_raw)),
-            Interpolated(nt) => match &nt.0 {
-                NtIdent(ident, is_raw) => Some((*ident, *is_raw)),
-                _ => None,
-            },
             _ => None,
         }
     }
@@ -836,7 +826,6 @@ pub enum Nonterminal {
     NtPat(P<ast::Pat>),
     NtExpr(P<ast::Expr>),
     NtTy(P<ast::Ty>),
-    NtIdent(Ident, /* is_raw */ bool),
     NtLifetime(Ident),
     NtLiteral(P<ast::Expr>),
     /// Stuff inside brackets for attributes
@@ -932,7 +921,7 @@ impl Nonterminal {
             NtPat(pat) => pat.span,
             NtExpr(expr) | NtLiteral(expr) => expr.span,
             NtTy(ty) => ty.span,
-            NtIdent(ident, _) | NtLifetime(ident) => ident.span,
+            NtLifetime(ident) => ident.span,
             NtMeta(attr_item) => attr_item.span(),
             NtPath(path) => path.span,
             NtVis(vis) => vis.span,
@@ -948,7 +937,6 @@ impl Nonterminal {
             NtExpr(..) => "expression",
             NtLiteral(..) => "literal",
             NtTy(..) => "type",
-            NtIdent(..) => "identifier",
             NtLifetime(..) => "lifetime",
             NtMeta(..) => "attribute",
             NtPath(..) => "path",
@@ -960,9 +948,6 @@ impl Nonterminal {
 impl PartialEq for Nonterminal {
     fn eq(&self, rhs: &Self) -> bool {
         match (self, rhs) {
-            (NtIdent(ident_lhs, is_raw_lhs), NtIdent(ident_rhs, is_raw_rhs)) => {
-                ident_lhs == ident_rhs && is_raw_lhs == is_raw_rhs
-            }
             (NtLifetime(ident_lhs), NtLifetime(ident_rhs)) => ident_lhs == ident_rhs,
             // FIXME: Assume that all "complex" nonterminal are not equal, we can't compare them
             // correctly based on data from AST. This will prevent them from matching each other
@@ -982,7 +967,6 @@ impl fmt::Debug for Nonterminal {
             NtPat(..) => f.pad("NtPat(..)"),
             NtExpr(..) => f.pad("NtExpr(..)"),
             NtTy(..) => f.pad("NtTy(..)"),
-            NtIdent(..) => f.pad("NtIdent(..)"),
             NtLiteral(..) => f.pad("NtLiteral(..)"),
             NtMeta(..) => f.pad("NtMeta(..)"),
             NtPath(..) => f.pad("NtPath(..)"),

compiler/rustc_ast/src/tokenstream.rs

@@ -478,9 +478,6 @@ impl TokenStream {
 
     pub fn from_nonterminal_ast(nt: &Nonterminal) -> TokenStream {
         match nt {
-            Nonterminal::NtIdent(ident, is_raw) => {
-                TokenStream::token_alone(token::Ident(ident.name, *is_raw), ident.span)
-            }
             Nonterminal::NtLifetime(ident) => {
                 TokenStream::token_alone(token::Lifetime(ident.name), ident.span)
             }
@@ -502,9 +499,6 @@ impl TokenStream {
 
     fn flatten_token(token: &Token, spacing: Spacing) -> TokenTree {
         match &token.kind {
-            token::Interpolated(nt) if let token::NtIdent(ident, is_raw) = nt.0 => {
-                TokenTree::Token(Token::new(token::Ident(ident.name, is_raw), ident.span), spacing)
-            }
             token::Interpolated(nt) => TokenTree::Delimited(
                 DelimSpan::from_single(token.span),
                 DelimSpacing::new(Spacing::JointHidden, spacing),

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -715,7 +715,6 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
             token::NtBlock(e) => self.block_to_string(e),
             token::NtStmt(e) => self.stmt_to_string(e),
             token::NtPat(e) => self.pat_to_string(e),
-            token::NtIdent(e, is_raw) => IdentPrinter::for_ast_ident(*e, *is_raw).to_string(),
             token::NtLifetime(e) => e.to_string(),
             token::NtLiteral(e) => self.expr_to_string(e),
             token::NtVis(e) => self.vis_to_string(e),

compiler/rustc_expand/src/mbe/transcribe.rs

@@ -13,7 +13,7 @@ use rustc_errors::DiagnosticBuilder;
 use rustc_errors::{pluralize, PResult};
 use rustc_span::hygiene::{LocalExpnId, Transparency};
 use rustc_span::symbol::{sym, Ident, MacroRulesNormalizedIdent};
-use rustc_span::Span;
+use rustc_span::{try_insert_metavar_span, Span};
 
 use smallvec::{smallvec, SmallVec};
 use std::mem;
@@ -245,6 +245,7 @@ pub(super) fn transcribe<'a>(
                         MatchedTokenTree(tt) => {
                             // `tt`s are emitted into the output stream directly as "raw tokens",
                             // without wrapping them into groups.
+                            marker.visit_span(&mut sp);
                             result.push(maybe_use_metavar_location(cx, &stack, sp, tt));
                         }
                         MatchedNonterminal(nt) => {
@@ -310,6 +311,15 @@ pub(super) fn transcribe<'a>(
     }
 }
 
+/// Store the metavariable span for this original span into a side table.
+/// FIXME: Try to put the metavariable span into `SpanData` instead of a side table (#118517).
+/// An optimal encoding for inlined spans will need to be selected to minimize regressions.
+/// The side table approach is relatively good, but not perfect due to collisions.
+/// The old heuristic below is used to improve spans in case of collisions, but diagnostics are
+/// still degraded sometimes in those cases.
+///
+/// The old heuristic:
+///
 /// Usually metavariables `$var` produce interpolated tokens, which have an additional place for
 /// keeping both the original span and the metavariable span. For `tt` metavariables that's not the
 /// case however, and there's no place for keeping a second span. So we try to give the single
@@ -329,37 +339,48 @@ pub(super) fn transcribe<'a>(
 ///   These are typically used for passing larger amounts of code, and tokens in that code usually
 ///   combine with each other and not with tokens outside of the sequence.
 /// - The metavariable span comes from a different crate, then we prefer the more local span.
-///
-/// FIXME: Find a way to keep both original and metavariable spans for all tokens without
-/// regressing compilation time too much. Several experiments for adding such spans were made in
-/// the past (PR #95580, #118517, #118671) and all showed some regressions.
 fn maybe_use_metavar_location(
     cx: &ExtCtxt<'_>,
     stack: &[Frame<'_>],
     metavar_span: Span,
     orig_tt: &TokenTree,
 ) -> TokenTree {
-    let undelimited_seq = matches!(
-        stack.last(),
-        Some(Frame::Sequence {
-            tts: [_],
-            sep: None,
-            kleene_op: KleeneOp::ZeroOrMore | KleeneOp::OneOrMore,
-            ..
-        })
-    );
-    if undelimited_seq || cx.source_map().is_imported(metavar_span) {
+    let no_collision = match orig_tt {
+        TokenTree::Token(token, ..) => try_insert_metavar_span(token.span, metavar_span),
+        TokenTree::Delimited(dspan, ..) => {
+            try_insert_metavar_span(dspan.open, metavar_span)
+                && try_insert_metavar_span(dspan.close, metavar_span)
+                && try_insert_metavar_span(dspan.entire(), metavar_span)
+        }
+    };
+    let undelimited_seq = || {
+        matches!(
+            stack.last(),
+            Some(Frame::Sequence {
+                tts: [_],
+                sep: None,
+                kleene_op: KleeneOp::ZeroOrMore | KleeneOp::OneOrMore,
+                ..
+            })
+        )
+    };
+    if no_collision || undelimited_seq() || cx.source_map().is_imported(metavar_span) {
         return orig_tt.clone();
     }
 
+    // Setting metavar spans for the heuristic spans gives better opportunities for combining them
+    // with neighboring spans even despite their different syntactic contexts.
     match orig_tt {
         TokenTree::Token(Token { kind, span }, spacing) => {
             let span = metavar_span.with_ctxt(span.ctxt());
+            let _ = try_insert_metavar_span(span, metavar_span);
             TokenTree::Token(Token { kind: kind.clone(), span }, *spacing)
         }
         TokenTree::Delimited(dspan, dspacing, delimiter, tts) => {
-            let open = metavar_span.shrink_to_lo().with_ctxt(dspan.open.ctxt());
-            let close = metavar_span.shrink_to_hi().with_ctxt(dspan.close.ctxt());
+            let open = metavar_span.with_ctxt(dspan.open.ctxt());
+            let close = metavar_span.with_ctxt(dspan.close.ctxt());
+            let _ = try_insert_metavar_span(open, metavar_span);
+            let _ = try_insert_metavar_span(close, metavar_span);
             let dspan = DelimSpan::from_pair(open, close);
             TokenTree::Delimited(dspan, *dspacing, *delimiter, tts.clone())
         }

compiler/rustc_expand/src/proc_macro_server.rs

@@ -242,14 +242,6 @@ impl FromInternal<(TokenStream, &mut Rustc<'_, '_>)> for Vec<TokenTree<TokenStre
                     }));
                 }
 
-                Interpolated(ref nt) if let NtIdent(ident, is_raw) = &nt.0 => {
-                    trees.push(TokenTree::Ident(Ident {
-                        sym: ident.name,
-                        is_raw: *is_raw,
-                        span: ident.span,
-                    }))
-                }
-
                 Interpolated(nt) => {
                     let stream = TokenStream::from_nonterminal_ast(&nt.0);
                     // A hack used to pass AST fragments to attribute and derive

compiler/rustc_parse/src/parser/attr_wrapper.rs

@@ -226,8 +226,7 @@ impl<'a> Parser<'a> {
         let (mut ret, trailing) = ret?;
 
         // When we're not in `capture-cfg` mode, then bail out early if:
-        // 1. Our target doesn't support tokens at all (e.g we're parsing an `NtIdent`)
-        //    so there's nothing for us to do.
+        // 1. Our target doesn't support tokens at all, so there's nothing for us to do.
         // 2. Our target already has tokens set (e.g. we've parsed something
         // like `#[my_attr] $item`. The actual parsing code takes care of prepending
         // any attributes to the nonterminal, so we don't need to modify the

compiler/rustc_parse/src/parser/nonterminal.rs

@@ -3,7 +3,7 @@ use rustc_ast::token::{self, Delimiter, Nonterminal::*, NonterminalKind, Token};
 use rustc_ast::HasTokens;
 use rustc_ast_pretty::pprust;
 use rustc_errors::PResult;
-use rustc_span::symbol::{kw, Ident};
+use rustc_span::symbol::kw;
 
 use crate::errors::UnexpectedNonterminal;
 use crate::parser::pat::{CommaRecoveryMode, RecoverColon, RecoverComma};
@@ -24,7 +24,6 @@ impl<'a> Parser<'a> {
                 | NtPat(_)
                 | NtExpr(_)
                 | NtTy(_)
-                | NtIdent(..)
                 | NtLiteral(_) // `true`, `false`
                 | NtMeta(_)
                 | NtPath(_) => true,
@@ -45,7 +44,7 @@ impl<'a> Parser<'a> {
                 && !token.is_keyword(kw::Const)
             }
             NonterminalKind::Ty => token.can_begin_type(),
-            NonterminalKind::Ident => get_macro_ident(token).is_some(),
+            NonterminalKind::Ident => is_macro_ident(token),
             NonterminalKind::Literal => token.can_begin_literal_maybe_minus(),
             NonterminalKind::Vis => match token.kind {
                 // The follow-set of :vis + "priv" keyword + interpolated
@@ -56,8 +55,7 @@ impl<'a> Parser<'a> {
                 token::OpenDelim(Delimiter::Brace) => true,
                 token::Interpolated(nt) => match &nt.0 {
                     NtBlock(_) | NtLifetime(_) | NtStmt(_) | NtExpr(_) | NtLiteral(_) => true,
-                    NtItem(_) | NtPat(_) | NtTy(_) | NtIdent(..) | NtMeta(_) | NtPath(_)
-                    | NtVis(_) => false,
+                    NtItem(_) | NtPat(_) | NtTy(_) | NtMeta(_) | NtPath(_) | NtVis(_) => false,
                 },
                 _ => false,
             },
@@ -108,8 +106,17 @@ impl<'a> Parser<'a> {
         // in advance whether or not a proc-macro will be (transitively) invoked,
         // we always capture tokens for any `Nonterminal` which needs them.
         let mut nt = match kind {
-            // Note that TT is treated differently to all the others.
+            // Note that `tt` and `ident` are treated differently to all the others.
             NonterminalKind::TT => return Ok(ParseNtResult::Tt(self.parse_token_tree())),
+            NonterminalKind::Ident if is_macro_ident(&self.token) => {
+                return Ok(ParseNtResult::Tt(self.parse_token_tree()));
+            }
+            NonterminalKind::Ident => {
+                return Err(self.dcx().create_err(UnexpectedNonterminal::Ident {
+                    span: self.token.span,
+                    token: self.token.clone(),
+                }));
+            }
             NonterminalKind::Item => match self.parse_item(ForceCollect::Yes)? {
                 Some(item) => NtItem(item),
                 None => {
@@ -153,18 +160,6 @@ impl<'a> Parser<'a> {
             NonterminalKind::Ty => {
                 NtTy(self.collect_tokens_no_attrs(|this| this.parse_ty_no_question_mark_recover())?)
             }
-
-            // this could be handled like a token, since it is one
-            NonterminalKind::Ident if let Some((ident, is_raw)) = get_macro_ident(&self.token) => {
-                self.bump();
-                NtIdent(ident, is_raw)
-            }
-            NonterminalKind::Ident => {
-                return Err(self.dcx().create_err(UnexpectedNonterminal::Ident {
-                    span: self.token.span,
-                    token: self.token.clone(),
-                }));
-            }
             NonterminalKind::Path => {
                 NtPath(P(self.collect_tokens_no_attrs(|this| this.parse_path(PathStyle::Type))?))
             }
@@ -201,6 +196,6 @@ impl<'a> Parser<'a> {
 
 /// The token is an identifier, but not `_`.
 /// We prohibit passing `_` to macros expecting `ident` for now.
-fn get_macro_ident(token: &Token) -> Option<(Ident, bool)> {
-    token.ident().filter(|(ident, _)| ident.name != kw::Underscore)
+fn is_macro_ident(token: &Token) -> bool {
+    token.ident().map_or(false, |(ident, _)| ident.name != kw::Underscore)
 }