Auto merge of #1002 - pepyakin:derive-partialord-when-possible, r=fit…

…zgen

Derive PartialOrd when possible

Fixes #882

r? @fitzgen

src/codegen/mod.rs

@@ -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};
@@ -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");
         }

src/ir/analysis/mod.rs

@@ -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;
 

src/ir/context.rs

@@ -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};
@@ -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)
     }
 }
@@ -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.
     ///
@@ -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,
@@ -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;
@@ -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

src/ir/derive.rs

@@ -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.
 ///
@@ -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;
 }

src/ir/function.rs

@@ -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;
@@ -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;
         }

src/ir/item.rs

@@ -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;
@@ -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())
     }
 }

src/ir/layout.rs

@@ -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};
@@ -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
         })

src/lib.rs

@@ -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());
         }
@@ -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`.
@@ -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,
@@ -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,

src/options.rs

@@ -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"),
@@ -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);
     }

tests/expectations/tests/derive-hash-struct-with-anon-struct-float.rs

@@ -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,

tests/expectations/tests/derive-hash-struct-with-float-array.rs

@@ -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],
 }