From 75e87d1f81290052a07fe85e3809d48a46613fb1 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <n.nethercote@gmail.com>
Date: Mon, 30 Jan 2023 16:29:52 +1100
Subject: [PATCH] Fix syntax in `-Zunpretty-expanded` output for derived
 `PartialEq`.

If you do `derive(PartialEq)` on a packed struct, the output shown by
`-Zunpretty=expanded` includes expressions like this:
```
{ self.x } == { other.x }
```
This is invalid syntax. This doesn't break compilation, because the AST
nodes are constructed within the compiler. But it does mean anyone using
`-Zunpretty=expanded` output as a guide for hand-written impls could get
a nasty surprise.

This commit fixes things by instead using this form:
```
({ self.x }) == ({ other.x })
```
---
 .../src/deriving/cmp/partial_eq.rs            | 24 +++++++++++++++----
 compiler/rustc_expand/src/build.rs            |  4 ++++
 tests/ui/deriving/deriving-all-codegen.stdout |  6 ++---
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
index 88d454fbc11..bad47db0de1 100644
--- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
+++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
@@ -29,16 +29,30 @@ fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOr
                         cx.span_bug(field.span, "not exactly 2 arguments in `derive(PartialEq)`");
                     };
 
-                    // We received `&T` arguments. Convert them to `T` by
-                    // stripping `&` or adding `*`. This isn't necessary for
-                    // type checking, but it results in much better error
-                    // messages if something goes wrong.
+                    // We received arguments of type `&T`. Convert them to type `T` by stripping
+                    // any leading `&` or adding `*`. This isn't necessary for type checking, but
+                    // it results in better error messages if something goes wrong.
+                    //
+                    // Note: for arguments that look like `&{ x }`, which occur with packed
+                    // structs, this would cause expressions like `{ self.x } == { other.x }`,
+                    // which isn't valid Rust syntax. This wouldn't break compilation because these
+                    // AST nodes are constructed within the compiler. But it would mean that code
+                    // printed by `-Zunpretty=expanded` (or `cargo expand`) would have invalid
+                    // syntax, which would be suboptimal. So we wrap these in parens, giving
+                    // `({ self.x }) == ({ other.x })`, which is valid syntax.
                     let convert = |expr: &P<Expr>| {
                         if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) =
                             &expr.kind
                         {
-                            inner.clone()
+                            if let ExprKind::Block(..) = &inner.kind {
+                                // `&{ x }` form: remove the `&`, add parens.
+                                cx.expr_paren(field.span, inner.clone())
+                            } else {
+                                // `&x` form: remove the `&`.
+                                inner.clone()
+                            }
                         } else {
+                            // No leading `&`: add a leading `*`.
                             cx.expr_deref(field.span, expr.clone())
                         }
                     };
diff --git a/compiler/rustc_expand/src/build.rs b/compiler/rustc_expand/src/build.rs
index 9b16e79d49a..6cd56852f9d 100644
--- a/compiler/rustc_expand/src/build.rs
+++ b/compiler/rustc_expand/src/build.rs
@@ -272,6 +272,10 @@ pub fn expr_addr_of(&self, sp: Span, e: P<ast::Expr>) -> P<ast::Expr> {
         self.expr(sp, ast::ExprKind::AddrOf(ast::BorrowKind::Ref, ast::Mutability::Not, e))
     }
 
+    pub fn expr_paren(&self, sp: Span, e: P<ast::Expr>) -> P<ast::Expr> {
+        self.expr(sp, ast::ExprKind::Paren(e))
+    }
+
     pub fn expr_call(
         &self,
         span: Span,
diff --git a/tests/ui/deriving/deriving-all-codegen.stdout b/tests/ui/deriving/deriving-all-codegen.stdout
index b4874cef134..8e238a509d2 100644
--- a/tests/ui/deriving/deriving-all-codegen.stdout
+++ b/tests/ui/deriving/deriving-all-codegen.stdout
@@ -209,7 +209,7 @@ impl ::core::marker::StructuralPartialEq for PackedPoint { }
 impl ::core::cmp::PartialEq for PackedPoint {
     #[inline]
     fn eq(&self, other: &PackedPoint) -> bool {
-        { self.x } == { other.x } && { self.y } == { other.y }
+        ({ self.x }) == ({ other.x }) && ({ self.y }) == ({ other.y })
     }
 }
 #[automatically_derived]
@@ -718,8 +718,8 @@ impl<T: ::core::cmp::PartialEq + ::core::marker::Copy + Trait,
     ::core::marker::Copy {
     #[inline]
     fn eq(&self, other: &PackedGeneric<T, U>) -> bool {
-        { self.0 } == { other.0 } && { self.1 } == { other.1 } &&
-            { self.2 } == { other.2 }
+        ({ self.0 }) == ({ other.0 }) && ({ self.1 }) == ({ other.1 }) &&
+            ({ self.2 }) == ({ other.2 })
     }
 }
 #[automatically_derived]
-- 
2.44.0