Skip to content

Commit

Permalink
Auto merge of rust-lang#13732 - rami3l:fix/gen-partial-eq, r=jonas-sc…
Browse files Browse the repository at this point in the history
…hievink

fix: add fallback case in generated `PartialEq` impl

Partially fixes rust-lang#13727.

When generating `PartialEq` implementations for enums, the original code can already generate the following fallback case:

```rs
_ => std::mem::discriminant(self) == std::mem::discriminant(other),
```

However, it has been suppressed in the following example for no good reason:

```rs
enum Either<T, U> {
    Left(T),
    Right(U),
}

impl<T, U> PartialEq for Either<T, U> {
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (Self::Left(l0), Self::Left(r0)) => l0 == r0,
            (Self::Right(l0), Self::Right(r0)) => l0 == r0,
            // _ => std::mem::discriminant(self) == std::mem::discriminant(other),
            // ^ this completes the match arms!
        }
    }
}
```

This PR has removed that suppression logic.

~~Of course, the PR could have suppressed the fallback case generation for single-variant enums instead, but I believe that this case is quite rare and should be caught by `#[warn(unreachable_patterns)]` anyway.~~

After this fix, when the enum has >1 variants, the following fallback arm will be generated :

* `_ => false,` if we've already gone through every case where the variants of `self` and `other` match;
* The original one (as stated above) in other cases.

---

Note: The code example is still wrong after the fix due to incorrect trait bounds.
  • Loading branch information
bors committed Dec 12, 2022
2 parents 16c70fe + 57fb18e commit 3a7215b
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,34 @@ impl PartialEq for Foo {
}

#[test]
fn add_custom_impl_partial_eq_tuple_enum() {
fn add_custom_impl_partial_eq_single_variant_tuple_enum() {
check_assist(
replace_derive_with_manual_impl,
r#"
//- minicore: eq, derive
#[derive(Partial$0Eq)]
enum Foo {
Bar(String),
}
"#,
r#"
enum Foo {
Bar(String),
}
impl PartialEq for Foo {
$0fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Self::Bar(l0), Self::Bar(r0)) => l0 == r0,
}
}
}
"#,
)
}

#[test]
fn add_custom_impl_partial_eq_partial_tuple_enum() {
check_assist(
replace_derive_with_manual_impl,
r#"
Expand Down Expand Up @@ -936,6 +963,37 @@ impl PartialEq for Foo {
)
}

#[test]
fn add_custom_impl_partial_eq_tuple_enum() {
check_assist(
replace_derive_with_manual_impl,
r#"
//- minicore: eq, derive
#[derive(Partial$0Eq)]
enum Foo {
Bar(String),
Baz(i32),
}
"#,
r#"
enum Foo {
Bar(String),
Baz(i32),
}
impl PartialEq for Foo {
$0fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Self::Bar(l0), Self::Bar(r0)) => l0 == r0,
(Self::Baz(l0), Self::Baz(r0)) => l0 == r0,
_ => false,
}
}
}
"#,
)
}

#[test]
fn add_custom_impl_partial_eq_record_enum() {
check_assist(
Expand Down
14 changes: 11 additions & 3 deletions crates/ide-assists/src/utils/gen_trait_fn_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,10 +516,18 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {

let expr = match arms.len() {
0 => eq_check,
_ => {
if n_cases > arms.len() {
arms_len => {
// Generate the fallback arm when this enum has >1 variants.
// The fallback arm will be `_ => false,` if we've already gone through every case where the variants of self and other match,
// and `_ => std::mem::discriminant(self) == std::mem::discriminant(other),` otherwise.
if n_cases > 1 {
let lhs = make::wildcard_pat().into();
arms.push(make::match_arm(Some(lhs), None, eq_check));
let rhs = if arms_len == n_cases {
make::expr_literal("false").into()
} else {
eq_check
};
arms.push(make::match_arm(Some(lhs), None, rhs));
}

let match_target = make::expr_tuple(vec![lhs_name, rhs_name]);
Expand Down

0 comments on commit 3a7215b

Please sign in to comment.