From 4982f913461a2a6dfbac8bace53961233322646a Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 7 Mar 2016 18:35:01 -0500 Subject: [PATCH] fix FIXME(#6449) in #[derive(PartialOrd, Ord)] 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. --- src/libsyntax_ext/deriving/cmp/ord.rs | 50 +++++++++--------- src/libsyntax_ext/deriving/cmp/partial_ord.rs | 51 +++++++++---------- 2 files changed, 47 insertions(+), 54 deletions(-) diff --git a/src/libsyntax_ext/deriving/cmp/ord.rs b/src/libsyntax_ext/deriving/cmp/ord.rs index 2fa847ee430d..f4082e0b123b 100644 --- a/src/libsyntax_ext/deriving/cmp/ord.rs +++ b/src/libsyntax_ext/deriving/cmp/ord.rs @@ -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![ @@ -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) } diff --git a/src/libsyntax_ext/deriving/cmp/partial_ord.rs b/src/libsyntax_ext/deriving/cmp/partial_ord.rs index e857f7d52f91..3353addebc97 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_ord.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_ord.rs @@ -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 = { @@ -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 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)); - 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_))) + cx.expr_match(span, new, vec![eq_arm, neq_arm]) }, equals_expr.clone(), Box::new(|cx, span, (self_args, tag_tuple), _non_self_args| {