Skip to content

Commit 0ee79f2

Browse files
nnethercotennethercote
committed
Avoid the unnecessary innermost match in partial_cmp/cmp.
We currently do a match on the comparison of every field in a struct or enum variant. But the last field has a degenerate match like this: ``` match ::core::cmp::Ord::cmp(&self.y, &other.y) { ::core::cmp::Ordering::Equal => ::core::cmp::Ordering::Equal, cmp => cmp, }, ``` This commit changes it to this: ``` ::core::cmp::Ord::cmp(&self.y, &other.y), ``` This is fairly straightforward thanks to the existing `cs_fold1` function. The commit also removes the `cs_fold` function which is no longer used. (Note: there is some repetition now in `cs_cmp` and `cs_partial_cmp`. I will remove that in a follow-up PR.)
1 parent 2c911dc commit 0ee79f2

File tree

4 files changed

+57
-139
lines changed

4 files changed

+57
-139
lines changed

‎compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> Bl
7070
// cmp => cmp
7171
// }
7272
//
73-
let expr = cs_fold(
73+
let expr = cs_fold1(
7474
// foldr nests the if-elses correctly, leaving the first field
7575
// as the outermost one, and the last as the innermost.
7676
false,
@@ -79,15 +79,12 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> Bl
7979
// ::std::cmp::Ordering::Equal => old,
8080
// cmp => cmp
8181
// }
82-
8382
let new = {
8483
let [other_f] = other_fs else {
8584
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`");
8685
};
87-
8886
let args =
8987
vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())];
90-
9188
cx.expr_call_global(span, cmp_path.clone(), args)
9289
};
9390

@@ -96,7 +93,21 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> Bl
9693

9794
cx.expr_match(span, new, vec![eq_arm, neq_arm])
9895
},
99-
cx.expr_path(equals_path.clone()),
96+
|cx, args| match args {
97+
Some((span, self_f, other_fs)) => {
98+
let new = {
99+
let [other_f] = other_fs else {
100+
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`");
101+
};
102+
let args =
103+
vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())];
104+
cx.expr_call_global(span, cmp_path.clone(), args)
105+
};
106+
107+
new
108+
}
109+
None => cx.expr_path(equals_path.clone()),
110+
},
100111
Box::new(|cx, span, tag_tuple| {
101112
if tag_tuple.len() != 2 {
102113
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`")

‎compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use crate::deriving::generic::ty::*;
22
use crate::deriving::generic::*;
33
use crate::deriving::{path_std, pathvec_std};
44

5-
use rustc_ast::MetaItem;
5+
use rustc_ast::ptr::P;
6+
use rustc_ast::{Expr, MetaItem};
67
use rustc_expand::base::{Annotatable, ExtCtxt};
78
use rustc_span::symbol::{sym, Ident};
89
use rustc_span::Span;
@@ -51,7 +52,6 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
5152
let test_id = Ident::new(sym::cmp, span);
5253
let ordering = cx.path_global(span, cx.std_path(&[sym::cmp, sym::Ordering, sym::Equal]));
5354
let ordering_expr = cx.expr_path(ordering.clone());
54-
let equals_expr = cx.expr_some(span, ordering_expr);
5555

5656
let partial_cmp_path = cx.std_path(&[sym::cmp, sym::PartialOrd, sym::partial_cmp]);
5757

@@ -68,7 +68,7 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
6868
// cmp => cmp
6969
// }
7070
//
71-
let expr = cs_fold(
71+
let expr = cs_fold1(
7272
// foldr nests the if-elses correctly, leaving the first field
7373
// as the outermost one, and the last as the innermost.
7474
false,
@@ -94,7 +94,21 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
9494

9595
cx.expr_match(span, new, vec![eq_arm, neq_arm])
9696
},
97-
equals_expr,
97+
|cx: &mut ExtCtxt<'_>, args: Option<(Span, P<Expr>, &[P<Expr>])>| match args {
98+
Some((span, self_f, other_fs)) => {
99+
let new = {
100+
let [other_f] = other_fs else {
101+
cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`");
102+
};
103+
let args =
104+
vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())];
105+
cx.expr_call_global(span, partial_cmp_path.clone(), args)
106+
};
107+
108+
new
109+
}
110+
None => cx.expr_some(span, ordering_expr.clone()),
111+
},
98112
Box::new(|cx, span, tag_tuple| {
99113
if tag_tuple.len() != 2 {
100114
cx.span_bug(span, "not exactly 2 arguments in `derive(PartialOrd)`")

‎compiler/rustc_builtin_macros/src/deriving/generic/mod.rs‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,31 +1694,6 @@ fn cs_fold_static(cx: &mut ExtCtxt<'_>, trait_span: Span) -> P<Expr> {
16941694
cx.span_bug(trait_span, "static function in `derive`")
16951695
}
16961696

1697-
/// Fold the fields. `use_foldl` controls whether this is done
1698-
/// left-to-right (`true`) or right-to-left (`false`).
1699-
pub fn cs_fold<F>(
1700-
use_foldl: bool,
1701-
f: F,
1702-
base: P<Expr>,
1703-
enum_nonmatch_f: EnumNonMatchCollapsedFunc<'_>,
1704-
cx: &mut ExtCtxt<'_>,
1705-
trait_span: Span,
1706-
substructure: &Substructure<'_>,
1707-
) -> P<Expr>
1708-
where
1709-
F: FnMut(&mut ExtCtxt<'_>, Span, P<Expr>, P<Expr>, &[P<Expr>]) -> P<Expr>,
1710-
{
1711-
match *substructure.fields {
1712-
EnumMatching(.., ref all_fields) | Struct(_, ref all_fields) => {
1713-
cs_fold_fields(use_foldl, f, base, cx, all_fields)
1714-
}
1715-
EnumNonMatchingCollapsed(..) => {
1716-
cs_fold_enumnonmatch(enum_nonmatch_f, cx, trait_span, substructure)
1717-
}
1718-
StaticEnum(..) | StaticStruct(..) => cs_fold_static(cx, trait_span),
1719-
}
1720-
}
1721-
17221697
/// Function to fold over fields, with three cases, to generate more efficient and concise code.
17231698
/// When the `substructure` has grouped fields, there are two cases:
17241699
/// Zero fields: call the base case function with `None` (like the usual base case of `cs_fold`).

‎src/test/ui/deriving/deriving-all-codegen.stdout‎

Lines changed: 23 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,7 @@ impl ::core::cmp::PartialOrd for Point {
161161
-> ::core::option::Option<::core::cmp::Ordering> {
162162
match ::core::cmp::PartialOrd::partial_cmp(&self.x, &other.x) {
163163
::core::option::Option::Some(::core::cmp::Ordering::Equal) =>
164-
match ::core::cmp::PartialOrd::partial_cmp(&self.y, &other.y)
165-
{
166-
::core::option::Option::Some(::core::cmp::Ordering::Equal)
167-
=>
168-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
169-
cmp => cmp,
170-
},
164+
::core::cmp::PartialOrd::partial_cmp(&self.y, &other.y),
171165
cmp => cmp,
172166
}
173167
}
@@ -179,11 +173,7 @@ impl ::core::cmp::Ord for Point {
179173
fn cmp(&self, other: &Point) -> ::core::cmp::Ordering {
180174
match ::core::cmp::Ord::cmp(&self.x, &other.x) {
181175
::core::cmp::Ordering::Equal =>
182-
match ::core::cmp::Ord::cmp(&self.y, &other.y) {
183-
::core::cmp::Ordering::Equal =>
184-
::core::cmp::Ordering::Equal,
185-
cmp => cmp,
186-
},
176+
::core::cmp::Ord::cmp(&self.y, &other.y),
187177
cmp => cmp,
188178
}
189179
}
@@ -323,13 +313,7 @@ impl ::core::cmp::PartialOrd for Big {
323313
&other.b7) {
324314
::core::option::Option::Some(::core::cmp::Ordering::Equal)
325315
=>
326-
match ::core::cmp::PartialOrd::partial_cmp(&self.b8,
327-
&other.b8) {
328-
::core::option::Option::Some(::core::cmp::Ordering::Equal)
329-
=>
330-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
331-
cmp => cmp,
332-
},
316+
::core::cmp::PartialOrd::partial_cmp(&self.b8, &other.b8),
333317
cmp => cmp,
334318
},
335319
cmp => cmp,
@@ -365,11 +349,7 @@ impl ::core::cmp::Ord for Big {
365349
::core::cmp::Ordering::Equal =>
366350
match ::core::cmp::Ord::cmp(&self.b7, &other.b7) {
367351
::core::cmp::Ordering::Equal =>
368-
match ::core::cmp::Ord::cmp(&self.b8, &other.b8) {
369-
::core::cmp::Ordering::Equal =>
370-
::core::cmp::Ordering::Equal,
371-
cmp => cmp,
372-
},
352+
::core::cmp::Ord::cmp(&self.b8, &other.b8),
373353
cmp => cmp,
374354
},
375355
cmp => cmp,
@@ -461,11 +441,7 @@ impl ::core::cmp::PartialOrd for Packed {
461441
-> ::core::option::Option<::core::cmp::Ordering> {
462442
let Self(__self_0_0) = *self;
463443
let Self(__self_1_0) = *other;
464-
match ::core::cmp::PartialOrd::partial_cmp(&__self_0_0, &__self_1_0) {
465-
::core::option::Option::Some(::core::cmp::Ordering::Equal) =>
466-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
467-
cmp => cmp,
468-
}
444+
::core::cmp::PartialOrd::partial_cmp(&__self_0_0, &__self_1_0)
469445
}
470446
}
471447
#[automatically_derived]
@@ -475,10 +451,7 @@ impl ::core::cmp::Ord for Packed {
475451
fn cmp(&self, other: &Packed) -> ::core::cmp::Ordering {
476452
let Self(__self_0_0) = *self;
477453
let Self(__self_1_0) = *other;
478-
match ::core::cmp::Ord::cmp(&__self_0_0, &__self_1_0) {
479-
::core::cmp::Ordering::Equal => ::core::cmp::Ordering::Equal,
480-
cmp => cmp,
481-
}
454+
::core::cmp::Ord::cmp(&__self_0_0, &__self_1_0)
482455
}
483456
}
484457

@@ -621,13 +594,7 @@ impl ::core::cmp::PartialOrd for Enum1 {
621594
match (&*self, &*other) {
622595
(&Enum1::Single { x: ref __self_0 }, &Enum1::Single {
623596
x: ref __arg_1_0 }) =>
624-
match ::core::cmp::PartialOrd::partial_cmp(&*__self_0,
625-
&*__arg_1_0) {
626-
::core::option::Option::Some(::core::cmp::Ordering::Equal)
627-
=>
628-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
629-
cmp => cmp,
630-
},
597+
::core::cmp::PartialOrd::partial_cmp(&*__self_0, &*__arg_1_0),
631598
}
632599
}
633600
}
@@ -639,11 +606,7 @@ impl ::core::cmp::Ord for Enum1 {
639606
match (&*self, &*other) {
640607
(&Enum1::Single { x: ref __self_0 }, &Enum1::Single {
641608
x: ref __arg_1_0 }) =>
642-
match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) {
643-
::core::cmp::Ordering::Equal =>
644-
::core::cmp::Ordering::Equal,
645-
cmp => cmp,
646-
},
609+
::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0),
647610
}
648611
}
649612
}
@@ -873,26 +836,16 @@ impl ::core::cmp::PartialOrd for Mixed {
873836
if __self_vi == __arg_1_vi {
874837
match (&*self, &*other) {
875838
(&Mixed::R(ref __self_0), &Mixed::R(ref __arg_1_0)) =>
876-
match ::core::cmp::PartialOrd::partial_cmp(&*__self_0,
877-
&*__arg_1_0) {
878-
::core::option::Option::Some(::core::cmp::Ordering::Equal)
879-
=>
880-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
881-
cmp => cmp,
882-
},
839+
::core::cmp::PartialOrd::partial_cmp(&*__self_0,
840+
&*__arg_1_0),
883841
(&Mixed::S { d1: ref __self_0, d2: ref __self_1 },
884842
&Mixed::S { d1: ref __arg_1_0, d2: ref __arg_1_1 }) =>
885843
match ::core::cmp::PartialOrd::partial_cmp(&*__self_0,
886844
&*__arg_1_0) {
887845
::core::option::Option::Some(::core::cmp::Ordering::Equal)
888846
=>
889-
match ::core::cmp::PartialOrd::partial_cmp(&*__self_1,
890-
&*__arg_1_1) {
891-
::core::option::Option::Some(::core::cmp::Ordering::Equal)
892-
=>
893-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
894-
cmp => cmp,
895-
},
847+
::core::cmp::PartialOrd::partial_cmp(&*__self_1,
848+
&*__arg_1_1),
896849
cmp => cmp,
897850
},
898851
_ =>
@@ -913,20 +866,12 @@ impl ::core::cmp::Ord for Mixed {
913866
if __self_vi == __arg_1_vi {
914867
match (&*self, &*other) {
915868
(&Mixed::R(ref __self_0), &Mixed::R(ref __arg_1_0)) =>
916-
match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) {
917-
::core::cmp::Ordering::Equal =>
918-
::core::cmp::Ordering::Equal,
919-
cmp => cmp,
920-
},
869+
::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0),
921870
(&Mixed::S { d1: ref __self_0, d2: ref __self_1 },
922871
&Mixed::S { d1: ref __arg_1_0, d2: ref __arg_1_1 }) =>
923872
match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) {
924873
::core::cmp::Ordering::Equal =>
925-
match ::core::cmp::Ord::cmp(&*__self_1, &*__arg_1_1) {
926-
::core::cmp::Ordering::Equal =>
927-
::core::cmp::Ordering::Equal,
928-
cmp => cmp,
929-
},
874+
::core::cmp::Ord::cmp(&*__self_1, &*__arg_1_1),
930875
cmp => cmp,
931876
},
932877
_ => ::core::cmp::Ordering::Equal,
@@ -1054,29 +999,14 @@ impl ::core::cmp::PartialOrd for Fielded {
1054999
if __self_vi == __arg_1_vi {
10551000
match (&*self, &*other) {
10561001
(&Fielded::X(ref __self_0), &Fielded::X(ref __arg_1_0)) =>
1057-
match ::core::cmp::PartialOrd::partial_cmp(&*__self_0,
1058-
&*__arg_1_0) {
1059-
::core::option::Option::Some(::core::cmp::Ordering::Equal)
1060-
=>
1061-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
1062-
cmp => cmp,
1063-
},
1002+
::core::cmp::PartialOrd::partial_cmp(&*__self_0,
1003+
&*__arg_1_0),
10641004
(&Fielded::Y(ref __self_0), &Fielded::Y(ref __arg_1_0)) =>
1065-
match ::core::cmp::PartialOrd::partial_cmp(&*__self_0,
1066-
&*__arg_1_0) {
1067-
::core::option::Option::Some(::core::cmp::Ordering::Equal)
1068-
=>
1069-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
1070-
cmp => cmp,
1071-
},
1005+
::core::cmp::PartialOrd::partial_cmp(&*__self_0,
1006+
&*__arg_1_0),
10721007
(&Fielded::Z(ref __self_0), &Fielded::Z(ref __arg_1_0)) =>
1073-
match ::core::cmp::PartialOrd::partial_cmp(&*__self_0,
1074-
&*__arg_1_0) {
1075-
::core::option::Option::Some(::core::cmp::Ordering::Equal)
1076-
=>
1077-
::core::option::Option::Some(::core::cmp::Ordering::Equal),
1078-
cmp => cmp,
1079-
},
1008+
::core::cmp::PartialOrd::partial_cmp(&*__self_0,
1009+
&*__arg_1_0),
10801010
_ => unsafe { ::core::intrinsics::unreachable() }
10811011
}
10821012
} else {
@@ -1094,23 +1024,11 @@ impl ::core::cmp::Ord for Fielded {
10941024
if __self_vi == __arg_1_vi {
10951025
match (&*self, &*other) {
10961026
(&Fielded::X(ref __self_0), &Fielded::X(ref __arg_1_0)) =>
1097-
match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) {
1098-
::core::cmp::Ordering::Equal =>
1099-
::core::cmp::Ordering::Equal,
1100-
cmp => cmp,
1101-
},
1027+
::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0),
11021028
(&Fielded::Y(ref __self_0), &Fielded::Y(ref __arg_1_0)) =>
1103-
match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) {
1104-
::core::cmp::Ordering::Equal =>
1105-
::core::cmp::Ordering::Equal,
1106-
cmp => cmp,
1107-
},
1029+
::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0),
11081030
(&Fielded::Z(ref __self_0), &Fielded::Z(ref __arg_1_0)) =>
1109-
match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) {
1110-
::core::cmp::Ordering::Equal =>
1111-
::core::cmp::Ordering::Equal,
1112-
cmp => cmp,
1113-
},
1031+
::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0),
11141032
_ => unsafe { ::core::intrinsics::unreachable() }
11151033
}
11161034
} else { ::core::cmp::Ord::cmp(&__self_vi, &__arg_1_vi) }

0 commit comments

Comments
 (0)