Deriving changes

src/libsyntax/ext/deriving/clone.rs

@@ -38,9 +38,7 @@ pub fn expand_deriving_clone(cx: @ExtCtxt,
         ]
     };
 
-    expand_deriving_generic(cx, span,
-                            mitem, in_items,
-                            &trait_def)
+    trait_def.expand(cx, span, mitem, in_items)
 }
 
 pub fn expand_deriving_deep_clone(cx: @ExtCtxt,
@@ -67,9 +65,7 @@ pub fn expand_deriving_deep_clone(cx: @ExtCtxt,
         ]
     };
 
-    expand_deriving_generic(cx, span,
-                            mitem, in_items,
-                            &trait_def)
+    trait_def.expand(cx, span, mitem, in_items)
 }
 
 fn cs_clone(

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

@@ -54,7 +54,5 @@ pub fn expand_deriving_eq(cx: @ExtCtxt,
             md!("ne", cs_ne)
         ]
     };
-
-    expand_deriving_generic(cx, span, mitem, in_items,
-                            &trait_def)
+    trait_def.expand(cx, span, mitem, in_items)
 }

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

@@ -10,6 +10,7 @@
 
 use core::prelude::*;
 
+use ast;
 use ast::{meta_item, item, expr};
 use codemap::span;
 use ext::base::ExtCtxt;
