From 41fd82441536cd9f8bc31461cab63cb6ec586d4e Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Mon, 18 Oct 2021 14:38:53 +0200 Subject: [PATCH 1/2] Revert "Simplify generated PartialOrd code" This reverts commit 601ed3a10dacc2ba2ee0ca436c23529ae7fde292. --- .../replace_derive_with_manual_impl.rs | 20 ++++++- .../src/utils/gen_trait_fn_body.rs | 53 +++++++++++++------ 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs index 138c694d49..72f49657fb 100644 --- a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs @@ -722,7 +722,15 @@ struct Foo { impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option { - (self.bin, self.bar, self.baz).partial_cmp(&(other.bin, other.bar, other.baz)) + match self.bin.partial_cmp(&other.bin) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + match self.bar.partial_cmp(&other.bar) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + self.baz.partial_cmp(&other.baz) } } "#, @@ -743,7 +751,15 @@ struct Foo(usize, usize, usize); impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option { - (self.0, self.1, self.2).partial_cmp(&(other.0, other.1, other.2)) + match self.0.partial_cmp(&other.0) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + match self.1.partial_cmp(&other.1) { + Some(core::cmp::Ordering::Eq) => {} + ord => return ord, + } + self.2.partial_cmp(&other.2) } } "#, diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 40c3734f9d..200c9e25db 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -574,11 +574,24 @@ fn gen_partial_eq(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { } fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { - fn gen_partial_cmp_call(mut lhs: Vec, mut rhs: Vec) -> ast::Expr { - let (lhs, rhs) = match (lhs.len(), rhs.len()) { - (1, 1) => (lhs.pop().unwrap(), rhs.pop().unwrap()), - _ => (make::expr_tuple(lhs.into_iter()), make::expr_tuple(rhs.into_iter())), - }; + fn gen_partial_eq_match(match_target: ast::Expr) -> Option { + let mut arms = vec![]; + + let variant_name = + make::path_pat(make::ext::path_from_idents(["core", "cmp", "Ordering", "Eq"])?); + let lhs = make::tuple_struct_pat(make::ext::path_from_idents(["Some"])?, [variant_name]); + arms.push(make::match_arm(Some(lhs.into()), None, make::expr_empty_block())); + + arms.push(make::match_arm( + [make::ident_pat(false, false, make::name("ord")).into()], + None, + make::expr_return(Some(make::expr_path(make::ext::ident_path("ord")))), + )); + let list = make::match_arm_list(arms).indent(ast::edit::IndentLevel(1)); + Some(make::expr_stmt(make::expr_match(match_target, list)).into()) + } + + fn gen_partial_cmp_call(lhs: ast::Expr, rhs: ast::Expr) -> ast::Expr { let rhs = make::expr_ref(rhs, false); let method = make::name_ref("partial_cmp"); make::expr_method_call(lhs, method, make::arg_list(Some(rhs))) @@ -594,35 +607,41 @@ fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { ast::Adt::Enum(_) => return None, ast::Adt::Struct(strukt) => match strukt.field_list() { Some(ast::FieldList::RecordFieldList(field_list)) => { - let mut l_fields = vec![]; - let mut r_fields = vec![]; + let mut exprs = vec![]; for field in field_list.fields() { let lhs = make::expr_path(make::ext::ident_path("self")); let lhs = make::expr_field(lhs, &field.name()?.to_string()); let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_field(rhs, &field.name()?.to_string()); - l_fields.push(lhs); - r_fields.push(rhs); + let ord = gen_partial_cmp_call(lhs, rhs); + exprs.push(ord); } - let expr = gen_partial_cmp_call(l_fields, r_fields); - make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) + let tail = exprs.pop(); + let stmts = exprs + .into_iter() + .map(gen_partial_eq_match) + .collect::>>()?; + make::block_expr(stmts.into_iter(), tail).indent(ast::edit::IndentLevel(1)) } Some(ast::FieldList::TupleFieldList(field_list)) => { - let mut l_fields = vec![]; - let mut r_fields = vec![]; + let mut exprs = vec![]; for (i, _) in field_list.fields().enumerate() { let idx = format!("{}", i); let lhs = make::expr_path(make::ext::ident_path("self")); let lhs = make::expr_field(lhs, &idx); let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_field(rhs, &idx); - l_fields.push(lhs); - r_fields.push(rhs); + let ord = gen_partial_cmp_call(lhs, rhs); + exprs.push(ord); } - let expr = gen_partial_cmp_call(l_fields, r_fields); - make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) + let tail = exprs.pop(); + let stmts = exprs + .into_iter() + .map(gen_partial_eq_match) + .collect::>>()?; + make::block_expr(stmts.into_iter(), tail).indent(ast::edit::IndentLevel(1)) } // No fields in the body means there's nothing to hash. From e346d32e690a24ad9de69e94b9f302d7a74e4ec5 Mon Sep 17 00:00:00 2001 From: Yoshua Wuyts Date: Mon, 18 Oct 2021 14:45:24 +0200 Subject: [PATCH 2/2] fix Ordering::Equal path --- .../src/handlers/replace_derive_with_manual_impl.rs | 8 ++++---- crates/ide_assists/src/utils/gen_trait_fn_body.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs index 72f49657fb..042f31e5c4 100644 --- a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs @@ -723,11 +723,11 @@ struct Foo { impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option { match self.bin.partial_cmp(&other.bin) { - Some(core::cmp::Ordering::Eq) => {} + Some(core::cmp::Ordering::Equal) => {} ord => return ord, } match self.bar.partial_cmp(&other.bar) { - Some(core::cmp::Ordering::Eq) => {} + Some(core::cmp::Ordering::Equal) => {} ord => return ord, } self.baz.partial_cmp(&other.baz) @@ -752,11 +752,11 @@ struct Foo(usize, usize, usize); impl PartialOrd for Foo { $0fn partial_cmp(&self, other: &Self) -> Option { match self.0.partial_cmp(&other.0) { - Some(core::cmp::Ordering::Eq) => {} + Some(core::cmp::Ordering::Equal) => {} ord => return ord, } match self.1.partial_cmp(&other.1) { - Some(core::cmp::Ordering::Eq) => {} + Some(core::cmp::Ordering::Equal) => {} ord => return ord, } self.2.partial_cmp(&other.2) diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 200c9e25db..908bb136be 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -578,7 +578,7 @@ fn gen_partial_ord(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { let mut arms = vec![]; let variant_name = - make::path_pat(make::ext::path_from_idents(["core", "cmp", "Ordering", "Eq"])?); + make::path_pat(make::ext::path_from_idents(["core", "cmp", "Ordering", "Equal"])?); let lhs = make::tuple_struct_pat(make::ext::path_from_idents(["Some"])?, [variant_name]); arms.push(make::match_arm(Some(lhs.into()), None, make::expr_empty_block()));