From 8407ec9fedccdd410dee2bca67651ce245ed5cd3 Mon Sep 17 00:00:00 2001
From: Huon Wilson <dbau.pp+github@gmail.com>
Date: Sun, 28 Jul 2013 01:16:30 +1000
Subject: [PATCH] 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.
---
 src/libstd/cmp.rs                          |  1 -
 src/libsyntax/ext/deriving/cmp/totalord.rs | 63 ++++++++++++++++------
 2 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/src/libstd/cmp.rs b/src/libstd/cmp.rs
index 43a632562b2..b66f89e8341 100644
--- a/src/libstd/cmp.rs
+++ b/src/libstd/cmp.rs
@@ -153,7 +153,6 @@ pub fn cmp2<A:TotalOrd,B:TotalOrd>(
 Return `o1` if it is not `Equal`, otherwise `o2`. Simulates the
 lexical ordering on a type `(int, int)`.
 */
-// used in deriving code in libsyntax
 #[inline]
 pub fn lexical_ordering(o1: Ordering, o2: Ordering) -> Ordering {
     match o1 {
diff --git a/src/libsyntax/ext/deriving/cmp/totalord.rs b/src/libsyntax/ext/deriving/cmp/totalord.rs
index 01dacdfe453..001e9235528 100644
--- a/src/libsyntax/ext/deriving/cmp/totalord.rs
+++ b/src/libsyntax/ext/deriving/cmp/totalord.rs
@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use ast;
 use ast::{MetaItem, item, expr};
 use codemap::span;
 use ext::base::ExtCtxt;
@@ -40,40 +41,70 @@ pub fn expand_deriving_totalord(cx: @ExtCtxt,
 }
 
 
-pub fn ordering_const(cx: @ExtCtxt, span: span, cnst: Ordering) -> @expr {
+pub fn ordering_const(cx: @ExtCtxt, span: span, cnst: Ordering) -> ast::Path {
     let cnst = match cnst {
         Less => "Less",
         Equal => "Equal",
         Greater => "Greater"
     };
-    cx.expr_path(
-        cx.path_global(span,
-                       ~[cx.ident_of("std"),
-                         cx.ident_of("cmp"),
-                         cx.ident_of(cnst)]))
+    cx.path_global(span,
+                   ~[cx.ident_of("std"),
+                     cx.ident_of("cmp"),
+                     cx.ident_of(cnst)])
 }
 
 pub fn cs_cmp(cx: @ExtCtxt, span: span,
               substr: &Substructure) -> @expr {
+    let test_id = cx.ident_of("__test");
+    let equals_path = ordering_const(cx, span, Equal);
 
+    /*
+    Builds:
+
+    let __test = self_field1.cmp(&other_field2);
+    if other == ::std::cmp::Equal {
+        let __test = self_field2.cmp(&other_field2);
+        if __test == ::std::cmp::Equal {
+            ...
+        } else {
+            __test
+        }
+    } else {
+        __test
+    }
+
+    FIXME #6449: These `if`s could/should be `match`es.
+    */
     cs_same_method_fold(
-        // foldr (possibly) nests the matches in lexical_ordering better
+        // foldr nests the if-elses correctly, leaving the first field
+        // as the outermost one, and the last as the innermost.
         false,
         |cx, span, old, new| {
-            cx.expr_call_global(span,
-                                ~[cx.ident_of("std"),
-                                  cx.ident_of("cmp"),
-                                  cx.ident_of("lexical_ordering")],
-                                ~[old, new])
+            // let __test = new;
+            // if __test == ::std::cmp::Equal {
+            //    old
+            // } else {
+            //    __test
+            // }
+
+            let assign = cx.stmt_let(span, false, test_id, new);
+
+            let cond = cx.expr_binary(span, ast::eq,
+                                      cx.expr_ident(span, test_id),
+                                      cx.expr_path(equals_path.clone()));
+            let if_ = cx.expr_if(span,
+                                 cond,
+                                 old, Some(cx.expr_ident(span, test_id)));
+            cx.expr_block(cx.block(span, ~[assign], Some(if_)))
         },
-        ordering_const(cx, span, Equal),
+        cx.expr_path(equals_path.clone()),
         |cx, span, list, _| {
             match list {
                 // an earlier nonmatching variant is Less than a
-                // later one
+                // later one.
                 [(self_var, _, _),
-                 (other_var, _, _)] => ordering_const(cx, span,
-                                                      self_var.cmp(&other_var)),
+                 (other_var, _, _)] => cx.expr_path(ordering_const(cx, span,
+                                                                   self_var.cmp(&other_var))),
                 _ => cx.span_bug(span, "Not exactly 2 arguments in `deriving(TotalOrd)`")
             }
         },