diff --git a/src/etc/generate-deriving-span-tests.py b/src/etc/generate-deriving-span-tests.py index 15c9fc2e504a1..da9be3aee93ee 100755 --- a/src/etc/generate-deriving-span-tests.py +++ b/src/etc/generate-deriving-span-tests.py @@ -113,7 +113,7 @@ def write_file(name, string): for (trait, supers, errs) in [('Clone', [], 1), ('PartialEq', [], 2), - ('PartialOrd', ['PartialEq'], 3), + ('PartialOrd', ['PartialEq'], 5), ('Eq', ['PartialEq'], 1), ('Ord', ['Eq', 'PartialOrd', 'PartialEq'], 1), ('Debug', [], 1), diff --git a/src/libsyntax_ext/deriving/cmp/partial_eq.rs b/src/libsyntax_ext/deriving/cmp/partial_eq.rs index 75db7cc1e4c07..81ca7e732283d 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_eq.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_eq.rs @@ -26,41 +26,48 @@ pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt, push: &mut FnMut(Annotatable)) { // structures are equal if all fields are equal, and non equal, if // any fields are not equal or if the enum variants are different - fn cs_eq(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P { - cs_fold(true, // use foldl - |cx, span, subexpr, self_f, other_fs| { + fn cs_op(cx: &mut ExtCtxt, + span: Span, + substr: &Substructure, + op: BinOpKind, + combiner: BinOpKind, + base: bool) + -> P + { + let op = |cx: &mut ExtCtxt, span: Span, self_f: P, other_fs: &[P]| { 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(PartialEq)`"), }; - let eq = cx.expr_binary(span, BinOpKind::Eq, self_f, other_f.clone()); + cx.expr_binary(span, op, self_f, other_f.clone()) + }; - cx.expr_binary(span, BinOpKind::And, subexpr, eq) - }, - cx.expr_bool(span, true), - Box::new(|cx, span, _, _| cx.expr_bool(span, false)), - cx, - span, - substr) + cs_fold1(true, // use foldl + |cx, span, subexpr, self_f, other_fs| { + let eq = op(cx, span, self_f, other_fs); + cx.expr_binary(span, combiner, subexpr, eq) + }, + |cx, args| { + match args { + Some((span, self_f, other_fs)) => { + // Special-case the base case to generate cleaner code. + op(cx, span, self_f, other_fs) + } + None => cx.expr_bool(span, base), + } + }, + Box::new(|cx, span, _, _| cx.expr_bool(span, !base)), + cx, + span, + substr) } - fn cs_ne(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P { - cs_fold(true, // use foldl - |cx, span, subexpr, self_f, other_fs| { - 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(PartialEq)`"), - }; - - let eq = cx.expr_binary(span, BinOpKind::Ne, self_f, other_f.clone()); - cx.expr_binary(span, BinOpKind::Or, subexpr, eq) - }, - cx.expr_bool(span, false), - Box::new(|cx, span, _, _| cx.expr_bool(span, true)), - cx, - span, - substr) + fn cs_eq(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P { + cs_op(cx, span, substr, BinOpKind::Eq, BinOpKind::And, true) + } + fn cs_ne(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P { + cs_op(cx, span, substr, BinOpKind::Ne, BinOpKind::Or, false) } macro_rules! md { diff --git a/src/libsyntax_ext/deriving/cmp/partial_ord.rs b/src/libsyntax_ext/deriving/cmp/partial_ord.rs index 92183c58eb269..9560fd0570af7 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_ord.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_ord.rs @@ -190,54 +190,86 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P< /// Strict inequality. fn cs_op(less: bool, equal: bool, cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P { - let op = if less { BinOpKind::Lt } else { BinOpKind::Gt }; - cs_fold(false, // need foldr, - |cx, span, subexpr, self_f, other_fs| { - // 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` - // - // ``` - // self.f1 < other.f1 || (!(other.f1 < self.f1) && - // (self.f2 < other.f2 || (!(other.f2 < self.f2) && - // (false) - // )) - // ) - // ``` - // - // The optimiser should remove the redundancy. We explicitly - // get use the binops to avoid auto-deref dereferencing too many - // layers of pointers, if the type includes pointers. - // - 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)`"), - }; - - let cmp = cx.expr_binary(span, op, self_f.clone(), other_f.clone()); + let strict_op = if less { BinOpKind::Lt } else { BinOpKind::Gt }; + cs_fold1(false, // need foldr, + |cx, span, subexpr, self_f, other_fs| { + // 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` + // + // ``` + // self.f1 < other.f1 || (!(other.f1 < self.f1) && + // self.f2 < other.f2 + // ) + // ``` + // + // and for op == + // `ast::le` + // + // ``` + // self.f1 < other.f1 || (self.f1 == other.f1 && + // self.f2 <= other.f2 + // ) + // ``` + // + // The optimiser should remove the redundancy. We explicitly + // get use the binops to avoid auto-deref dereferencing too many + // layers of pointers, if the type includes pointers. + // + 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)`"), + }; - let not_cmp = cx.expr_unary(span, - ast::UnOp::Not, - cx.expr_binary(span, op, other_f.clone(), self_f)); + let strict_ineq = cx.expr_binary(span, strict_op, self_f.clone(), other_f.clone()); - let and = cx.expr_binary(span, BinOpKind::And, not_cmp, subexpr); - cx.expr_binary(span, BinOpKind::Or, cmp, and) - }, - cx.expr_bool(span, equal), - 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 `derive(PartialOrd)`") - } else { - let op = match (less, equal) { - (true, true) => LeOp, - (true, false) => LtOp, - (false, true) => GeOp, - (false, false) => GtOp, + let deleg_cmp = if !equal { + cx.expr_unary(span, + ast::UnOp::Not, + cx.expr_binary(span, strict_op, other_f.clone(), self_f)) + } else { + cx.expr_binary(span, BinOpKind::Eq, self_f, other_f.clone()) }; - some_ordering_collapsed(cx, span, op, tag_tuple) - } - }), - cx, - span, - substr) + + let and = cx.expr_binary(span, BinOpKind::And, deleg_cmp, subexpr); + cx.expr_binary(span, BinOpKind::Or, strict_ineq, and) + }, + |cx, args| { + match args { + Some((span, self_f, other_fs)) => { + // Special-case the base case to generate cleaner code with + // fewer operations (e.g. `<=` instead of `<` and `==`). + 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)`"), + }; + + let op = match (less, equal) { + (false, false) => BinOpKind::Gt, + (false, true) => BinOpKind::Ge, + (true, false) => BinOpKind::Lt, + (true, true) => BinOpKind::Le, + }; + + cx.expr_binary(span, op, self_f, other_f.clone()) + } + None => cx.expr_bool(span, equal) + } + }, + 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 `derive(PartialOrd)`") + } else { + let op = match (less, equal) { + (false, false) => GtOp, + (false, true) => GeOp, + (true, false) => LtOp, + (true, true) => LeOp, + }; + some_ordering_collapsed(cx, span, op, tag_tuple) + } + }), + cx, + span, + substr) } diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 4126ce79f3555..82ed6a29b779d 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -1676,12 +1676,55 @@ impl<'a> TraitDef<'a> { // helpful premade recipes +pub fn cs_fold_fields<'a, F>(use_foldl: bool, + mut f: F, + base: P, + cx: &mut ExtCtxt, + all_fields: &[FieldInfo<'a>]) + -> P + where F: FnMut(&mut ExtCtxt, Span, P, P, &[P]) -> P +{ + if use_foldl { + all_fields.iter().fold(base, |old, field| { + f(cx, field.span, old, field.self_.clone(), &field.other) + }) + } else { + all_fields.iter().rev().fold(base, |old, field| { + f(cx, field.span, old, field.self_.clone(), &field.other) + }) + } +} + +pub fn cs_fold_enumnonmatch(mut enum_nonmatch_f: EnumNonMatchCollapsedFunc, + cx: &mut ExtCtxt, + trait_span: Span, + substructure: &Substructure) + -> P +{ + match *substructure.fields { + EnumNonMatchingCollapsed(ref all_args, _, tuple) => { + enum_nonmatch_f(cx, + trait_span, + (&all_args[..], tuple), + substructure.nonself_args) + } + _ => cx.span_bug(trait_span, "cs_fold_enumnonmatch expected an EnumNonMatchingCollapsed") + } +} + +pub fn cs_fold_static(cx: &mut ExtCtxt, + trait_span: Span) + -> P +{ + cx.span_bug(trait_span, "static function in `derive`") +} + /// Fold the fields. `use_foldl` controls whether this is done /// left-to-right (`true`) or right-to-left (`false`). pub fn cs_fold(use_foldl: bool, - mut f: F, + f: F, base: P, - mut enum_nonmatch_f: EnumNonMatchCollapsedFunc, + enum_nonmatch_f: EnumNonMatchCollapsedFunc, cx: &mut ExtCtxt, trait_span: Span, substructure: &Substructure) @@ -1691,26 +1734,65 @@ pub fn cs_fold(use_foldl: bool, match *substructure.fields { EnumMatching(.., ref all_fields) | Struct(_, ref all_fields) => { - if use_foldl { - all_fields.iter().fold(base, |old, field| { - f(cx, field.span, old, field.self_.clone(), &field.other) - }) - } else { - all_fields.iter().rev().fold(base, |old, field| { - f(cx, field.span, old, field.self_.clone(), &field.other) - }) - } + cs_fold_fields(use_foldl, f, base, cx, all_fields) } - EnumNonMatchingCollapsed(ref all_args, _, tuple) => { - enum_nonmatch_f(cx, - trait_span, - (&all_args[..], tuple), - substructure.nonself_args) + EnumNonMatchingCollapsed(..) => { + cs_fold_enumnonmatch(enum_nonmatch_f, cx, trait_span, substructure) + } + StaticEnum(..) | StaticStruct(..) => { + cs_fold_static(cx, trait_span) } - StaticEnum(..) | StaticStruct(..) => cx.span_bug(trait_span, "static function in `derive`"), } } +/// Function to fold over fields, with three cases, to generate more efficient and concise code. +/// When the `substructure` has grouped fields, there are two cases: +/// Zero fields: call the base case function with None (like the usual base case of `cs_fold`). +/// One or more fields: call the base case function on the first value (which depends on +/// `use_fold`), and use that as the base case. Then perform `cs_fold` on the remainder of the +/// fields. +/// When the `substructure` is a `EnumNonMatchingCollapsed`, the result of `enum_nonmatch_f` +/// is returned. Statics may not be folded over. +/// See `cs_op` in `partial_ord.rs` for a model example. +pub fn cs_fold1(use_foldl: bool, + f: F, + mut b: B, + enum_nonmatch_f: EnumNonMatchCollapsedFunc, + cx: &mut ExtCtxt, + trait_span: Span, + substructure: &Substructure) + -> P + where F: FnMut(&mut ExtCtxt, Span, P, P, &[P]) -> P, + B: FnMut(&mut ExtCtxt, Option<(Span, P, &[P])>) -> P +{ + match *substructure.fields { + EnumMatching(.., ref all_fields) | + Struct(_, ref all_fields) => { + let (base, all_fields) = match (all_fields.is_empty(), use_foldl) { + (false, true) => { + let field = &all_fields[0]; + let args = (field.span, field.self_.clone(), &field.other[..]); + (b(cx, Some(args)), &all_fields[1..]) + } + (false, false) => { + let idx = all_fields.len() - 1; + let field = &all_fields[idx]; + let args = (field.span, field.self_.clone(), &field.other[..]); + (b(cx, Some(args)), &all_fields[..idx]) + } + (true, _) => (b(cx, None), &all_fields[..]) + }; + + cs_fold_fields(use_foldl, f, base, cx, all_fields) + } + EnumNonMatchingCollapsed(..) => { + cs_fold_enumnonmatch(enum_nonmatch_f, cx, trait_span, substructure) + } + StaticEnum(..) | StaticStruct(..) => { + cs_fold_static(cx, trait_span) + } + } +} /// Call the method that is being derived on all the fields, and then /// process the collected results. i.e. diff --git a/src/test/compile-fail/derives-span-PartialOrd-enum-struct-variant.rs b/src/test/compile-fail/derives-span-PartialOrd-enum-struct-variant.rs index cf3d69bc16c43..dcf02f308307d 100644 --- a/src/test/compile-fail/derives-span-PartialOrd-enum-struct-variant.rs +++ b/src/test/compile-fail/derives-span-PartialOrd-enum-struct-variant.rs @@ -1,4 +1,4 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -19,6 +19,8 @@ enum Enum { x: Error //~ ERROR //~^ ERROR //~^^ ERROR +//~^^^ ERROR +//~^^^^ ERROR } } diff --git a/src/test/compile-fail/derives-span-PartialOrd-enum.rs b/src/test/compile-fail/derives-span-PartialOrd-enum.rs index c4d587237a52f..7eb44c7e19e84 100644 --- a/src/test/compile-fail/derives-span-PartialOrd-enum.rs +++ b/src/test/compile-fail/derives-span-PartialOrd-enum.rs @@ -1,4 +1,4 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -19,6 +19,8 @@ enum Enum { Error //~ ERROR //~^ ERROR //~^^ ERROR +//~^^^ ERROR +//~^^^^ ERROR ) } diff --git a/src/test/compile-fail/derives-span-PartialOrd-struct.rs b/src/test/compile-fail/derives-span-PartialOrd-struct.rs index e065abd9b46a2..36dae0124ce9b 100644 --- a/src/test/compile-fail/derives-span-PartialOrd-struct.rs +++ b/src/test/compile-fail/derives-span-PartialOrd-struct.rs @@ -1,4 +1,4 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -18,6 +18,8 @@ struct Struct { x: Error //~ ERROR //~^ ERROR //~^^ ERROR +//~^^^ ERROR +//~^^^^ ERROR } fn main() {} diff --git a/src/test/compile-fail/derives-span-PartialOrd-tuple-struct.rs b/src/test/compile-fail/derives-span-PartialOrd-tuple-struct.rs index f2df01222b989..fd2df0967545b 100644 --- a/src/test/compile-fail/derives-span-PartialOrd-tuple-struct.rs +++ b/src/test/compile-fail/derives-span-PartialOrd-tuple-struct.rs @@ -1,4 +1,4 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -18,6 +18,8 @@ struct Struct( Error //~ ERROR //~^ ERROR //~^^ ERROR +//~^^^ ERROR +//~^^^^ ERROR ); fn main() {} diff --git a/src/test/compile-fail/range_traits-1.rs b/src/test/compile-fail/range_traits-1.rs index 7645dbb1a6dee..df766e361d5bd 100644 --- a/src/test/compile-fail/range_traits-1.rs +++ b/src/test/compile-fail/range_traits-1.rs @@ -42,6 +42,8 @@ struct AllTheRanges { //~^^ ERROR Ord //~^^^ ERROR binary operation `<` cannot be applied to type //~^^^^ ERROR binary operation `>` cannot be applied to type + //~^^^^^ ERROR binary operation `<=` cannot be applied to type + //~^^^^^^ ERROR binary operation `>=` cannot be applied to type } fn main() {} diff --git a/src/test/run-pass/derive-partialord-correctness.rs b/src/test/run-pass/derive-partialord-correctness.rs new file mode 100644 index 0000000000000..bc9e9a700875e --- /dev/null +++ b/src/test/run-pass/derive-partialord-correctness.rs @@ -0,0 +1,18 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Original issue: #49650 + +#[derive(PartialOrd, PartialEq)] +struct FloatWrapper(f64); + +fn main() { + assert!((0.0 / 0.0 >= 0.0) == (FloatWrapper(0.0 / 0.0) >= FloatWrapper(0.0))) +}