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>
This commit is contained in:
bors[bot] 2021-10-18 12:56:33 +00:00 committed by GitHub
commit 87d5ef8c4a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 54 additions and 19 deletions

View file

@ -722,7 +722,15 @@ struct Foo {
impl PartialOrd for Foo { impl PartialOrd for Foo {
$0fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> { $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 { impl PartialOrd for Foo {
$0fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> { $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)
} }
} }
"#, "#,

View file

@ -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_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 { fn gen_partial_eq_match(match_target: ast::Expr) -> Option<ast::Stmt> {
let (lhs, rhs) = match (lhs.len(), rhs.len()) { let mut arms = vec![];
(1, 1) => (lhs.pop().unwrap(), rhs.pop().unwrap()),
_ => (make::expr_tuple(lhs.into_iter()), make::expr_tuple(rhs.into_iter())), 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 rhs = make::expr_ref(rhs, false);
let method = make::name_ref("partial_cmp"); let method = make::name_ref("partial_cmp");
make::expr_method_call(lhs, method, make::arg_list(Some(rhs))) 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::Enum(_) => return None,
ast::Adt::Struct(strukt) => match strukt.field_list() { ast::Adt::Struct(strukt) => match strukt.field_list() {
Some(ast::FieldList::RecordFieldList(field_list)) => { Some(ast::FieldList::RecordFieldList(field_list)) => {
let mut l_fields = vec![]; let mut exprs = vec![];
let mut r_fields = vec![];
for field in field_list.fields() { for field in field_list.fields() {
let lhs = make::expr_path(make::ext::ident_path("self")); let lhs = make::expr_path(make::ext::ident_path("self"));
let lhs = make::expr_field(lhs, &field.name()?.to_string()); let lhs = make::expr_field(lhs, &field.name()?.to_string());
let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_path(make::ext::ident_path("other"));
let rhs = make::expr_field(rhs, &field.name()?.to_string()); let rhs = make::expr_field(rhs, &field.name()?.to_string());
l_fields.push(lhs); let ord = gen_partial_cmp_call(lhs, rhs);
r_fields.push(rhs); exprs.push(ord);
} }
let expr = gen_partial_cmp_call(l_fields, r_fields); let tail = exprs.pop();
make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) 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)) => { Some(ast::FieldList::TupleFieldList(field_list)) => {
let mut l_fields = vec![]; let mut exprs = vec![];
let mut r_fields = vec![];
for (i, _) in field_list.fields().enumerate() { for (i, _) in field_list.fields().enumerate() {
let idx = format!("{}", i); let idx = format!("{}", i);
let lhs = make::expr_path(make::ext::ident_path("self")); let lhs = make::expr_path(make::ext::ident_path("self"));
let lhs = make::expr_field(lhs, &idx); let lhs = make::expr_field(lhs, &idx);
let rhs = make::expr_path(make::ext::ident_path("other")); let rhs = make::expr_path(make::ext::ident_path("other"));
let rhs = make::expr_field(rhs, &idx); let rhs = make::expr_field(rhs, &idx);
l_fields.push(lhs); let ord = gen_partial_cmp_call(lhs, rhs);
r_fields.push(rhs); exprs.push(ord);
} }
let expr = gen_partial_cmp_call(l_fields, r_fields); let tail = exprs.pop();
make::block_expr(None, Some(expr)).indent(ast::edit::IndentLevel(1)) 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. // No fields in the body means there's nothing to hash.