Skip to content

Commit 3a7215b

Browse files
committed
Auto merge of rust-lang#13732 - rami3l:fix/gen-partial-eq, r=jonas-schievink
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.
2 parents 16c70fe + 57fb18e commit 3a7215b

File tree

2 files changed

+70
-4
lines changed

2 files changed

+70
-4
lines changed

crates/ide-assists/src/handlers/replace_derive_with_manual_impl.rs

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,34 @@ impl PartialEq for Foo {
907907
}
908908

909909
#[test]
910-
fn add_custom_impl_partial_eq_tuple_enum() {
910+
fn add_custom_impl_partial_eq_single_variant_tuple_enum() {
911+
check_assist(
912+
replace_derive_with_manual_impl,
913+
r#"
914+
//- minicore: eq, derive
915+
#[derive(Partial$0Eq)]
916+
enum Foo {
917+
Bar(String),
918+
}
919+
"#,
920+
r#"
921+
enum Foo {
922+
Bar(String),
923+
}
924+
925+
impl PartialEq for Foo {
926+
$0fn eq(&self, other: &Self) -> bool {
927+
match (self, other) {
928+
(Self::Bar(l0), Self::Bar(r0)) => l0 == r0,
929+
}
930+
}
931+
}
932+
"#,
933+
)
934+
}
935+
936+
#[test]
937+
fn add_custom_impl_partial_eq_partial_tuple_enum() {
911938
check_assist(
912939
replace_derive_with_manual_impl,
913940
r#"
@@ -936,6 +963,37 @@ impl PartialEq for Foo {
936963
)
937964
}
938965

966+
#[test]
967+
fn add_custom_impl_partial_eq_tuple_enum() {
968+
check_assist(
969+
replace_derive_with_manual_impl,
970+
r#"
971+
//- minicore: eq, derive
972+
#[derive(Partial$0Eq)]
973+
enum Foo {
974+
Bar(String),
975+
Baz(i32),
976+
}
977+
"#,
978+
r#"
979+
enum Foo {
980+
Bar(String),
981+
Baz(i32),
982+
}
983+
984+
impl PartialEq for Foo {
985+
$0fn eq(&self, other: &Self) -> bool {
986+
match (self, other) {
987+
(Self::Bar(l0), Self::Bar(r0)) => l0 == r0,
988+
(Self::Baz(l0), Self::Baz(r0)) => l0 == r0,
989+
_ => false,
990+
}
991+
}
992+
}
993+
"#,
994+
)
995+
}
996+
939997
#[test]
940998
fn add_custom_impl_partial_eq_record_enum() {
941999
check_assist(

crates/ide-assists/src/utils/gen_trait_fn_body.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -516,10 +516,18 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> {
516516

517517
let expr = match arms.len() {
518518
0 => eq_check,
519-
_ => {
520-
if n_cases > arms.len() {
519+
arms_len => {
520+
// Generate the fallback arm when this enum has >1 variants.
521+
// The fallback arm will be `_ => false,` if we've already gone through every case where the variants of self and other match,
522+
// and `_ => std::mem::discriminant(self) == std::mem::discriminant(other),` otherwise.
523+
if n_cases > 1 {
521524
let lhs = make::wildcard_pat().into();
522-
arms.push(make::match_arm(Some(lhs), None, eq_check));
525+
let rhs = if arms_len == n_cases {
526+
make::expr_literal("false").into()
527+
} else {
528+
eq_check
529+
};
530+
arms.push(make::match_arm(Some(lhs), None, rhs));
523531
}
524532

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

0 commit comments

Comments
 (0)