Auto merge of #32139 - durka:derive-fix, r=alexcrichton

cleanups and fixes for #[derive]

This contains a bunch of little cleanups and fixes to `#[derive]`. There are more extensive comments on each individual commit.

- hygiene cleanups
- use `discriminant_value` instead of variant index in `#[derive(Hash)]`
- ~~don't move out of borrowed content in `#[derive(PartialOrd, PartialEq)]`~~
- use `intrinsics::unreachable()` instead of `unreachable!()`

I don't believe there are any breaking changes in here, but I do want some more eyes on that.

Fixes #2810 (!), I believe (we still assume that "std" or "core" is the standard library but so does the rest of rustc...).
Fixes #21714 (cc @apasel422).
~~Fixes~~ (postponed) #24047 (cc @withoutboats @bluss).
Fixes #31714 (cc @alexcrichton @bluss).
Fixes #31886 (cc @oli-obk).

Author: bors

src/libsyntax_ext/deriving/cmp/ord.rs

@@ -11,7 +11,7 @@
 use deriving::generic::*;
 use deriving::generic::ty::*;
 
-use syntax::ast::{MetaItem, Expr, BinOpKind, self};
+use syntax::ast::{MetaItem, Expr, self};
 use syntax::codemap::Span;
 use syntax::ext::base::{ExtCtxt, Annotatable};
 use syntax::ext::build::AstBuilder;
