[passes tests, mostly*] Remove Nonterminal::NtExpr.

[*] two ui tests temporarily removed, and one unit test has a couple of
failing-to-parse assertions removed.

XXX:
- Some tests have sub-optimal error messages, I want to improve them
- Added a `src` field to `Delimiter::Invisible` which indicates where it
  came from, partly replicates the Nonterminal kind within
  `Interpolated`.
- parse_meta_item_inner required significant changes, it was assuming
  that parse_unsuffixed_lit didn't consume any tokens on failure

Author: nnethercote

compiler/rustc_ast/src/ast.rs

@@ -1629,7 +1629,7 @@ impl MacDelimiter {
             Delimiter::Parenthesis => Some(MacDelimiter::Parenthesis),
             Delimiter::Bracket => Some(MacDelimiter::Bracket),
             Delimiter::Brace => Some(MacDelimiter::Brace),
-            Delimiter::Invisible => None,
+            Delimiter::Invisible(_) => None,
         }
     }
 }

compiler/rustc_ast/src/ast_traits.rs

@@ -233,7 +233,7 @@ impl HasTokens for Nonterminal {
         match self {
             Nonterminal::NtItem(item) => item.tokens(),
             Nonterminal::NtStmt(stmt) => stmt.tokens(),
-            Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => expr.tokens(),
+            Nonterminal::NtLiteral(expr) => expr.tokens(),
             Nonterminal::NtPat(pat) => pat.tokens(),
             Nonterminal::NtTy(ty) => ty.tokens(),
             Nonterminal::NtMeta(attr_item) => attr_item.tokens(),
@@ -247,7 +247,7 @@ impl HasTokens for Nonterminal {
         match self {
             Nonterminal::NtItem(item) => item.tokens_mut(),
             Nonterminal::NtStmt(stmt) => stmt.tokens_mut(),
-            Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => expr.tokens_mut(),
+            Nonterminal::NtLiteral(expr) => expr.tokens_mut(),
             Nonterminal::NtPat(pat) => pat.tokens_mut(),
             Nonterminal::NtTy(ty) => ty.tokens_mut(),
             Nonterminal::NtMeta(attr_item) => attr_item.tokens_mut(),

compiler/rustc_ast/src/attr/mod.rs

@@ -551,7 +551,7 @@ impl MetaItemKind {
         tokens: &mut impl Iterator<Item = TokenTree>,
     ) -> Option<MetaItemKind> {
         match tokens.next() {
-            Some(TokenTree::Delimited(_, Delimiter::Invisible, inner_tokens)) => {
+            Some(TokenTree::Delimited(_, Delimiter::Invisible(_), inner_tokens)) => {
                 MetaItemKind::name_value_from_tokens(&mut inner_tokens.into_trees())
             }
             Some(TokenTree::Token(token)) => {
@@ -621,7 +621,7 @@ impl NestedMetaItem {
                 tokens.next();
                 return Some(NestedMetaItem::Literal(lit));
             }
-            Some(TokenTree::Delimited(_, Delimiter::Invisible, inner_tokens)) => {
+            Some(TokenTree::Delimited(_, Delimiter::Invisible(_), inner_tokens)) => {
                 let inner_tokens = inner_tokens.clone();
                 tokens.next();
                 return NestedMetaItem::from_tokens(&mut inner_tokens.into_trees().peekable());

compiler/rustc_ast/src/mut_visit.rs

@@ -777,7 +777,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),

compiler/rustc_ast/src/token.rs

@@ -41,8 +41,7 @@ pub enum BinOpToken {
 /// structure should implement some additional traits.
 /// The `None` variant is also renamed to `Invisible` to be
 /// less confusing and better convey the semantics.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
-#[derive(Encodable, Decodable, Hash, HashStable_Generic)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Encodable, Decodable, Hash, HashStable_Generic)]
 pub enum Delimiter {
     /// `( ... )`
     Parenthesis,
@@ -56,7 +55,36 @@ pub enum Delimiter {
     /// `$var * 3` where `$var` is `1 + 2`.
     /// Invisible delimiters are not directly writable in normal Rust code except as comments.
     /// Therefore, they might not survive a roundtrip of a token stream through a string.
-    Invisible,
+    Invisible(InvisibleSource),
+}
+
+// We are in the process of migrating interpolated nonterminals to
+// invisible-delimited token sequences. This enum will grow as `Nonterminal`
+// shrinks.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Encodable, Decodable, Hash, HashStable_Generic)]
+pub enum InvisibleSource {
+    // From the expansion of a `expr` metavar in a declarative macro.
+    ExprMv,
+    // Converted from `proc_macro::Delimiter`, i.e. returned by a proc macro.
+    ProcMacro,
+    // Converted from `TokenKind::Interpolated` in `flatten_token`. Treated
+    // similarly to `ProcMacro`.
+    FlattenToken,
+}
+
+impl Delimiter {
+    // Should the parser skip these delimeters? Only happens for certain kinds
+    // of invisible delimiters. Eventually the answer should be `false` for all
+    // kinds, whereupon this function can be removed.
+    pub fn skip(&self) -> bool {
+        match self {
+            Delimiter::Invisible(src) => match src {
+                InvisibleSource::FlattenToken | InvisibleSource::ProcMacro => true,
+                InvisibleSource::ExprMv => false,
+            },
+            Delimiter::Parenthesis | Delimiter::Bracket | Delimiter::Brace => false,
+        }
+    }
 }
 
 #[derive(Clone, Copy, PartialEq, Encodable, Decodable, Debug, HashStable_Generic)]
@@ -246,7 +274,9 @@ pub enum TokenKind {
     /// - It requires special handling in a bunch of places in the parser.
     /// - It prevents `Token` from implementing `Copy`.
     /// It adds complexity and likely slows things down. Please don't add new
-    /// occurrences of this token kind!
+    /// occurrences of this token kind! See `InvisibleSource` for details on
+    /// how it will be removed, and #96764 for potential speed benefits of
+    /// making `Token` implement `Copy`.
     Interpolated(Lrc<Nonterminal>),
 
     /// A doc comment token.
@@ -377,7 +407,7 @@ impl Token {
         match self.uninterpolate().kind {
             Ident(name, is_raw)              =>
                 ident_can_begin_expr(name, self.span, is_raw), // value name or keyword
-            OpenDelim(..)                     | // tuple, array or block
+            OpenDelim(..)                     | // tuple, array, block, or macro output
             Literal(..)                       | // literal
             Not                               | // operator not
             BinOp(Minus)                      | // unary minus
@@ -392,7 +422,6 @@ impl Token {
             Lifetime(..)                      | // labeled loop
             Pound                             => true, // expression attributes
             Interpolated(ref nt) => matches!(**nt, NtLiteral(..) |
-                NtExpr(..)    |
                 NtBlock(..)   |
                 NtPath(..)),
             _ => false,
@@ -422,8 +451,8 @@ impl Token {
     /// Returns `true` if the token can appear at the start of a const param.
     pub fn can_begin_const_arg(&self) -> bool {
         match self.kind {
-            OpenDelim(Delimiter::Brace) => true,
-            Interpolated(ref nt) => matches!(**nt, NtExpr(..) | NtBlock(..) | NtLiteral(..)),
+            OpenDelim(Delimiter::Brace | Delimiter::Invisible(InvisibleSource::ExprMv)) => true,
+            Interpolated(ref nt) => matches!(**nt, NtBlock(..) | NtLiteral(..)),
             _ => self.can_begin_literal_maybe_minus(),
         }
     }
@@ -448,21 +477,25 @@ impl Token {
     /// In other words, would this token be a valid start of `parse_literal_maybe_minus`?
     ///
     /// Keep this in sync with and `Lit::from_token`, excluding unary negation.
+    // njn: ugh to that comment
     pub fn can_begin_literal_maybe_minus(&self) -> bool {
         match self.uninterpolate().kind {
             Literal(..) | BinOp(Minus) => true,
             Ident(name, false) if name.is_bool_lit() => true,
-            Interpolated(ref nt) => match &**nt {
-                NtLiteral(_) => true,
-                NtExpr(e) => match &e.kind {
-                    ast::ExprKind::Lit(_) => true,
-                    ast::ExprKind::Unary(ast::UnOp::Neg, e) => {
-                        matches!(&e.kind, ast::ExprKind::Lit(_))
-                    }
-                    _ => false,
-                },
-                _ => false,
-            },
+            OpenDelim(Delimiter::Invisible(InvisibleSource::ExprMv)) => true,
+            Interpolated(ref nt) => matches!(**nt, NtLiteral(_)),
+            _ => false,
+        }
+    }
+
+    // Can this token be a valid start of `parse_unsuffixed_literal`?
+    pub fn can_begin_unsuffixed_literal(&self) -> bool {
+        match self.uninterpolate().kind {
+            Literal(..) => true,
+            Ident(name, false) if name.is_bool_lit() => true,
+            OpenDelim(Delimiter::Invisible(InvisibleSource::ExprMv)) => true,
+            Interpolated(ref nt) => matches!(**nt, NtLiteral(_)),
+            Dot => true, // not valid, but accepted for recovery
             _ => false,
         }
     }
@@ -536,19 +569,6 @@ impl Token {
         false
     }
 
-    /// Would `maybe_whole_expr` in `parser.rs` return `Ok(..)`?
-    /// 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
-            && let NtExpr(_) | NtLiteral(_) | NtPath(_) | NtBlock(_) = **nt
-        {
-            return true;
-        }
-
-        false
-    }
-
     // Is the token an interpolated block (`$b:block`)?
     pub fn is_whole_block(&self) -> bool {
         if let Interpolated(ref nt) = self.kind && let NtBlock(..) = **nt {
@@ -690,14 +710,16 @@ impl PartialEq<TokenKind> for Token {
     }
 }
 
+// We are in the process of migrating interpolated nonterminals to
+// invisible-delimited token sequences (e.g. #96724). This enum will shrink as
+// `InvisibleSource` grows.
 #[derive(Clone, Encodable, Decodable)]
 /// For interpolation during macro expansion.
 pub enum Nonterminal {
     NtItem(P<ast::Item>),
     NtBlock(P<ast::Block>),
     NtStmt(P<ast::Stmt>),
     NtPat(P<ast::Pat>),
-    NtExpr(P<ast::Expr>),
     NtTy(P<ast::Ty>),
     NtIdent(Ident, /* is_raw */ bool),
     NtLifetime(Ident),
@@ -797,7 +819,7 @@ impl Nonterminal {
             NtBlock(block) => block.span,
             NtStmt(stmt) => stmt.span,
             NtPat(pat) => pat.span,
-            NtExpr(expr) | NtLiteral(expr) => expr.span,
+            NtLiteral(expr) => expr.span,
             NtTy(ty) => ty.span,
             NtIdent(ident, _) | NtLifetime(ident) => ident.span,
             NtMeta(attr_item) => attr_item.span(),
@@ -830,7 +852,6 @@ impl fmt::Debug for Nonterminal {
             NtBlock(..) => f.pad("NtBlock(..)"),
             NtStmt(..) => f.pad("NtStmt(..)"),
             NtPat(..) => f.pad("NtPat(..)"),
-            NtExpr(..) => f.pad("NtExpr(..)"),
             NtTy(..) => f.pad("NtTy(..)"),
             NtIdent(..) => f.pad("NtIdent(..)"),
             NtLiteral(..) => f.pad("NtLiteral(..)"),

compiler/rustc_ast/src/tokenstream.rs

@@ -15,7 +15,7 @@
 
 use crate::ast::StmtKind;
 use crate::ast_traits::{HasAttrs, HasSpan, HasTokens};
-use crate::token::{self, Delimiter, Nonterminal, Token, TokenKind};
+use crate::token::{self, Delimiter, InvisibleSource, Nonterminal, Token, TokenKind};
 use crate::AttrVec;
 
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
@@ -508,7 +508,7 @@ impl TokenStream {
             Nonterminal::NtMeta(attr) => TokenStream::from_ast(attr),
             Nonterminal::NtPath(path) => TokenStream::from_ast(path),
             Nonterminal::NtVis(vis) => TokenStream::from_ast(vis),
-            Nonterminal::NtExpr(expr) | Nonterminal::NtLiteral(expr) => TokenStream::from_ast(expr),
+            Nonterminal::NtLiteral(expr) => TokenStream::from_ast(expr),
         }
     }
 
@@ -519,7 +519,7 @@ impl TokenStream {
             }
             token::Interpolated(nt) => TokenTree::Delimited(
                 DelimSpan::from_single(token.span),
-                Delimiter::Invisible,
+                Delimiter::Invisible(InvisibleSource::FlattenToken),
                 TokenStream::from_nonterminal_ast(&nt).flattened(),
             ),
             _ => TokenTree::Token(token.clone()),

compiler/rustc_ast/src/util/literal.rs

@@ -215,19 +215,26 @@ impl Lit {
     /// Converts arbitrary token into an AST literal.
     ///
     /// Keep this in sync with `Token::can_begin_literal_or_bool` excluding unary negation.
+    // njn: need to do something here? It's hard to keep in sync with
+    // can_begin_literal_or_bool when a literal can span 3 tokens: `«`, `lit`, `»`
+    // petrochenkov: Some use sites of Lit::from_token may need an adjustment,
+    // it's better to keep this function for 1-to-1 correspondence.
     pub fn from_token(token: &Token) -> Result<Lit, LitError> {
         let lit = match token.uninterpolate().kind {
             token::Ident(name, false) if name.is_bool_lit() => {
                 token::Lit::new(token::Bool, name, None)
             }
             token::Literal(lit) => lit,
             token::Interpolated(ref nt) => {
-                if let token::NtExpr(expr) | token::NtLiteral(expr) = &**nt
-                    && let ast::ExprKind::Lit(lit) = &expr.kind
-                {
-                    return Ok(lit.clone());
-                }
-                return Err(LitError::NotLiteral);
+                return if let token::NtLiteral(expr) = &**nt {
+                    if let ast::ExprKind::Lit(lit) = &expr.kind {
+                        Ok(lit.clone())
+                    } else {
+                        unreachable!()
+                    }
+                } else {
+                    Err(LitError::NotLiteral)
+                };
             }
             _ => return Err(LitError::NotLiteral),
         };

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -6,7 +6,8 @@ use crate::pp::Breaks::{Consistent, Inconsistent};
 use crate::pp::{self, Breaks};
 
 use rustc_ast::ptr::P;
-use rustc_ast::token::{self, BinOpToken, CommentKind, Delimiter, Nonterminal, Token, TokenKind};
+use rustc_ast::token::{self, BinOpToken, CommentKind, Delimiter, InvisibleSource, Nonterminal};
+use rustc_ast::token::{Token, TokenKind};
 use rustc_ast::tokenstream::{TokenStream, TokenTree};
 use rustc_ast::util::classify;
 use rustc_ast::util::comments::{gather_comments, Comment, CommentStyle};
@@ -146,18 +147,26 @@ pub fn print_crate<'a>(
 /// and also addresses some specific regressions described in #63896 and #73345.
 fn tt_prepend_space(tt: &TokenTree, prev: &TokenTree) -> bool {
     if let TokenTree::Token(token) = prev {
-        if matches!(token.kind, token::Dot | token::Dollar) {
+        // No space after these tokens, e.g. `x.y`, `$e`, `a::b`
+        // (The carets point to `prev`)        ^     ^      ^^
+        if matches!(token.kind, token::Dot | token::Dollar | token::ModSep) {
             return false;
         }
         if let token::DocComment(comment_kind, ..) = token.kind {
             return comment_kind != CommentKind::Line;
         }
     }
     match tt {
-        TokenTree::Token(token) => !matches!(token.kind, token::Comma | token::Not | token::Dot),
+        // No space before these tokens, e.g. `foo,`, `println!`, `x.y`, `a::b`
+        // (The carets point to `token`)          ^           ^     ^      ^^
+        TokenTree::Token(token) => {
+            !matches!(token.kind, token::Comma | token::Not | token::Dot | token::ModSep)
+        }
+        // No space before parentheses if preceded by these tokens, e.g. `foo(...)`...).
         TokenTree::Delimited(_, Delimiter::Parenthesis, _) => {
             !matches!(prev, TokenTree::Token(Token { kind: token::Ident(..), .. }))
         }
+        // No space before brackets if preceded by these tokens, e.g. e.g. `#[...]`.
         TokenTree::Delimited(_, Delimiter::Bracket, _) => {
             !matches!(prev, TokenTree::Token(Token { kind: token::Pound, .. }))
         }
@@ -599,7 +608,7 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
                 self.end();
                 self.bclose(span, empty);
             }
-            Some(Delimiter::Invisible) => {
+            Some(Delimiter::Invisible(_)) => {
                 self.word("/*«*/");
                 let empty = tts.is_empty();
                 if !empty {
@@ -727,7 +736,6 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
 
     fn nonterminal_to_string(&self, nt: &Nonterminal) -> String {
         match *nt {
-            token::NtExpr(ref e) => self.expr_to_string(e),
             token::NtMeta(ref e) => self.attr_item_to_string(e),
             token::NtTy(ref e) => self.ty_to_string(e),
             token::NtPath(ref e) => self.path_to_string(e),
@@ -786,8 +794,28 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
             token::CloseDelim(Delimiter::Bracket) => "]".into(),
             token::OpenDelim(Delimiter::Brace) => "{".into(),
             token::CloseDelim(Delimiter::Brace) => "}".into(),
-            token::OpenDelim(Delimiter::Invisible) => "/*«*/".into(),
-            token::CloseDelim(Delimiter::Invisible) => "/*»*/".into(),
+            // We use comment-delimited phrases here for invisible delimiters,
+            // because these cases tend to occur in error messages. In
+            // `print_mac_common` we use `/*«*/` and `/*»*/` for compactness,
+            // because that occurs in other cases like pretty-printing.
+            token::OpenDelim(Delimiter::Invisible(src)) => match src {
+                // njn: petrochenkov: Parser uses fn token_descr(_opt) for
+                // printing expected/found tokens, so it can also be used as a
+                // customization point. (You won't need to wrap anything in
+                // comments there, for example.)
+                InvisibleSource::ExprMv => "/*start of expr expansion*/",
+                InvisibleSource::FlattenToken | InvisibleSource::ProcMacro => {
+                    "/*invisible open delimiter*/"
+                }
+            }
+            .into(),
+            token::CloseDelim(Delimiter::Invisible(src)) => match src {
+                InvisibleSource::ExprMv => "/*end of expr expansion*/",
+                InvisibleSource::FlattenToken | InvisibleSource::ProcMacro => {
+                    "/*invisible close delimiter*/"
+                }
+            }
+            .into(),
             token::Pound => "#".into(),
             token::Dollar => "$".into(),
             token::Question => "?".into(),

compiler/rustc_builtin_macros/src/source_util.rs

@@ -74,7 +74,7 @@ pub fn expand_stringify(
     // `stringify!` is nearly always called on `macro_rules` meta-variables and the intent is to
     // stringify the underlying tokens without meta-variable's own invisible delimiters, so we
     // are stripping such delimiters here (possibly multiple layers of them).
-    while let [TokenTree::Delimited(_, Delimiter::Invisible, inner_tts)] =
+    while let [TokenTree::Delimited(_, Delimiter::Invisible(_), inner_tts)] =
         &mut tts.clone().into_trees().collect::<Vec<_>>()[..]
     {
         tts = inner_tts.clone();