Skip to content

Commit

Permalink
Auto merge of #1003 - pepyakin:derive-ord-when-possible, r=fitzgen
Browse files Browse the repository at this point in the history
Derive ord when possible

Fixes #884

r? @fitzgen
  • Loading branch information
bors-servo authored Sep 19, 2017
2 parents 1906a26 + 27dad0b commit de0180e
Show file tree
Hide file tree
Showing 17 changed files with 100 additions and 35 deletions.
8 changes: 6 additions & 2 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use ir::comp::{Base, Bitfield, BitfieldUnit, CompInfo, CompKind, Field,
FieldData, FieldMethods, Method, MethodKind};
use ir::context::{BindgenContext, ItemId};
use ir::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault,
CanDeriveHash, CanDerivePartialOrd, CanDerivePartialEq,
CanDeriveEq};
CanDeriveHash, CanDerivePartialOrd, CanDeriveOrd,
CanDerivePartialEq, CanDeriveEq};
use ir::dot;
use ir::enum_ty::{Enum, EnumVariant, EnumVariantValue};
use ir::function::{Abi, Function, FunctionSig};
Expand Down Expand Up @@ -1494,6 +1494,10 @@ impl CodeGenerator for CompInfo {
derives.push("PartialOrd");
}

if item.can_derive_ord(ctx) {
derives.push("Ord");
}

if item.can_derive_partialeq(ctx) {
derives.push("PartialEq");
}
Expand Down
14 changes: 11 additions & 3 deletions src/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use super::analysis::{CannotDeriveCopy, CannotDeriveDebug,
HasVtableAnalysis, HasDestructorAnalysis, UsedTemplateParameters,
HasFloat, analyze};
use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault,
CanDeriveHash, CanDerivePartialOrd, CanDerivePartialEq,
CanDeriveEq};
CanDeriveHash, CanDerivePartialOrd, CanDeriveOrd,
CanDerivePartialEq, CanDeriveEq};
use super::int::IntKind;
use super::item::{HasTypeParamInArray, IsOpaque, Item, ItemAncestors,
ItemCanonicalPath, ItemSet};
Expand Down Expand Up @@ -91,6 +91,14 @@ impl CanDeriveEq for ItemId {
}
}

impl CanDeriveOrd for ItemId {
fn can_derive_ord(&self, ctx: &BindgenContext) -> bool {
ctx.options().derive_ord &&
ctx.lookup_item_id_can_derive_partialeq_or_partialord(*self) &&
!ctx.lookup_item_id_has_float(&self)
}
}

