Skip to content

Commit

Permalink
Rollup merge of rust-lang#32139 - durka:derive-fix, r=alexcrichton
Browse files Browse the repository at this point in the history 
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 rust-lang#2810 (!), I believe (we still assume that "std" or "core" is the standard library but so does the rest of rustc...).
Fixes rust-lang#21714 (cc @apasel422).
~~Fixes~~ (postponed) rust-lang#24047 (cc @withoutboats @bluss).
Fixes rust-lang#31714 (cc @alexcrichton @bluss).
Fixes rust-lang#31886 (cc @oli-obk).
  • Loading branch information
@Manishearth
Manishearth committed Mar 12, 2016
2 parents 1a019dc + 8cccdd0 commit 7ca8fad
Show file tree
Hide file tree
Showing 11 changed files with 242 additions and 157 deletions.
54 changes: 25 additions & 29 deletions src/libsyntax_ext/deriving/cmp/ord.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"]));

Expand All @@ -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![
Expand All @@ -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)
}
Expand Down
55 changes: 26 additions & 29 deletions src/libsyntax_ext/deriving/cmp/partial_ord.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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| {
Expand Down
12 changes: 7 additions & 5 deletions src/libsyntax_ext/deriving/decodable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;

Expand Down Expand Up @@ -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(),
Expand All @@ -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
)),
Expand Down
12 changes: 7 additions & 5 deletions src/libsyntax_ext/deriving/encodable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
//! }
//! ```

use deriving;
use deriving::generic::*;
use deriving::generic::ty::*;

Expand Down Expand Up @@ -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(),
Expand All @@ -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
)),
Expand Down
Loading

0 comments on commit 7ca8fad

Please sign in to comment.