OpenE2K
/
rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #50011 - varkor:partialord-opt-ii, r=Manishearth 

Ensure derive(PartialOrd) is no longer accidentally exponential

Previously, two comparison operations would be generated for each field, each of which could delegate to another derived PartialOrd. Now we use ordering and optional chaining to ensure each pair of fields is only compared once, addressing https://github.com/rust-lang/rust/issues/49650#issuecomment-379467572.

Closes #49505.

r? @Manishearth (sorry for changing it again so soon!)

Close #50755
This commit is contained in:
bors 2018-05-15 03:14:46 +00:00
parent 308b7b05c3 6805e5abd2
commit d711dc9d57
7 changed files with 102 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/etc/generate-deriving-span-tests.py
View File

@ -122,7 +122,7 @@ traits = {
for (trait, supers, errs) in [('Clone', [], 1),
('PartialEq', [], 2),
('PartialOrd', ['PartialEq'], 5),
('PartialOrd', ['PartialEq'], 1),
('Eq', ['PartialEq'], 1),
('Ord', ['Eq', 'PartialOrd', 'PartialEq'], 1),
('Debug', [], 1),

167
src/libsyntax_ext/deriving/cmp/partial_ord.rs
View File

@ -147,34 +147,37 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<
// as the outermost one, and the last as the innermost.
false,
|cx, span, old, self_f, other_fs| {
// match new {
// Some(::std::cmp::Ordering::Equal) => old,
// cmp => cmp
// }
// match new {
// Some(::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)`"),
};
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)`")
}
};
let args = vec![
cx.expr_addr_of(span, self_f),
cx.expr_addr_of(span, other_f.clone()),
];
let args = vec![
cx.expr_addr_of(span, self_f),
cx.expr_addr_of(span, other_f.clone()),
];
cx.expr_call_global(span, partial_cmp_path.clone(), args)
};
cx.expr_call_global(span, partial_cmp_path.clone(), args)
};
let eq_arm = cx.arm(span,
vec![cx.pat_some(span, cx.pat_path(span, ordering.clone()))],
old);
let neq_arm = cx.arm(span,
vec![cx.pat_ident(span, test_id)],
cx.expr_ident(span, test_id));
let eq_arm = cx.arm(span,
vec![cx.pat_some(span, cx.pat_path(span, ordering.clone()))],
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])
},
cx.expr_match(span, new, vec![eq_arm, neq_arm])
},
equals_expr.clone(),
Box::new(|cx, span, (self_args, tag_tuple), _non_self_args| {
if self_args.len() != 2 {
@ -189,78 +192,99 @@ 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<Expr> {
let strict_op = if less { BinOpKind::Lt } else { BinOpKind::Gt };
cs_fold1(false, // need foldr,
fn cs_op(less: bool,
inclusive: bool,
cx: &mut ExtCtxt,
span: Span,
substr: &Substructure) -> P<Expr> {
let ordering_path = |cx: &mut ExtCtxt, name: &str| {
cx.expr_path(cx.path_global(span, cx.std_path(&["cmp", "Ordering", name])))
};
let par_cmp = |cx: &mut ExtCtxt, span, self_f: P<Expr>, other_fs: &[P<Expr>], default| {
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)`"),
};
// `PartialOrd::partial_cmp(self.fi, other.fi)`
let cmp_path = cx.expr_path(cx.path_global(span, cx.std_path(&["cmp",
"PartialOrd",
"partial_cmp"])));
let cmp = cx.expr_call(span,
cmp_path,
vec![cx.expr_addr_of(span, self_f),
cx.expr_addr_of(span, other_f.clone())]);
let default = ordering_path(cx, default);
// `Option::unwrap_or(_, Ordering::Equal)`
let unwrap_path = cx.expr_path(cx.path_global(span, cx.std_path(&["option",
"Option",
"unwrap_or"])));
cx.expr_call(span, unwrap_path, vec![cmp, default])
};
let fold = cs_fold1(false, // need foldr
|cx, span, subexpr, self_f, other_fs| {
// build up a series of chain ||'s and &&'s from the inside
// build up a series of `partial_cmp`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
// Ordering::then_with(
// Option::unwrap_or(
// PartialOrd::partial_cmp(self.f1, other.f1), Ordering::Equal)
// ),
// Option::unwrap_or(
// PartialOrd::partial_cmp(self.f2, other.f2), Ordering::Greater)
// )
// )
// == Ordering::Less
// ```
//
// and for op ==
// `ast::le`
//
// ```
// self.f1 < other.f1 || (self.f1 == other.f1 &&
// self.f2 <= other.f2
// Ordering::then_with(
// Option::unwrap_or(
// PartialOrd::partial_cmp(self.f1, other.f1), Ordering::Equal)
// ),
// Option::unwrap_or(
// PartialOrd::partial_cmp(self.f2, other.f2), Ordering::Greater)
// )
// )
// != Ordering::Greater
// ```
//
// 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 strict_ineq = cx.expr_binary(span, strict_op, self_f.clone(), other_f.clone());
// `Option::unwrap_or(PartialOrd::partial_cmp(self.fi, other.fi), Ordering::Equal)`
let par_cmp = par_cmp(cx, span, self_f, other_fs, "Equal");
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())
};
let and = cx.expr_binary(span, BinOpKind::And, deleg_cmp, subexpr);
cx.expr_binary(span, BinOpKind::Or, strict_ineq, and)
// `Ordering::then_with(Option::unwrap_or(..), ..)`
let then_with_path = cx.expr_path(cx.path_global(span,
cx.std_path(&["cmp",
"Ordering",
"then_with"])));
cx.expr_call(span, then_with_path, vec![par_cmp, cx.lambda0(span, subexpr)])
},
|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)
let opposite = if less { "Greater" } else { "Less" };
par_cmp(cx, span, self_f, other_fs, opposite)
},
None => cx.expr_bool(span, inclusive)
}
},
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) {
let op = match (less, inclusive) {
(false, false) => GtOp,
(false, true) => GeOp,
(true, false) => LtOp,
@ -271,5 +295,16 @@ fn cs_op(less: bool, equal: bool, cx: &mut ExtCtxt, span: Span, substr: &Substru
}),
cx,
span,
substr)
substr);
match *substr.fields {
EnumMatching(.., ref all_fields) |
Struct(.., ref all_fields) if !all_fields.is_empty() => {
let ordering = ordering_path(cx, if less ^ inclusive { "Less" } else { "Greater" });
let comp_op = if inclusive { BinOpKind::Ne } else { BinOpKind::Eq };
cx.expr_binary(span, comp_op, fold, ordering)
}
_ => fold
}
}