/// A key used to index a resolved type, so we only process it once.
///
/// This is almost always a USR string (an unique identifier generated by
Expand Down Expand Up @@ -2191,7 +2199,7 @@ impl BindgenContext {
fn compute_has_float(&mut self) {
let _t = self.timer("compute_has_float");
assert!(self.has_float.is_none());
if self.options.derive_eq {
if self.options.derive_eq || self.options.derive_ord {
self.has_float = Some(analyze::<HasFloat>(self));
}
}
Expand Down
15 changes: 14 additions & 1 deletion src/ir/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub trait CanDerivePartialEq {
///
/// This should ideally be a no-op that just returns `true`, but instead needs
/// to be a recursive method that checks whether all the proper members can
/// derive default or not, because of the limit rust has on 32 items as max in the
/// derive partial ord or not, because of the limit rust has on 32 items as max in the
/// array.
pub trait CanDerivePartialOrd {
/// Return `true` if `PartialOrd` can be derived for this thing, `false`
Expand All @@ -121,6 +121,19 @@ pub trait CanDeriveEq {
-> bool;
}

/// A trait that encapsulates the logic for whether or not we can derive `Ord`
/// for a given thing.
///
/// This should ideally be a no-op that just returns `true`, but instead needs
/// to be a recursive method that checks whether all the proper members can
/// derive ord or not, because of the limit rust has on 32 items as max in the
/// array.
pub trait CanDeriveOrd {
/// Return `true` if `Ord` can be derived for this thing, `false`
/// otherwise.
fn can_derive_ord(&self, ctx: &BindgenContext) -> bool;
}

/// A trait that encapsulates the logic for whether or not we can derive `Hash`.
/// The difference between this trait and the CanDeriveHash is that the type
/// implementing this trait cannot use recursion or lookup result from fix point
Expand Down
12 changes: 10 additions & 2 deletions src/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use super::comment;
use super::comp::MethodKind;
use super::context::{BindgenContext, ItemId, PartialType};
use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault,
CanDeriveHash, CanDerivePartialOrd, CanDerivePartialEq,
CanDeriveEq};
CanDeriveHash, CanDerivePartialOrd, CanDeriveOrd,
CanDerivePartialEq, CanDeriveEq};
use super::dot::DotAttributes;
use super::function::{Function, FunctionKind};
use super::item_kind::ItemKind;
Expand Down Expand Up @@ -353,6 +353,14 @@ impl CanDeriveEq for Item {
}
}

impl CanDeriveOrd for Item {
fn can_derive_ord(&self, ctx: &BindgenContext) -> bool {
ctx.options().derive_ord &&
ctx.lookup_item_id_can_derive_partialeq_or_partialord(self.id()) &&
!ctx.lookup_item_id_has_float(&self.id())
}
}

/// An item is the base of the bindgen representation, it can be either a
/// module, a type, a function, or a variable (see `ItemKind` for more
/// information).
Expand Down
23 changes: 23 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,10 @@ impl Builder {
output_vector.push("--with-derive-partialord".into());
}

if self.options.derive_ord {
output_vector.push("--with-derive-ord".into());
}

if self.options.derive_partialeq {
output_vector.push("--with-derive-partialeq".into());
}
Expand Down Expand Up @@ -819,7 +823,21 @@ impl Builder {
}

/// Set whether `PartialOrd` should be derived by default.
/// If we don't compute partialord, we also cannot compute
/// ord. Set the derive_ord to `false` when doit is `false`.
pub fn derive_partialord(mut self, doit: bool) -> Self {
self.options.derive_partialord = doit;
if !doit {
self.options.derive_ord = false;
}
self
}

/// Set whether `Ord` should be derived by default.
/// We can't compute `Ord` without computing `PartialOrd`,
/// so we set the same option to derive_partialord.
pub fn derive_ord(mut self, doit: bool) -> Self {
self.options.derive_ord = doit;
self.options.derive_partialord = doit;
self
}
Expand Down Expand Up @@ -1190,6 +1208,10 @@ struct BindgenOptions {
/// and types.
derive_partialord: bool,

/// True if we should derive Ord trait implementations for C/C++ structures
/// and types.
derive_ord: bool,

/// True if we should derive PartialEq trait implementations for C/C++ structures
/// and types.
derive_partialeq: bool,
Expand Down Expand Up @@ -1340,6 +1362,7 @@ impl Default for BindgenOptions {
derive_default: false,
derive_hash: false,
derive_partialord: false,
derive_ord: false,
derive_partialeq: false,
derive_eq: false,
enable_cxx_namespaces: false,
Expand Down
11 changes: 10 additions & 1 deletion src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,12 @@ where
.help("Derive partialord on any type."),
Arg::with_name("with-derive-eq")
.long("with-derive-eq")
.help("Derive eq on any type. Enable this option also enables --with-derive-partialeq"),
.help("Derive eq on any type. Enable this option also \
enables --with-derive-partialeq"),
Arg::with_name("with-derive-ord")
.long("with-derive-ord")
.help("Derive ord on any type. Enable this option also \
enables --with-derive-partialord"),
Arg::with_name("no-doc-comments")
.long("no-doc-comments")
.help("Avoid including doc comments in the output, see: \
Expand Down Expand Up @@ -351,6 +356,10 @@ where
builder = builder.derive_eq(true);
}

if matches.is_present("with-derive-ord") {
builder = builder.derive_ord(true);
}

if matches.is_present("no-derive-default") {
builder = builder.derive_default(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@



/// A struct containing a struct containing a float that cannot derive hash/eq but can derive partialeq and partialord
/// A struct containing a struct containing a float that cannot derive Hash/Eq/Ord but can derive PartialEq/PartialOrd
#[repr(C)]
#[derive(Debug, Default, Copy, PartialOrd, PartialEq)]
pub struct foo {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@



/// A struct containing an array of floats that cannot derive hash/eq but can derive partialeq and partialord
/// A struct containing an array of floats that cannot derive Hash/Eq/Ord but can derive PartialEq/PartialOrd
#[repr(C)]
#[derive(Debug, Default, Copy, PartialOrd, PartialEq)]
pub struct foo {
Expand Down
10 changes: 5 additions & 5 deletions tests/expectations/tests/derive-hash-struct-with-pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@



/// Pointers can derive Hash/PartialOrd/PartialEq/Eq
/// Pointers can derive Hash/PartialOrd/Ord/PartialEq/Eq
#[repr(C)]
#[derive(Debug, Copy, Hash, PartialOrd, PartialEq, Eq)]
#[derive(Debug, Copy, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub struct ConstPtrMutObj {
pub bar: *const ::std::os::raw::c_int,
}
Expand Down Expand Up @@ -45,7 +45,7 @@ impl Default for ConstPtrMutObj {
}
}
#[repr(C)]
#[derive(Debug, Copy, Hash, PartialOrd, PartialEq, Eq)]
#[derive(Debug, Copy, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub struct MutPtrMutObj {
pub bar: *mut ::std::os::raw::c_int,
}
Expand Down Expand Up @@ -83,7 +83,7 @@ impl Default for MutPtrMutObj {
}
}
#[repr(C)]
#[derive(Debug, Copy, Hash, PartialOrd, PartialEq, Eq)]
#[derive(Debug, Copy, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub struct MutPtrConstObj {
pub bar: *const ::std::os::raw::c_int,
}
Expand Down Expand Up @@ -121,7 +121,7 @@ impl Default for MutPtrConstObj {
}
}
#[repr(C)]
#[derive(Debug, Copy, Hash, PartialOrd, PartialEq, Eq)]
#[derive(Debug, Copy, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub struct ConstPtrConstObj {
pub bar: *const ::std::os::raw::c_int,
}
Expand Down
2 changes: 1 addition & 1 deletion tests/expectations/tests/derive-hash-template-def-float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@



