Skip to content

Derive PartialOrd when possible #1002

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.

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

Merged
merged 5 commits into from
Sep 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +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, CanDerivePartialEq, CanDeriveEq};
CanDeriveHash, CanDerivePartialOrd, CanDerivePartialEq,
CanDeriveEq};
use ir::dot;
use ir::enum_ty::{Enum, EnumVariant, EnumVariantValue};
use ir::function::{Abi, Function, FunctionSig};
Expand Down Expand Up @@ -1489,6 +1490,10 @@ impl CodeGenerator for CompInfo {
derives.push("Hash");
}

if item.can_derive_partialord(ctx) {
derives.push("PartialOrd");
}

if item.can_derive_partialeq(ctx) {
derives.push("PartialEq");
}
Expand Down

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/ir/analysis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ mod has_type_param_in_array;
pub use self::has_type_param_in_array::HasTypeParameterInArray;
mod derive_hash;
pub use self::derive_hash::CannotDeriveHash;
mod derive_partial_eq;
pub use self::derive_partial_eq::CannotDerivePartialEq;
mod derive_partial_eq_or_partial_ord;
pub use self::derive_partial_eq_or_partial_ord::CannotDerivePartialEqOrPartialOrd;
mod has_float;
pub use self::has_float::HasFloat;

Expand Down
43 changes: 25 additions & 18 deletions src/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

use super::analysis::{CannotDeriveCopy, CannotDeriveDebug,
CannotDeriveDefault, CannotDeriveHash,
CannotDerivePartialEq, HasTypeParameterInArray,
CannotDerivePartialEqOrPartialOrd, HasTypeParameterInArray,
HasVtableAnalysis, HasDestructorAnalysis, UsedTemplateParameters,
HasFloat, analyze};
use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault,
CanDeriveHash, CanDerivePartialEq, CanDeriveEq};
CanDeriveHash, CanDerivePartialOrd, CanDerivePartialEq,
CanDeriveEq};
use super::int::IntKind;
use super::item::{HasTypeParamInArray, IsOpaque, Item, ItemAncestors,
ItemCanonicalPath, ItemSet};
Expand Down Expand Up @@ -68,17 +69,24 @@ impl CanDeriveHash for ItemId {
}
}

impl CanDerivePartialOrd for ItemId {
fn can_derive_partialord(&self, ctx: &BindgenContext) -> bool {
ctx.options().derive_partialord &&
ctx.lookup_item_id_can_derive_partialeq_or_partialord(*self)
}
}

impl CanDerivePartialEq for ItemId {
fn can_derive_partialeq(&self, ctx: &BindgenContext) -> bool {
ctx.options().derive_partialeq &&
ctx.lookup_item_id_can_derive_partialeq(*self)
ctx.lookup_item_id_can_derive_partialeq_or_partialord(*self)
}
}

