diff options
| author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2021-10-18 12:56:33 +0000 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2021-10-18 12:56:33 +0000 |
| commit | 87d5ef8c4a367d112d085827e8ad4f84b8ff809f (patch) | |
| tree | 5d0504c542930adb026879a781b0b698c012ee4d | |
| parent | 48c3be922e9ecc49bcdc77e21dbf6149f792a380 (diff) | |
| parent | e346d32e690a24ad9de69e94b9f302d7a74e4ec5 (diff) | |
| download | rust-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.rs | 20 | ||||
| -rw-r--r-- | crates/ide_assists/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 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. |
