Implement comparison of InvisibleOrigin without PartialEq

Author: Kobzol

compiler/rustc_ast/src/attr/mod.rs

@@ -269,7 +269,9 @@ impl AttrItem {
 
     pub fn meta_item_list(&self) -> Option<ThinVec<MetaItemInner>> {
         match &self.args {
-            AttrArgs::Delimited(args) if args.delim == Delimiter::Parenthesis => {
+            AttrArgs::Delimited(args)
+                if args.delim.eq_special_invisible_origin(&Delimiter::Parenthesis) =>
+            {
                 MetaItemKind::list_from_tokens(args.tokens.clone())
             }
             AttrArgs::Delimited(_) | AttrArgs::Eq { .. } | AttrArgs::Empty => None,

compiler/rustc_ast/src/token.rs

@@ -22,8 +22,7 @@ pub enum CommentKind {
     Block,
 }
 
-// This type must not implement `Hash` due to the unusual `PartialEq` impl below.
-#[derive(Copy, Clone, Debug, Encodable, Decodable, HashStable_Generic)]
+#[derive(Copy, Clone, PartialEq, Debug, Encodable, Decodable, HashStable_Generic)]
 pub enum InvisibleOrigin {
     // From the expansion of a metavariable in a declarative macro.
     MetaVar(MetaVarKind),
@@ -45,20 +44,6 @@ impl InvisibleOrigin {
     }
 }
 
-impl PartialEq for InvisibleOrigin {
-    #[inline]
-    fn eq(&self, _other: &InvisibleOrigin) -> bool {
-        // When we had AST-based nonterminals we couldn't compare them, and the
-        // old `Nonterminal` type had an `eq` that always returned false,
-        // resulting in this restriction:
-        // https://doc.rust-lang.org/nightly/reference/macros-by-example.html#forwarding-a-matched-fragment
-        // This `eq` emulates that behaviour. We could consider lifting this
-        // restriction now but there are still cases involving invisible
-        // delimiters that make it harder than it first appears.
-        false
-    }
-}
-
 /// Annoyingly similar to `NonterminalKind`, but the slight differences are important.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Encodable, Decodable, Hash, HashStable_Generic)]
 pub enum MetaVarKind {
@@ -114,7 +99,7 @@ impl fmt::Display for MetaVarKind {
 /// Describes how a sequence of token trees is delimited.
 /// Cannot use `proc_macro::Delimiter` directly because this
 /// structure should implement some additional traits.
-#[derive(Copy, Clone, Debug, PartialEq, Encodable, Decodable, HashStable_Generic)]
+#[derive(Copy, Clone, Debug, Encodable, Decodable, HashStable_Generic)]
 pub enum Delimiter {
     /// `( ... )`
     Parenthesis,
@@ -130,6 +115,20 @@ pub enum Delimiter {
     Invisible(InvisibleOrigin),
 }
 
+impl PartialEq for Delimiter {
+    fn eq(&self, other: &Self) -> bool {
+        match (self, other) {
+            (Delimiter::Parenthesis, Delimiter::Parenthesis) => true,
+            (Delimiter::Brace, Delimiter::Brace) => true,
+            (Delimiter::Bracket, Delimiter::Bracket) => true,
+            (Delimiter::Invisible(_), _) | (_, Delimiter::Invisible(_)) => {
+                panic!("Comparing an invisible delimiter using PartialEq");
+            }
+            _ => false,
+        }
+    }
+}
+
 impl Delimiter {
     // Should the parser skip these delimiters? Only happens for certain kinds
     // of invisible delimiters. Ideally this function will eventually disappear
@@ -142,7 +141,8 @@ impl Delimiter {
         }
     }
 
-    // This exists because `InvisibleOrigin`s should be compared. It is only used for assertions.
+    // This exists because `InvisibleOrigin`s should not be compared. It is only used for
+    // assertions.
     pub fn eq_ignoring_invisible_origin(&self, other: &Delimiter) -> bool {
         match (self, other) {
             (Delimiter::Parenthesis, Delimiter::Parenthesis) => true,
@@ -153,6 +153,18 @@ impl Delimiter {
         }
     }
 
+    /// Compare two delimiters while always considering invisible delimiters to NOT be equal
+    /// to anything else.
+    pub fn eq_special_invisible_origin(&self, other: &Delimiter) -> bool {
+        match (self, other) {
+            (Delimiter::Parenthesis, Delimiter::Parenthesis) => true,
+            (Delimiter::Brace, Delimiter::Brace) => true,
+            (Delimiter::Bracket, Delimiter::Bracket) => true,
+            (Delimiter::Invisible(_), _) | (_, Delimiter::Invisible(_)) => false,
+            _ => false,
+        }
+    }
+
     pub fn as_open_token_kind(&self) -> TokenKind {
         match *self {
             Delimiter::Parenthesis => OpenParen,

compiler/rustc_ast/src/tokenstream.rs

@@ -48,7 +48,7 @@ impl TokenTree {
         match (self, other) {
             (TokenTree::Token(token, _), TokenTree::Token(token2, _)) => token.kind == token2.kind,
             (TokenTree::Delimited(.., delim, tts), TokenTree::Delimited(.., delim2, tts2)) => {
-                delim == delim2
+                delim.eq_special_invisible_origin(delim2)
                     && tts.len() == tts2.len()
                     && tts.iter().zip(tts2.iter()).all(|(a, b)| a.eq_unspanned(b))
             }

compiler/rustc_ast/src/util/classify.rs

@@ -77,7 +77,7 @@ pub fn expr_is_complete(e: &ast::Expr) -> bool {
 /// ```
 pub fn expr_requires_semi_to_be_stmt(e: &ast::Expr) -> bool {
     match &e.kind {
-        MacCall(mac_call) => mac_call.args.delim != Delimiter::Brace,
+        MacCall(mac_call) => !mac_call.args.delim.eq_special_invisible_origin(&Delimiter::Brace),
         _ => !expr_is_complete(e),
     }
 }
@@ -209,7 +209,8 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<TrailingBrace<'_>> {
             }
 
             MacCall(mac) => {
-                break (mac.args.delim == Delimiter::Brace).then_some(TrailingBrace::MacCall(mac));
+                break (mac.args.delim.eq_special_invisible_origin(&Delimiter::Brace))
+                    .then_some(TrailingBrace::MacCall(mac));
             }
 
             InlineAsm(_) | OffsetOf(_, _) | IncludedBytes(_) | FormatArgs(_) => {
@@ -252,7 +253,8 @@ fn type_trailing_braced_mac_call(mut ty: &ast::Ty) -> Option<&ast::MacCall> {
     loop {
         match &ty.kind {
             ast::TyKind::MacCall(mac) => {
-                break (mac.args.delim == Delimiter::Brace).then_some(mac);
+                break (mac.args.delim.eq_special_invisible_origin(&Delimiter::Brace))
+                    .then_some(mac);
             }
 
             ast::TyKind::Ptr(mut_ty)

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -795,7 +795,8 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
         convert_dollar_crate: bool,
         span: Span,
     ) {
-        let cb = (delim == Delimiter::Brace).then(|| self.cbox(INDENT_UNIT));
+        let cb =
+            delim.eq_special_invisible_origin(&Delimiter::Brace).then(|| self.cbox(INDENT_UNIT));
         match header {
             Some(MacHeader::Path(path)) => self.print_path(path, false, 0),
             Some(MacHeader::Keyword(kw)) => self.word(kw),

compiler/rustc_expand/src/mbe/macro_parser.rs

@@ -77,7 +77,7 @@ use std::rc::Rc;
 
 pub(crate) use NamedMatch::*;
 pub(crate) use ParseResult::*;
-use rustc_ast::token::{self, DocComment, NonterminalKind, Token};
+use rustc_ast::token::{self, DocComment, NonterminalKind, Token, TokenKind};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::ErrorGuaranteed;
 use rustc_lint_defs::pluralize;
@@ -397,7 +397,29 @@ fn token_name_eq(t1: &Token, t2: &Token) -> bool {
     {
         ident1.name == ident2.name && is_raw1 == is_raw2
     } else {
-        t1.kind == t2.kind
+        // Note: we SHOULD NOT use `t1.kind == t2.kind` here, and we should instead compare the
+        // tokens using the special comparison function.
+        compare_token_kind(t1.kind, t2.kind)
+    }
+}
+
+/// Compares two token kinds while making sure that variants containing `InvisibleOrigin` will
+/// never compare equal to one another.
+///
+/// When we had AST-based nonterminals we couldn't compare them, and the
+/// old `Nonterminal` type had an `eq` that always returned false,
+/// resulting in this restriction:
+/// https://doc.rust-lang.org/nightly/reference/macros-by-example.html#forwarding-a-matched-fragment
+/// This comparison logic emulates that behaviour. We could consider lifting this
+/// restriction now but there are still cases involving invisible
+/// delimiters that make it harder than it first appears.
+fn compare_token_kind(t1: TokenKind, t2: TokenKind) -> bool {
+    match (t1, t2) {
+        (TokenKind::OpenInvisible(_), _)
+        | (TokenKind::CloseInvisible(_), _)
+        | (_, TokenKind::CloseInvisible(_))
+        | (_, TokenKind::OpenInvisible(_)) => false,
+        (a, b) => a == b,
     }
 }
 

compiler/rustc_expand/src/mbe/transcribe.rs

@@ -223,7 +223,7 @@ pub(super) fn transcribe<'a>(
                 FrameKind::Delimited { delim, span, mut spacing, .. } => {
                     // Hack to force-insert a space after `]` in certain case.
                     // See discussion of the `hex-literal` crate in #114571.
-                    if delim == Delimiter::Bracket {
+                    if delim.eq_special_invisible_origin(&Delimiter::Bracket) {
                         spacing.close = Spacing::Alone;
                     }
                     if tscx.result_stack.is_empty() {