about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-10-18 12:56:33 +0000
committerGitHub <noreply@github.com>2021-10-18 12:56:33 +0000
commit87d5ef8c4a367d112d085827e8ad4f84b8ff809f (patch)
tree5d0504c542930adb026879a781b0b698c012ee4d
parent48c3be922e9ecc49bcdc77e21dbf6149f792a380 (diff)
parente346d32e690a24ad9de69e94b9f302d7a74e4ec5 (diff)
downloadrust-87d5ef8c4a367d112d085827e8ad4f84b8ff809f.tar.gz
rust-87d5ef8c4a367d112d085827e8ad4f84b8ff809f.zip
Merge #10578
10578: Fix partialord codegen take 2 r=lnicola a=yoshuawuyts

Fixes https://github.com/rust-analyzer/rust-analyzer/issues/10576. This reverts "generate `PartialOrd` to our previous match-based design, and in turn uses that to correctly take references for multi-value comparisons. This is a bit more verbose, but it should be more readable and easier to edit by end-users than multiple nested layers of borrows. I also manually verified every example in the Rust playground to ensure it works. Thanks!

cc/ `@WaffleLapkin` 

Co-authored-by: Yoshua Wuyts <yoshuawuyts@gmail.com>
-rw-r--r--crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs20
-rw-r--r--crates/ide_assists/src/utils/gen_trait_fn_body.rs53
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 138c694d496..042f31e5c4d 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<core::cmp::Ordering> {
-        (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::Equal) => {}
+            ord => return ord,
+        }
+        match self.bar.partial_cmp(&other.bar) {
+            Some(core::cmp::Ordering::Equal) => {}
+            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<core::cmp::Ordering> {
-        (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::Equal) => {}
+            ord => return ord,
+        }
+        match self.1.partial_cmp(&other.1) {
+            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 40c3734f9dd..908bb136beb 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<ast::Expr>, mut rhs: Vec<ast::Expr>) -> 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<ast::Stmt> {
+        let mut arms = vec![];
+
+        let variant_name =
+            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()));
+
+        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::<Option<Vec<ast::Stmt>>>()?;
+                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::<Option<Vec<ast::Stmt>>>()?;
+                make::block_expr(stmts.into_iter(), tail).indent(ast::edit::IndentLevel(1))
             }
 
             // No fields in the body means there's nothing to hash.