Skip to content

Commit

Permalink
fix FIXME(#6449) in #[derive(PartialOrd, Ord)]
Browse files Browse the repository at this point in the history 
This replaces some `if`s with `match`es. This was originally not possible
because using a global path in a match statement caused a "non-constant
path in constant expr" ICE. The issue is long since closed, though you still
hit it (as an error now, not an ICE) if you try to generate match patterns
using pat_lit(expr_path). But it works when constructing the patterns more
carefully.
  • Loading branch information
@durka
durka committed Mar 14, 2016
1 parent d19f1b6 commit 4982f91
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 55 deletions.
50 changes: 23 additions & 27 deletions src/libsyntax_ext/deriving/cmp/ord.rs
View file
Expand Up @@ -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 => {
...
}
__test => __test
},
__test => __test
}
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,
// __test => __test
// }

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
53 changes: 25 additions & 28 deletions src/libsyntax_ext/deriving/cmp/partial_ord.rs
View file
Expand Up @@ -110,38 +110,33 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span,
let test_id = cx.ident_of("__test");
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) => {
...
}
__test => __test
},
__test => __test
}
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,
// __test => __test
// }

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

0 comments on commit 4982f91

Please sign in to comment.