Skip to content

Commit

Permalink
Rollup merge of #107488 - nnethercote:fix-PartialEq-syntax, r=RalfJung
Browse files Browse the repository at this point in the history
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 })
```

r? ``@RalfJung``
  • Loading branch information
matthiaskrgr authored Feb 2, 2023
2 parents f838fa8 + 75e87d1 commit 150b9d7
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
24 changes: 19 additions & 5 deletions compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,30 @@ pub fn expand_deriving_partial_eq(
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())
}
};
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_expand/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ impl<'a> ExtCtxt<'a> {
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,
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/deriving/deriving-all-codegen.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit 150b9d7

Please sign in to comment.