Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix derived PartialOrd operators #81384

Merged
merged 2 commits into from
Feb 9, 2021
Merged

Fix derived PartialOrd operators #81384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e4efccd
Fix derived PartialOrd operators
tmiasko Jan 25, 2021
62366ee
./x.py test --bless
tmiasko Feb 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
197 changes: 7 additions & 190 deletions compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
pub use OrderingOp::*;

use crate::deriving::generic::ty::*;
use crate::deriving::generic::*;
use crate::deriving::{path_local, path_std, pathvec_std};
use crate::deriving::{path_std, pathvec_std};

use rustc_ast::ptr::P;
use rustc_ast::{self as ast, BinOpKind, Expr, MetaItem};
use rustc_ast::{Expr, MetaItem};
use rustc_expand::base::{Annotatable, ExtCtxt};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::symbol::{sym, Ident};
use rustc_span::Span;

pub fn expand_deriving_partial_ord(
Expand All @@ -17,26 +15,6 @@ pub fn expand_deriving_partial_ord(
item: &Annotatable,
push: &mut dyn FnMut(Annotatable),
) {
macro_rules! md {
($name:expr, $op:expr, $equal:expr) => {{
let inline = cx.meta_word(span, sym::inline);
let attrs = vec![cx.attribute(inline)];
MethodDef {
name: $name,
generics: Bounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![(borrowed_self(), sym::other)],
ret_ty: Literal(path_local!(bool)),
attributes: attrs,
is_unsafe: false,
unify_fieldless_variants: true,
combine_substructure: combine_substructure(Box::new(|cx, span, substr| {
cs_op($op, $equal, cx, span, substr)
})),
}
}};
}

let ordering_ty = Literal(path_std!(cmp::Ordering));
let ret_ty = Literal(Path::new_(
pathvec_std!(option::Option),
Expand All @@ -62,21 +40,6 @@ pub fn expand_deriving_partial_ord(
})),
};

// avoid defining extra methods if we can
// c-like enums, enums without any fields and structs without fields
// can safely define only `partial_cmp`.
let methods = if is_type_without_fields(item) {
vec![partial_cmp_def]
} else {
vec![
partial_cmp_def,
md!(sym::lt, true, false),
md!(sym::le, true, true),
md!(sym::gt, false, false),
md!(sym::ge, false, true),
]
};

let trait_def = TraitDef {
span,
attributes: vec![],
Expand All @@ -85,39 +48,12 @@ pub fn expand_deriving_partial_ord(
generics: Bounds::empty(),
is_unsafe: false,
supports_unions: false,
methods,
methods: vec![partial_cmp_def],
associated_types: Vec::new(),
};
trait_def.expand(cx, mitem, item, push)
}

#[derive(Copy, Clone)]
pub enum OrderingOp {
PartialCmpOp,
LtOp,
LeOp,
GtOp,
GeOp,
}

pub fn some_ordering_collapsed(
cx: &mut ExtCtxt<'_>,
span: Span,
op: OrderingOp,
self_arg_tags: &[Ident],
) -> P<ast::Expr> {
let lft = cx.expr_ident(span, self_arg_tags[0]);
let rgt = cx.expr_addr_of(span, cx.expr_ident(span, self_arg_tags[1]));
let op_sym = match op {
PartialCmpOp => sym::partial_cmp,
LtOp => sym::lt,
LeOp => sym::le,
GtOp => sym::gt,
GeOp => sym::ge,
};
cx.expr_method_call(span, lft, Ident::new(op_sym, span), vec![rgt])
}

pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<Expr> {
let test_id = Ident::new(sym::cmp, span);
let ordering = cx.path_global(span, cx.std_path(&[sym::cmp, sym::Ordering, sym::Equal]));
Expand Down Expand Up @@ -171,132 +107,13 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
if self_args.len() != 2 {
cx.span_bug(span, "not exactly 2 arguments in `derive(PartialOrd)`")
} else {
some_ordering_collapsed(cx, span, PartialCmpOp, tag_tuple)
let lft = cx.expr_ident(span, tag_tuple[0]);
let rgt = cx.expr_addr_of(span, cx.expr_ident(span, tag_tuple[1]));
cx.expr_method_call(span, lft, Ident::new(sym::partial_cmp, span), vec![rgt])
}
}),
cx,
span,
substr,
)
}

/// Strict inequality.
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(&[sym::cmp, sym::Ordering, Symbol::intern(name)])),
)
};

let par_cmp = |cx: &mut ExtCtxt<'_>, span, self_f: P<Expr>, other_fs: &[P<Expr>], default| {
let other_f = match other_fs {
[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(&[sym::cmp, sym::PartialOrd, sym::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(&[sym::option, sym::Option, sym::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 `partial_cmp`s from the inside
// out (hence foldr) to get lexical ordering, i.e., for op ==
// `ast::lt`
//
// ```
// 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`
//
// ```
// 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.

// `Option::unwrap_or(PartialOrd::partial_cmp(self.fi, other.fi), Ordering::Equal)`
let par_cmp = par_cmp(cx, span, self_f, other_fs, "Equal");

// `Ordering::then_with(Option::unwrap_or(..), ..)`
let then_with_path = cx.expr_path(
cx.path_global(span, cx.std_path(&[sym::cmp, sym::Ordering, sym::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)) => {
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, inclusive) {
(false, false) => GtOp,
(false, true) => GeOp,
(true, false) => LtOp,
(true, true) => LeOp,
};
some_ordering_collapsed(cx, span, op, tag_tuple)
}
}),
cx,
span,
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,
}
}
82 changes: 0 additions & 82 deletions ...rtial_eq/partial_eq.{impl#2}-ge-{closure#0}-{closure#0}.-------.InstrumentCoverage.0.html
View file
Open in desktop

This file was deleted.

Loading