Skip to content

Commit 8407ec9

Browse files
committed
syntax: make #[deriving(TotalOrd)] lazy.
Previously it would call: f(sf1.cmp(&of1), f(sf2.cmp(&of2), ...)) (where s/of1 = 'self/other field 1', and f was std::cmp::lexical_ordering) This meant that every .cmp subcall got evaluated when calling a derived TotalOrd.cmp. This corrects this to use let test = sf1.cmp(&of1); if test == Equal { let test = sf2.cmp(&of2); if test == Equal { // ... } else { test } } else { test } This gives a lexical ordering by short-circuiting on the first comparison that is not Equal.
1 parent 44acdad commit 8407ec9

File tree

2 files changed

+47
-17
lines changed

2 files changed

+47
-17
lines changed

src/libstd/cmp.rs

-1
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ pub fn cmp2<A:TotalOrd,B:TotalOrd>(
153153
Return `o1` if it is not `Equal`, otherwise `o2`. Simulates the
154154
lexical ordering on a type `(int, int)`.
155155
*/
156-
// used in deriving code in libsyntax
157156
#[inline]
158157
pub fn lexical_ordering(o1: Ordering, o2: Ordering) -> Ordering {
159158
match o1 {

src/libsyntax/ext/deriving/cmp/totalord.rs

+47-16
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11+
use ast;
1112
use ast::{MetaItem, item, expr};
1213
use codemap::span;
1314
use ext::base::ExtCtxt;
@@ -40,40 +41,70 @@ pub fn expand_deriving_totalord(cx: @ExtCtxt,
4041
}
4142

4243

43-
pub fn ordering_const(cx: @ExtCtxt, span: span, cnst: Ordering) -> @expr {
44+
pub fn ordering_const(cx: @ExtCtxt, span: span, cnst: Ordering) -> ast::Path {
4445
let cnst = match cnst {
4546
Less => "Less",
4647
Equal => "Equal",
4748
Greater => "Greater"
4849
};
49-
cx.expr_path(
50-
cx.path_global(span,
51-
~[cx.ident_of("std"),
52-
cx.ident_of("cmp"),
53-
cx.ident_of(cnst)]))
50+
cx.path_global(span,
51+
~[cx.ident_of("std"),
52+
cx.ident_of("cmp"),
53+
cx.ident_of(cnst)])
5454
}
5555

5656
pub fn cs_cmp(cx: @ExtCtxt, span: span,
5757
substr: &Substructure) -> @expr {
58+
let test_id = cx.ident_of("__test");
59+
let equals_path = ordering_const(cx, span, Equal);
5860

61+
/*
62+
Builds:
63+
64+
let __test = self_field1.cmp(&other_field2);
65+
if other == ::std::cmp::Equal {
66+
let __test = self_field2.cmp(&other_field2);
67+
if __test == ::std::cmp::Equal {
68+
...
69+
} else {
70+
__test
71+
}
72+
} else {
73+
__test
74+
}
75+
76+
FIXME #6449: These `if`s could/should be `match`es.
77+
*/
5978
cs_same_method_fold(
60-
// foldr (possibly) nests the matches in lexical_ordering better
79+
// foldr nests the if-elses correctly, leaving the first field
80+
// as the outermost one, and the last as the innermost.
6181
false,
6282
|cx, span, old, new| {
63-
cx.expr_call_global(span,
64-
~[cx.ident_of("std"),
65-
cx.ident_of("cmp"),
66-
cx.ident_of("lexical_ordering")],
67-
~[old, new])
83+
// let __test = new;
84+
// if __test == ::std::cmp::Equal {
85+
// old
86+
// } else {
87+
// __test
88+
// }
89+
90+
let assign = cx.stmt_let(span, false, test_id, new);
91+
92+
let cond = cx.expr_binary(span, ast::eq,
93+
cx.expr_ident(span, test_id),
94+
cx.expr_path(equals_path.clone()));
95+
let if_ = cx.expr_if(span,
96+
cond,
97+
old, Some(cx.expr_ident(span, test_id)));
98+
cx.expr_block(cx.block(span, ~[assign], Some(if_)))
6899
},
69-
ordering_const(cx, span, Equal),
100+
cx.expr_path(equals_path.clone()),
70101
|cx, span, list, _| {
71102
match list {
72103
// an earlier nonmatching variant is Less than a
73-
// later one
104+
// later one.
74105
[(self_var, _, _),
75-
(other_var, _, _)] => ordering_const(cx, span,
76-
self_var.cmp(&other_var)),
106+
(other_var, _, _)] => cx.expr_path(ordering_const(cx, span,
107+
self_var.cmp(&other_var))),
77108
_ => cx.span_bug(span, "Not exactly 2 arguments in `deriving(TotalOrd)`")
78109
}
79110
},

0 commit comments

Comments
 (0)