4
src/test/compile-fail/derives-span-PartialOrd-enum-struct-variant.rs
View File

@ -17,10 +17,6 @@ struct Error;
enum Enum {
A {
x: Error //~ ERROR
//~^ ERROR
//~^^ ERROR
//~^^^ ERROR
//~^^^^ ERROR
}
}

4
src/test/compile-fail/derives-span-PartialOrd-enum.rs
View File

@ -17,10 +17,6 @@ struct Error;
enum Enum {
A(
Error //~ ERROR
//~^ ERROR
//~^^ ERROR
//~^^^ ERROR
//~^^^^ ERROR
)
}

4
src/test/compile-fail/derives-span-PartialOrd-struct.rs
View File

@ -16,10 +16,6 @@ struct Error;
#[derive(PartialOrd,PartialEq)]
struct Struct {
x: Error //~ ERROR
//~^ ERROR
//~^^ ERROR
//~^^^ ERROR
//~^^^^ ERROR
}
fn main() {}

4
src/test/compile-fail/derives-span-PartialOrd-tuple-struct.rs
View File

@ -16,10 +16,6 @@ struct Error;
#[derive(PartialOrd,PartialEq)]
struct Struct(
Error //~ ERROR
//~^ ERROR
//~^^ ERROR
//~^^^ ERROR
//~^^^^ ERROR
);
fn main() {}

14
src/test/compile-fail/range_traits-1.rs
View File

@ -15,35 +15,21 @@ struct AllTheRanges {
a: Range<usize>,
//~^ ERROR PartialOrd
//~^^ ERROR Ord
//~^^^ ERROR binary operation `<` cannot be applied to type
//~^^^^ ERROR binary operation `>` cannot be applied to type
b: RangeTo<usize>,
//~^ ERROR PartialOrd
//~^^ ERROR Ord
//~^^^ ERROR binary operation `<` cannot be applied to type
//~^^^^ ERROR binary operation `>` cannot be applied to type
c: RangeFrom<usize>,
//~^ ERROR PartialOrd
//~^^ ERROR Ord
//~^^^ ERROR binary operation `<` cannot be applied to type
//~^^^^ ERROR binary operation `>` cannot be applied to type
d: RangeFull,
//~^ ERROR PartialOrd
//~^^ ERROR Ord
//~^^^ ERROR binary operation `<` cannot be applied to type
//~^^^^ ERROR binary operation `>` cannot be applied to type
e: RangeInclusive<usize>,
//~^ ERROR PartialOrd
//~^^ ERROR Ord
//~^^^ ERROR binary operation `<` cannot be applied to type
//~^^^^ ERROR binary operation `>` cannot be applied to type
f: RangeToInclusive<usize>,
//~^ ERROR PartialOrd
//~^^ 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() {}