/// Template definition containing a float, which cannot derive hash/eq but can derive partialeq and partialord.
/// Template definition containing a float, which cannot derive Hash/Eq/Ord but can derive PartialEq/PartialOrd.
#[repr(C)]
#[derive(Debug, Copy, Clone, PartialOrd, PartialEq)]
pub struct foo<T> {
Expand Down
10 changes: 5 additions & 5 deletions tests/expectations/tests/derive-hash-template-inst-float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@



/// Template definition that doesn't contain float can derive hash/partialord/partialeq/eq
/// Template definition that doesn't contain float can derive Hash/PartialOrd/Ord/PartialEq/Eq
#[repr(C)]
#[derive(Debug, Copy, Clone, Hash, PartialOrd, PartialEq, Eq)]
#[derive(Debug, Copy, Clone, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub struct foo<T> {
pub data: T,
pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<T>>,
Expand All @@ -17,9 +17,9 @@ impl<T> Default for foo<T> {
unsafe { ::std::mem::zeroed() }
}
}
/// Can derive hash/partialeq/eq when instantiated with int
/// Can derive Hash/PartialOrd/Ord/PartialEq/Eq when instantiated with int
#[repr(C)]
#[derive(Debug, Copy, Hash, PartialOrd, PartialEq, Eq)]
#[derive(Debug, Copy, Hash, PartialOrd, Ord, PartialEq, Eq)]
pub struct IntStr {
pub a: foo<::std::os::raw::c_int>,
}
Expand Down Expand Up @@ -56,7 +56,7 @@ impl Default for IntStr {
unsafe { ::std::mem::zeroed() }
}
}
/// Cannot derive hash/eq when instantiated with float but can derive partialeq
/// Cannot derive Hash/Eq/Ord when instantiated with float but can derive PartialEq/PartialOrd
#[repr(C)]
#[derive(Debug, Copy, PartialOrd, PartialEq)]
pub struct FloatStr {
Expand Down
2 changes: 1 addition & 1 deletion tests/headers/derive-hash-blacklisting.hpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-partialeq --with-derive-eq --whitelist-type 'Whitelisted.*' --blacklist-type Blacklisted --raw-line "#[repr(C)] #[derive(Debug, Hash, Copy, Clone, PartialEq, Eq)] pub struct Blacklisted<T> {t: T, pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<T>> }"
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-ord --with-derive-partialeq --with-derive-eq --whitelist-type 'Whitelisted.*' --blacklist-type Blacklisted --raw-line "#[repr(C)] #[derive(Debug, Hash, Copy, Clone, PartialEq, Eq)] pub struct Blacklisted<T> {t: T, pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<T>> }"
//
template <class T>
struct Blacklisted {
Expand Down
4 changes: 2 additions & 2 deletions tests/headers/derive-hash-struct-with-anon-struct-float.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-partialeq --with-derive-eq
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-ord --with-derive-partialeq --with-derive-eq
//
/// A struct containing a struct containing a float that cannot derive hash/eq but can derive partialeq and partialord
/// A struct containing a struct containing a float that cannot derive Hash/Eq/Ord but can derive PartialEq/PartialOrd
struct foo {
struct {
float a;
Expand Down
4 changes: 2 additions & 2 deletions tests/headers/derive-hash-struct-with-float-array.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-partialeq --with-derive-eq
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-ord --with-derive-partialeq --with-derive-eq
//
/// A struct containing an array of floats that cannot derive hash/eq but can derive partialeq and partialord
/// A struct containing an array of floats that cannot derive Hash/Eq/Ord but can derive PartialEq/PartialOrd
struct foo {
float bar[3];
};
4 changes: 2 additions & 2 deletions tests/headers/derive-hash-struct-with-pointer.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-partialeq --with-derive-eq
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-ord --with-derive-partialeq --with-derive-eq
//
/// Pointers can derive Hash/PartialOrd/PartialEq/Eq
/// Pointers can derive Hash/PartialOrd/Ord/PartialEq/Eq
struct ConstPtrMutObj {
int* const bar;
};
Expand Down
4 changes: 2 additions & 2 deletions tests/headers/derive-hash-template-def-float.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-partialeq --with-derive-eq
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-ord --with-derive-partialeq --with-derive-eq
//
/// Template definition containing a float, which cannot derive hash/eq but can derive partialeq and partialord.
/// Template definition containing a float, which cannot derive Hash/Eq/Ord but can derive PartialEq/PartialOrd.
template <typename T>
struct foo {
T data;
Expand Down
8 changes: 4 additions & 4 deletions tests/headers/derive-hash-template-inst-float.hpp
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-partialeq --with-derive-eq
// bindgen-flags: --with-derive-hash --with-derive-partialord --with-derive-ord --with-derive-partialeq --with-derive-eq
//
/// Template definition that doesn't contain float can derive hash/partialord/partialeq/eq
/// Template definition that doesn't contain float can derive Hash/PartialOrd/Ord/PartialEq/Eq
template <typename T>
struct foo {
T data;
};

/// Can derive hash/partialeq/eq when instantiated with int
/// Can derive Hash/PartialOrd/Ord/PartialEq/Eq when instantiated with int
struct IntStr {
foo<int> a;
};

/// Cannot derive hash/eq when instantiated with float but can derive partialeq
/// Cannot derive Hash/Eq/Ord when instantiated with float but can derive PartialEq/PartialOrd
struct FloatStr {
foo<float> a;
};

0 comments on commit de0180e

Please sign in to comment.