impl CanDeriveEq for ItemId {
fn can_derive_eq(&self, ctx: &BindgenContext) -> bool {
ctx.options().derive_eq &&
ctx.lookup_item_id_can_derive_partialeq(*self) &&
ctx.lookup_item_id_can_derive_partialeq_or_partialord(*self) &&
!ctx.lookup_item_id_has_float(&self)
}
}
Expand Down Expand Up @@ -225,7 +233,7 @@ pub struct BindgenContext {
///
/// This is populated when we enter codegen by `compute_can_derive_partialeq`
/// and is always `None` before that and `Some` after.
cannot_derive_partialeq: Option<HashSet<ItemId>>,
cannot_derive_partialeq_or_partialord: Option<HashSet<ItemId>>,

/// The set of (`ItemId's of`) types that has vtable.
///
Expand Down Expand Up @@ -392,7 +400,7 @@ impl BindgenContext {
cannot_derive_copy: None,
cannot_derive_copy_in_array: None,
cannot_derive_hash: None,
cannot_derive_partialeq: None,
cannot_derive_partialeq_or_partialord: None,
have_vtable: None,
have_destructor: None,
has_type_param_in_array: None,
Expand Down Expand Up @@ -947,7 +955,7 @@ impl BindgenContext {
self.compute_has_type_param_in_array();
self.compute_has_float();
self.compute_cannot_derive_hash();
self.compute_cannot_derive_partialeq_or_eq();
self.compute_cannot_derive_partialord_partialeq_or_eq();

let ret = cb(self);
self.in_codegen = false;
Expand Down Expand Up @@ -2123,27 +2131,26 @@ impl BindgenContext {
!self.cannot_derive_hash.as_ref().unwrap().contains(&id)
}

/// Compute whether we can derive PartialEq. This method is also used in calculating
/// whether we can derive Eq
fn compute_cannot_derive_partialeq_or_eq(&mut self) {
let _t = self.timer("compute_cannot_derive_partialeq_or_eq");
assert!(self.cannot_derive_partialeq.is_none());
if self.options.derive_partialeq || self.options.derive_eq {
self.cannot_derive_partialeq = Some(analyze::<CannotDerivePartialEq>(self));
/// Compute whether we can derive PartialOrd, PartialEq or Eq.
fn compute_cannot_derive_partialord_partialeq_or_eq(&mut self) {
let _t = self.timer("compute_cannot_derive_partialord_partialeq_or_eq");
assert!(self.cannot_derive_partialeq_or_partialord.is_none());
if self.options.derive_partialord || self.options.derive_partialeq || self.options.derive_eq {
self.cannot_derive_partialeq_or_partialord = Some(analyze::<CannotDerivePartialEqOrPartialOrd>(self));
}
}

/// Look up whether the item with `id` can
/// derive partialeq or not.
pub fn lookup_item_id_can_derive_partialeq(&self, id: ItemId) -> bool {
/// derive partialeq or partialord.
pub fn lookup_item_id_can_derive_partialeq_or_partialord(&self, id: ItemId) -> bool {
assert!(
self.in_codegen_phase(),
"We only compute can_derive_debug when we enter codegen"
"We only compute can_derive_partialeq_or_partialord when we enter codegen"
);

// Look up the computed value for whether the item with `id` can
// derive partialeq or not.
!self.cannot_derive_partialeq.as_ref().unwrap().contains(&id)
!self.cannot_derive_partialeq_or_partialord.as_ref().unwrap().contains(&id)
}

/// Look up whether the item with `id` can
Expand Down
26 changes: 20 additions & 6 deletions src/ir/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,32 @@ pub trait CanDeriveHash {
fn can_derive_hash(&self, ctx: &BindgenContext) -> bool;
}

/// A trait that encapsulates the logic for whether or not we can derive `PartialEq`
/// A trait that encapsulates the logic for whether or not we can derive `PartialEq`
/// 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 default or not, because of the limit rust has on 32 items as max in the
/// array.
pub trait CanDerivePartialEq {
/// Return `true` if `Default` can be derived for this thing, `false`
/// Return `true` if `PartialEq` can be derived for this thing, `false`
/// otherwise.
fn can_derive_partialeq(&self, ctx: &BindgenContext) -> bool;
}

/// A trait that encapsulates the logic for whether or not we can derive `PartialOrd`
/// 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 default 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`
/// otherwise.
fn can_derive_partialord(&self, ctx: &BindgenContext) -> bool;
}

/// A trait that encapsulates the logic for whether or not we can derive `Eq`
/// for a given thing.
///
Expand All @@ -118,12 +131,13 @@ pub trait CanTriviallyDeriveHash {
fn can_trivially_derive_hash(&self) -> bool;
}

/// A trait that encapsulates the logic for whether or not we can derive `PartialEq`.
/// A trait that encapsulates the logic for whether or not we can derive `PartialEq`
/// or `PartialOrd`.
/// The difference between this trait and the CanDerivePartialEq is that the type
/// implementing this trait cannot use recursion or lookup result from fix point
/// analysis. It's a helper trait for fix point analysis.
pub trait CanTriviallyDerivePartialEq {
/// Return `true` if `PartialEq` can be derived for this thing, `false`
pub trait CanTriviallyDerivePartialEqOrPartialOrd {
/// Return `true` if `PartialEq` or `PartialOrd` can be derived for this thing, `false`
/// otherwise.
fn can_trivially_derive_partialeq(&self) -> bool;
fn can_trivially_derive_partialeq_or_partialord(&self) -> bool;
}
6 changes: 3 additions & 3 deletions src/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use super::ty::TypeKind;
use clang;
use clang_sys::{self, CXCallingConv};
use ir::derive::{CanTriviallyDeriveDebug, CanTriviallyDeriveHash,
CanTriviallyDerivePartialEq};
CanTriviallyDerivePartialEqOrPartialOrd};
use parse::{ClangItemParser, ClangSubItemParser, ParseError, ParseResult};
use quote;
use std::io;
Expand Down Expand Up @@ -556,8 +556,8 @@ impl CanTriviallyDeriveHash for FunctionSig {
}
}

impl CanTriviallyDerivePartialEq for FunctionSig {
fn can_trivially_derive_partialeq(&self) -> bool {
impl CanTriviallyDerivePartialEqOrPartialOrd for FunctionSig {
fn can_trivially_derive_partialeq_or_partialord(&self) -> bool {
if self.argument_types.len() > RUST_DERIVE_FUNPTR_LIMIT {
return false;
}
Expand Down
14 changes: 11 additions & 3 deletions src/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use super::comment;
use super::comp::MethodKind;
use super::context::{BindgenContext, ItemId, PartialType};
use super::derive::{CanDeriveCopy, CanDeriveDebug, CanDeriveDefault,
CanDeriveHash, CanDerivePartialEq, CanDeriveEq};
CanDeriveHash, CanDerivePartialOrd, CanDerivePartialEq,
CanDeriveEq};
use super::dot::DotAttributes;
use super::function::{Function, FunctionKind};
use super::item_kind::ItemKind;
Expand Down Expand Up @@ -330,17 +331,24 @@ impl CanDeriveHash for Item {
}
}

impl CanDerivePartialOrd for Item {
fn can_derive_partialord(&self, ctx: &BindgenContext) -> bool {
ctx.options().derive_partialord &&
ctx.lookup_item_id_can_derive_partialeq_or_partialord(self.id())
}
}

impl CanDerivePartialEq for Item {
fn can_derive_partialeq(&self, ctx: &BindgenContext) -> bool {
ctx.options().derive_partialeq &&
ctx.lookup_item_id_can_derive_partialeq(self.id())
ctx.lookup_item_id_can_derive_partialeq_or_partialord(self.id())
}
}

impl CanDeriveEq for Item {
fn can_derive_eq(&self, ctx: &BindgenContext) -> bool {
ctx.options().derive_eq &&
ctx.lookup_item_id_can_derive_partialeq(self.id()) &&
ctx.lookup_item_id_can_derive_partialeq_or_partialord(self.id()) &&
!ctx.lookup_item_id_has_float(&self.id())
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/ir/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use super::derive::{CanTriviallyDeriveCopy, CanTriviallyDeriveDebug,
CanTriviallyDeriveDefault, CanTriviallyDeriveHash,
CanTriviallyDerivePartialEq};
CanTriviallyDerivePartialEqOrPartialOrd};
use super::ty::{RUST_DERIVE_IN_ARRAY_LIMIT, Type, TypeKind};
use clang;
use std::{cmp, mem};
Expand Down Expand Up @@ -138,8 +138,8 @@ impl CanTriviallyDeriveHash for Opaque {
}
}

impl CanTriviallyDerivePartialEq for Opaque {
fn can_trivially_derive_partialeq(&self) -> bool {
impl CanTriviallyDerivePartialEqOrPartialOrd for Opaque {
fn can_trivially_derive_partialeq_or_partialord(&self) -> bool {
self.array_size().map_or(false, |size| {
size <= RUST_DERIVE_IN_ARRAY_LIMIT
})
Expand Down
15 changes: 15 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,10 @@ impl Builder {
output_vector.push("--with-derive-hash".into());
}

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

if self.options.derive_partialeq {
output_vector.push("--with-derive-partialeq".into());
}
Expand Down Expand Up @@ -814,6 +818,12 @@ impl Builder {
self
}

/// Set whether `PartialOrd` should be derived by default.
pub fn derive_partialord(mut self, doit: bool) -> Self {
self.options.derive_partialord = doit;
self
}

/// Set whether `PartialEq` should be derived by default.
/// If we don't compute partialeq, we also cannot compute
/// eq. Set the derive_eq to `false` when doit is `false`.
Expand Down Expand Up @@ -1176,6 +1186,10 @@ struct BindgenOptions {
/// and types.
derive_hash: bool,

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

/// True if we should derive PartialEq trait implementations for C/C++ structures
/// and types.
derive_partialeq: bool,
Expand Down Expand Up @@ -1325,6 +1339,7 @@ impl Default for BindgenOptions {
impl_debug: false,
derive_default: false,
derive_hash: false,
derive_partialord: false,
derive_partialeq: false,
derive_eq: false,
enable_cxx_namespaces: false,
Expand Down
7 changes: 7 additions & 0 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ where
Arg::with_name("with-derive-partialeq")
.long("with-derive-partialeq")
.help("Derive partialeq on any type."),
Arg::with_name("with-derive-partialord")
.long("with-derive-partialord")
.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"),
Expand Down Expand Up @@ -340,6 +343,10 @@ where
builder = builder.derive_partialeq(true);
}

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

if matches.is_present("with-derive-eq") {
builder = builder.derive_eq(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@



/// A struct containing a struct containing a float that cannot derive hash/eq but can derive partial eq.
/// A struct containing a struct containing a float that cannot derive hash/eq but can derive partialeq and partialord
#[repr(C)]
#[derive(Debug, Default, Copy, PartialEq)]
#[derive(Debug, Default, Copy, PartialOrd, PartialEq)]
pub struct foo {
pub bar: foo__bindgen_ty_1,
}
#[repr(C)]
#[derive(Debug, Default, Copy, PartialEq)]
#[derive(Debug, Default, Copy, PartialOrd, PartialEq)]
pub struct foo__bindgen_ty_1 {
pub a: f32,
pub b: f32,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@



/// A struct containing an array of floats that cannot derive hash/eq but can derive partialeq.
/// A struct containing an array of floats that cannot derive hash/eq but can derive partialeq and partialord
#[repr(C)]
#[derive(Debug, Default, Copy, PartialEq)]
#[derive(Debug, Default, Copy, PartialOrd, PartialEq)]
pub struct foo {
pub bar: [f32; 3usize],
}
Expand Down
Loading