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/expr.rs

@@ -2489,7 +2489,7 @@ impl<'a> Parser<'a> {
                 }
                 ExprKind::Block(_, None) => {
                     this.dcx().emit_err(errors::IfExpressionMissingCondition {
-                        if_span: lo.shrink_to_hi(),
+                        if_span: lo.between(cond.span).shrink_to_lo(),
                         block_span: self.sess.source_map().start_point(cond_span),
                     });
                     std::mem::replace(&mut cond, this.mk_expr_err(cond_span.shrink_to_hi()))
@@ -3735,7 +3735,7 @@ impl<'a> Parser<'a> {
     }
 
     pub(crate) fn mk_expr(&self, span: Span, kind: ExprKind) -> P<Expr> {
-        P(Expr { kind, span, attrs: AttrVec::new(), id: DUMMY_NODE_ID, tokens: None })
+        self.mk_expr_with_attrs(span, kind, AttrVec::new())
     }
 
     pub(super) fn mk_expr_err(&self, span: Span) -> P<Expr> {

compiler/rustc_parse/src/parser/item.rs

@@ -2118,7 +2118,7 @@ impl<'a> Parser<'a> {
                 Applicability::MaybeIncorrect,
             );
             err.span_suggestion(
-                span.shrink_to_hi(),
+                span.between(self.token.span).shrink_to_lo(),
                 "add a semicolon",
                 ';',
                 Applicability::MaybeIncorrect,
@@ -2632,7 +2632,7 @@ impl<'a> Parser<'a> {
 
             let is_name_required = match this.token.kind {
                 token::DotDotDot => false,
-                _ => req_name(this.token.span.edition()),
+                _ => req_name(this.token.span.with_env(this.prev_token.span).edition()),
             };
             let (pat, ty) = if is_name_required || this.is_named_param() {
                 debug!("parse_param_general parse_pat (is_name_required:{})", is_name_required);