@@ -21,104 +22,86 @@ pub fn expand_deriving_ord(cx: @ExtCtxt,
                            mitem: @meta_item,
                            in_items: ~[@item]) -> ~[@item] {
     macro_rules! md (
-        ($name:expr, $less:expr, $equal:expr) => {
+        ($name:expr, $op:expr, $equal:expr) => {
             MethodDef {
                 name: $name,
                 generics: LifetimeBounds::empty(),
                 explicit_self: borrowed_explicit_self(),
                 args: ~[borrowed_self()],
                 ret_ty: Literal(Path::new(~["bool"])),
                 const_nonmatching: false,
-                combine_substructure: |cx, span, substr|
-                    cs_ord($less, $equal, cx, span, substr)
+                combine_substructure: |cx, span, substr| cs_op($op, $equal, cx, span, substr)
             }
         }
     );
 
-
-
     let trait_def = TraitDef {
         path: Path::new(~["std", "cmp", "Ord"]),
-        // XXX: Ord doesn't imply Eq yet
-        additional_bounds: ~[Literal(Path::new(~["std", "cmp", "Eq"]))],
+        additional_bounds: ~[],
         generics: LifetimeBounds::empty(),
         methods: ~[
-            md!("lt", true,  false),
-            md!("le", true,  true),
+            md!("lt", true, false),
+            md!("le", true, true),
             md!("gt", false, false),
             md!("ge", false, true)
         ]
     };
-
-    expand_deriving_generic(cx, span, mitem, in_items,
-                            &trait_def)
+    trait_def.expand(cx, span, mitem, in_items)
 }
 
-/// `less`: is this `lt` or `le`? `equal`: is this `le` or `ge`?
-fn cs_ord(less: bool, equal: bool,
-          cx: @ExtCtxt, span: span,
-          substr: &Substructure) -> @expr {
-    let binop = if less {
-        cx.ident_of("lt")
-    } else {
-        cx.ident_of("gt")
-    };
-    let base = cx.expr_bool(span, equal);
-
+/// Strict inequality.
+fn cs_op(less: bool, equal: bool, cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr {
+    let op = if less {ast::lt} else {ast::gt};
     cs_fold(
         false, // need foldr,
         |cx, span, subexpr, self_f, other_fs| {
             /*
-
-            build up a series of nested ifs from the inside out to get
-            lexical ordering (hence foldr), i.e.
+            build up a series of chain ||'s and &&'s from the inside
+            out (hence foldr) to get lexical ordering, i.e. for op ==
+            `ast::lt`
 
             ```
-            if self.f1 `binop` other.f1 {
-                true
-            } else if self.f1 == other.f1 {
-                if self.f2 `binop` other.f2 {
-                    true
-                } else if self.f2 == other.f2 {
-                    `equal`
-                } else {
-                    false
-                }
-            } else {
-                false
-            }
+            self.f1 < other.f1 || (!(other.f1 < self.f1) &&
+                (self.f2 < other.f2 || (!(other.f2 < self.f2) &&
+                    (false)
+                ))
+            )
             ```
 
-            The inner "`equal`" case is only reached if the two
-            items have all fields equal.
+            The optimiser should remove the redundancy. We explicitly
+            get use the binops to avoid auto-deref derefencing too many
+            layers of pointers, if the type includes pointers.
             */
-            if other_fs.len() != 1 {
-                cx.span_bug(span, "Not exactly 2 arguments in `deriving(Ord)`");
-            }
+            let other_f = match other_fs {
+                [o_f] => o_f,
+                _ => cx.span_bug(span, "Not exactly 2 arguments in `deriving(Ord)`")
+            };
+
+            let cmp = cx.expr_binary(span, op,
+                                     cx.expr_deref(span, self_f),
+                                     cx.expr_deref(span, other_f));
 
-            let cmp = cx.expr_method_call(span,
-                                          self_f, cx.ident_of("eq"), other_fs.to_owned());
-            let elseif = cx.expr_if(span, cmp,
-                                    subexpr, Some(cx.expr_bool(span, false)));
+            let not_cmp = cx.expr_unary(span, ast::not,
+                                        cx.expr_binary(span, op,
+                                                       cx.expr_deref(span, other_f),
+                                                       cx.expr_deref(span, self_f)));
 
-            let cmp = cx.expr_method_call(span,
-                                          self_f, binop, other_fs.to_owned());
-            cx.expr_if(span, cmp,
-                        cx.expr_bool(span, true), Some(elseif))
+            let and = cx.expr_binary(span, ast::and, not_cmp, subexpr);
+            cx.expr_binary(span, ast::or, cmp, and)
         },
-        base,
+        cx.expr_bool(span, equal),
         |cx, span, args, _| {
             // nonmatching enums, order by the order the variants are
             // written
             match args {
                 [(self_var, _, _),
                  (other_var, _, _)] =>
                     cx.expr_bool(span,
-                                   if less {
-                                       self_var < other_var
-                                   } else {
-                                       self_var > other_var
-                                   }),
+                                 if less {
+                                     self_var < other_var
+                                 } else {
+                                     self_var > other_var
+                                 }),
                 _ => cx.span_bug(span, "Not exactly 2 arguments in `deriving(Ord)`")
             }
         },

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

@@ -42,7 +42,5 @@ pub fn expand_deriving_totaleq(cx: @ExtCtxt,
             }
         ]
     };
-
-    expand_deriving_generic(cx, span, mitem, in_items,
-                            &trait_def)
+    trait_def.expand(cx, span, mitem, in_items)
 }

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

@@ -38,8 +38,7 @@ pub fn expand_deriving_totalord(cx: @ExtCtxt,
         ]
     };
 
-    expand_deriving_generic(cx, span, mitem, in_items,
-                            &trait_def)
+    trait_def.expand(cx, span, mitem, in_items)
 }
 
 

src/libsyntax/ext/deriving/decodable.rs

@@ -49,8 +49,7 @@ pub fn expand_deriving_decodable(cx: @ExtCtxt,
         ]
     };
 
-    expand_deriving_generic(cx, span, mitem, in_items,
-                            &trait_def)
+    trait_def.expand(cx, span, mitem, in_items)
 }
 
 fn decodable_substructure(cx: @ExtCtxt, span: span,

src/libsyntax/ext/deriving/encodable.rs

@@ -109,8 +109,7 @@ pub fn expand_deriving_encodable(cx: @ExtCtxt,
         ]
     };
 
-    expand_deriving_generic(cx, span, mitem, in_items,
-                            &trait_def)
+    trait_def.expand(cx, span, mitem, in_items)
 }
 
 fn encodable_substructure(cx: @ExtCtxt, span: span,