@@ -64,7 +64,7 @@ pub fn ordering_collapsed(cx: &mut ExtCtxt,
 
 pub fn cs_cmp(cx: &mut ExtCtxt, span: Span,
               substr: &Substructure) -> P<Expr> {
-    let test_id = cx.ident_of("__test");
+    let test_id = cx.ident_of("cmp");
     let equals_path = cx.path_global(span,
                                      cx.std_path(&["cmp", "Ordering", "Equal"]));
 
@@ -73,36 +73,31 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span,
     /*
     Builds:
 
-    let __test = ::std::cmp::Ord::cmp(&self_field1, &other_field1);
-    if other == ::std::cmp::Ordering::Equal {
-        let __test = ::std::cmp::Ord::cmp(&self_field2, &other_field2);
-        if __test == ::std::cmp::Ordering::Equal {
-            ...
-        } else {
-            __test
-        }
-    } else {
-        __test
+    match ::std::cmp::Ord::cmp(&self_field1, &other_field1) {
+        ::std::cmp::Ordering::Equal =>
+            match ::std::cmp::Ord::cmp(&self_field2, &other_field2) {
+                ::std::cmp::Ordering::Equal => {
+                    ...
+                }
+                cmp => cmp
+            },
+        cmp => cmp
     }
-
-    FIXME #6449: These `if`s could/should be `match`es.
     */
     cs_fold(
         // foldr nests the if-elses correctly, leaving the first field
         // as the outermost one, and the last as the innermost.
         false,
         |cx, span, old, self_f, other_fs| {
-            // let __test = new;
-            // if __test == ::std::cmp::Ordering::Equal {
-            //    old
-            // } else {
-            //    __test
+            // match new {
+            //     ::std::cmp::Ordering::Equal => old,
+            //     cmp => cmp
             // }
 
             let new = {
                 let other_f = match (other_fs.len(), other_fs.get(0)) {
                     (1, Some(o_f)) => o_f,
-                    _ => cx.span_bug(span, "not exactly 2 arguments in `derive(PartialOrd)`"),
+                    _ => cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`"),
                 };
 
                 let args = vec![
@@ -113,20 +108,21 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span,
                 cx.expr_call_global(span, cmp_path.clone(), args)
             };
 
-            let assign = cx.stmt_let(span, false, test_id, new);
+            let eq_arm = cx.arm(span,
+                                vec![cx.pat_enum(span,
+                                                 equals_path.clone(),
+                                                 vec![])],
+                                old);
+            let neq_arm = cx.arm(span,
+                                 vec![cx.pat_ident(span, test_id)],
+                                 cx.expr_ident(span, test_id));
 
-            let cond = cx.expr_binary(span, BinOpKind::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, vec!(assign), Some(if_)))
+            cx.expr_match(span, new, vec![eq_arm, neq_arm])
         },
         cx.expr_path(equals_path.clone()),
         Box::new(|cx, span, (self_args, tag_tuple), _non_self_args| {
             if self_args.len() != 2 {
-                cx.span_bug(span, "not exactly 2 arguments in `derives(Ord)`")
+                cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`")
             } else {
                 ordering_collapsed(cx, span, tag_tuple)
             }

src/libsyntax_ext/deriving/cmp/partial_ord.rs

@@ -107,41 +107,36 @@ pub fn some_ordering_collapsed(cx: &mut ExtCtxt,
 
 pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span,
               substr: &Substructure) -> P<Expr> {
-    let test_id = cx.ident_of("__test");
+    let test_id = cx.ident_of("cmp");
     let ordering = cx.path_global(span,
                                   cx.std_path(&["cmp", "Ordering", "Equal"]));
-    let ordering = cx.expr_path(ordering);
-    let equals_expr = cx.expr_some(span, ordering);
+    let ordering_expr = cx.expr_path(ordering.clone());
+    let equals_expr = cx.expr_some(span, ordering_expr);
 
     let partial_cmp_path = cx.std_path(&["cmp", "PartialOrd", "partial_cmp"]);
 
     /*
     Builds:
 
-    let __test = ::std::cmp::PartialOrd::partial_cmp(&self_field1, &other_field1);
-    if __test == ::std::option::Option::Some(::std::cmp::Ordering::Equal) {
-        let __test = ::std::cmp::PartialOrd::partial_cmp(&self_field2, &other_field2);
-        if __test == ::std::option::Option::Some(::std::cmp::Ordering::Equal) {
-            ...
-        } else {
-            __test
-        }
-    } else {
-        __test
+    match ::std::cmp::PartialOrd::partial_cmp(&self_field1, &other_field1) {
+        ::std::option::Option::Some(::std::cmp::Ordering::Equal) =>
+            match ::std::cmp::PartialOrd::partial_cmp(&self_field2, &other_field2) {
+                ::std::option::Option::Some(::std::cmp::Ordering::Equal) => {
+                    ...
+                }
+                cmp => cmp
+            },
+        cmp => cmp
     }
-
-    FIXME #6449: These `if`s could/should be `match`es.
     */
     cs_fold(
         // foldr nests the if-elses correctly, leaving the first field
         // as the outermost one, and the last as the innermost.
         false,
         |cx, span, old, self_f, other_fs| {
-            // let __test = new;
-            // if __test == Some(::std::cmp::Ordering::Equal) {
-            //    old
-            // } else {
-            //    __test
+            // match new {
+            //     Some(::std::cmp::Ordering::Equal) => old,
+            //     cmp => cmp
             // }
 
             let new = {
@@ -158,15 +153,17 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span,
                 cx.expr_call_global(span, partial_cmp_path.clone(), args)
             };
 
-            let assign = cx.stmt_let(span, false, test_id, new);
-
-            let cond = cx.expr_binary(span, BinOpKind::Eq,
-                                      cx.expr_ident(span, test_id),
-                                      equals_expr.clone());
-            let if_ = cx.expr_if(span,
-                                 cond,
-                                 old, Some(cx.expr_ident(span, test_id)));
-            cx.expr_block(cx.block(span, vec!(assign), Some(if_)))
+            let eq_arm = cx.arm(span,
+                                vec![cx.pat_some(span,
+                                                 cx.pat_enum(span,
+                                                             ordering.clone(),
+                                                             vec![]))],
+                                old);
+            let neq_arm = cx.arm(span,
+                                 vec![cx.pat_ident(span, test_id)],
+                                 cx.expr_ident(span, test_id));
+
+            cx.expr_match(span, new, vec![eq_arm, neq_arm])
         },
         equals_expr.clone(),
         Box::new(|cx, span, (self_args, tag_tuple), _non_self_args| {

src/libsyntax_ext/deriving/decodable.rs

@@ -10,6 +10,7 @@
 
 //! The compiler code necessary for `#[derive(Decodable)]`. See encodable.rs for more.
 
+use deriving;
 use deriving::generic::*;
 use deriving::generic::ty::*;
 
@@ -54,6 +55,8 @@ fn expand_deriving_decodable_imp(cx: &mut ExtCtxt,
         return
     }
 
+    let typaram = &*deriving::hygienic_type_parameter(item, "__D");
+
     let trait_def = TraitDef {
         span: span,
         attributes: Vec::new(),
@@ -66,18 +69,17 @@ fn expand_deriving_decodable_imp(cx: &mut ExtCtxt,
                 name: "decode",
                 generics: LifetimeBounds {
                     lifetimes: Vec::new(),
-                    bounds: vec!(("__D", vec!(Path::new_(
-                                    vec!(krate, "Decoder"), None,
-                                    vec!(), true))))
+                    bounds: vec![(typaram,
+                                  vec![Path::new_(vec!(krate, "Decoder"), None, vec!(), true)])]
                 },
                 explicit_self: None,
-                args: vec!(Ptr(Box::new(Literal(Path::new_local("__D"))),
+                args: vec!(Ptr(Box::new(Literal(Path::new_local(typaram))),
                             Borrowed(None, Mutability::Mutable))),
                 ret_ty: Literal(Path::new_(
                     pathvec_std!(cx, core::result::Result),
                     None,
                     vec!(Box::new(Self_), Box::new(Literal(Path::new_(
-                        vec!["__D", "Error"], None, vec![], false
+                        vec![typaram, "Error"], None, vec![], false
                     )))),
                     true
                 )),

src/libsyntax_ext/deriving/encodable.rs

@@ -88,6 +88,7 @@
 //! }
 //! ```
 
+use deriving;
 use deriving::generic::*;
 use deriving::generic::ty::*;
 
@@ -130,6 +131,8 @@ fn expand_deriving_encodable_imp(cx: &mut ExtCtxt,
         return;
     }
 
+    let typaram = &*deriving::hygienic_type_parameter(item, "__S");
+
     let trait_def = TraitDef {
         span: span,
         attributes: Vec::new(),
@@ -142,18 +145,17 @@ fn expand_deriving_encodable_imp(cx: &mut ExtCtxt,
                 name: "encode",
                 generics: LifetimeBounds {
                     lifetimes: Vec::new(),
-                    bounds: vec!(("__S", vec!(Path::new_(
-                                    vec!(krate, "Encoder"), None,
-                                    vec!(), true))))
+                    bounds: vec![(typaram,
+                                  vec![Path::new_(vec![krate, "Encoder"], None, vec!(), true)])]
                 },
                 explicit_self: borrowed_explicit_self(),
-                args: vec!(Ptr(Box::new(Literal(Path::new_local("__S"))),
+                args: vec!(Ptr(Box::new(Literal(Path::new_local(typaram))),
                             Borrowed(None, Mutability::Mutable))),
                 ret_ty: Literal(Path::new_(
                     pathvec_std!(cx, core::result::Result),
                     None,
                     vec!(Box::new(Tuple(Vec::new())), Box::new(Literal(Path::new_(
-                        vec!["__S", "Error"], None, vec![], false
+                        vec![typaram, "Error"], None, vec![], false
                     )))),
                     true
                 )),