Define extra assertion macros

fitzgen · fitzgen · commit 2e14b19bc0fd · 2017-04-06T15:24:36.000-07:00
This commit defines a new set of assertion macros that are only checked in
testing/CI when the `testing_only_extra_assertions` feature is enabled. This
makes it so that *users* of bindgen that happen to be making a debug build don't
enable all these extra and expensive assertions.

Additionally, this removes the `testing_only_assert_no_dangling_items` feature,
and runs the assertions that were previously gated on that feature when the new
`testing_only_extra_assertions` feature is enabled.
diff --git a/Cargo.toml b/Cargo.toml
@@ -70,6 +70,6 @@ static = []
 
 # These features only exist for CI testing -- don't use them if you're not hacking
 # on bindgen!
-testing_only_assert_no_dangling_items = []
 testing_only_docs = []
+testing_only_extra_assertions = []
 testing_only_llvm_stable = []
diff --git a/ci/test.sh b/ci/test.sh
@@ -6,10 +6,13 @@ cd "$(dirname "$0")/.."
 # Regenerate the test headers' bindings in debug and release modes, and assert
 # that we always get the expected generated bindings.
 
-cargo test --features "$BINDGEN_FEATURES testing_only_assert_no_dangling_items"
+cargo test --features "$BINDGEN_FEATURES"
+./ci/assert-no-diff.sh
+
+cargo test --features "$BINDGEN_FEATURES testing_only_extra_assertions"
 ./ci/assert-no-diff.sh
 
-cargo test --release --features "$BINDGEN_FEATURES testing_only_assert_no_dangling_items"
+cargo test --release --features "$BINDGEN_FEATURES testing_only_extra_assertions"
 ./ci/assert-no-diff.sh
 
 # Now test the expectations' size and alignment tests.
diff --git a/src/clang.rs b/src/clang.rs
@@ -152,7 +152,7 @@ impl Cursor {
                 if n >= 0 {
                     Some(n as u32)
                 } else {
-                    debug_assert_eq!(n, -1);
+                    extra_assert_eq!(n, -1);
                     None
                 }
             })
@@ -813,7 +813,7 @@ impl Type {
         if n >= 0 {
             Some(n as u32)
         } else {
-            debug_assert_eq!(n, -1);
+            extra_assert_eq!(n, -1);
             None
         }
     }
@@ -842,7 +842,7 @@ impl Type {
                 let ret = Type {
                     x: unsafe { clang_getPointeeType(self.x) },
                 };
-                debug_assert!(ret.is_valid());
+                extra_assert!(ret.is_valid());
                 Some(ret)
             }
             _ => None,
diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs
@@ -1202,9 +1202,9 @@ impl CodeGenerator for CompInfo {
         let mut methods = vec![];
         let mut anonymous_field_count = 0;
         for field in struct_fields {
-            debug_assert_eq!(current_bitfield_width.is_some(),
+            extra_assert_eq!(current_bitfield_width.is_some(),
                              current_bitfield_layout.is_some());
-            debug_assert_eq!(current_bitfield_width.is_some(),
+            extra_assert_eq!(current_bitfield_width.is_some(),
                              !current_bitfield_fields.is_empty());
 
             let field_ty = ctx.resolve_type(field.ty());
@@ -1226,7 +1226,7 @@ impl CodeGenerator for CompInfo {
 
             // Flush the current bitfield.
             if current_bitfield_width.is_some() {
-                debug_assert!(!current_bitfield_fields.is_empty());
+                extra_assert!(!current_bitfield_fields.is_empty());
                 let bitfield_fields =
                     mem::replace(&mut current_bitfield_fields, vec![]);
                 let bitfield_layout = Bitfield::new(&mut bitfield_count,
@@ -1237,7 +1237,7 @@ impl CodeGenerator for CompInfo {
                 current_bitfield_width = None;
                 current_bitfield_layout = None;
             }
-            debug_assert!(current_bitfield_fields.is_empty());
+            extra_assert!(current_bitfield_fields.is_empty());
 
             if let Some(width) = field.bitfield() {
                 let layout = field_ty.layout(ctx)
@@ -1381,15 +1381,15 @@ impl CodeGenerator for CompInfo {
         // FIXME: Reduce duplication with the loop above.
         // FIXME: May need to pass current_bitfield_layout too.
         if current_bitfield_width.is_some() {
-            debug_assert!(!current_bitfield_fields.is_empty());
+            extra_assert!(!current_bitfield_fields.is_empty());
             let bitfield_fields = mem::replace(&mut current_bitfield_fields,
                                                vec![]);
             let bitfield_layout = Bitfield::new(&mut bitfield_count,
                                                 bitfield_fields)
                 .codegen_fields(ctx, self, &mut fields, &mut methods);
             struct_layout.saw_bitfield_batch(bitfield_layout);
         }
-        debug_assert!(current_bitfield_fields.is_empty());
+        extra_assert!(current_bitfield_fields.is_empty());
 
         if is_union && !ctx.options().unstable_rust {
             let layout = layout.expect("Unable to get layout information?");
@@ -2642,7 +2642,7 @@ impl TryToRustTy for TemplateInstantiation {
                 // This can happen if we generated an opaque type for a partial
                 // template specialization, and we've hit an instantiation of
                 // that partial specialization.
-                debug_assert!(ctx.resolve_type_through_type_refs(decl)
+                extra_assert!(ctx.resolve_type_through_type_refs(decl)
                                   .is_opaque());
                 return Err(error::Error::InstantiationOfOpaqueType);
             }
diff --git a/src/extra_assertions.rs b/src/extra_assertions.rs
@@ -0,0 +1,30 @@
+//! Macros for defining extra assertions that should only be checked in testing
+//! and/or CI when the `testing_only_extra_assertions` feature is enabled.
+
+#[macro_export]
+macro_rules! extra_assert {
+    ( $cond:expr ) => {
+        if cfg!(feature = "testing_only_extra_assertions") {
+            assert!($cond);
+        }
+    };
+    ( $cond:expr , $( $arg:tt )+ ) => {
+        if cfg!(feature = "testing_only_extra_assertions") {
+            assert!($cond, $( $arg )* )
+        }
+    };
+}
+
+#[macro_export]
+macro_rules! extra_assert_eq {
+    ( $lhs:expr , $rhs:expr ) => {
+        if cfg!(feature = "testing_only_extra_assertions") {
+            assert_eq!($lhs, $rhs);
+        }
+    };
+    ( $lhs:expr , $rhs:expr , $( $arg:tt )+ ) => {
+        if cfg!(feature = "testing_only_extra_assertions") {
+            assert!($lhs, $rhs, $( $arg )* );
+        }
+    };
+}
diff --git a/src/ir/comp.rs b/src/ir/comp.rs
@@ -645,7 +645,7 @@ impl CompInfo {
                 CXCursor_CXXMethod => {
                     let is_virtual = cur.method_is_virtual();
                     let is_static = cur.method_is_static();
-                    debug_assert!(!(is_static && is_virtual), "How?");
+                    extra_assert!(!(is_static && is_virtual), "How?");
 
                     ci.has_destructor |= cur.kind() == CXCursor_Destructor;
                     ci.has_vtable |= is_virtual;
diff --git a/src/ir/context.rs b/src/ir/context.rs
@@ -250,7 +250,7 @@ impl<'ctx> BindgenContext<'ctx> {
                item,
                declaration,
                location);
-        debug_assert!(declaration.is_some() || !item.kind().is_type() ||
+        extra_assert!(declaration.is_some() || !item.kind().is_type() ||
                       item.kind().expect_type().is_builtin_or_named() ||
                       item.kind().expect_type().is_opaque(),
                       "Adding a type without declaration?");
@@ -310,7 +310,7 @@ impl<'ctx> BindgenContext<'ctx> {
             };
 
             let old = self.types.insert(key, id);
-            debug_assert_eq!(old, None);
+            extra_assert_eq!(old, None);
         }
     }
 
@@ -397,7 +397,7 @@ impl<'ctx> BindgenContext<'ctx> {
     fn collect_typerefs
         (&mut self)
          -> Vec<(ItemId, clang::Type, clang::Cursor, Option<ItemId>)> {
-        debug_assert!(!self.collected_typerefs);
+        extra_assert!(!self.collected_typerefs);
         self.collected_typerefs = true;
         let mut typerefs = vec![];
         for (id, ref mut item) in &mut self.items {
@@ -439,7 +439,7 @@ impl<'ctx> BindgenContext<'ctx> {
             // Something in the STL is trolling me. I don't need this assertion
             // right now, but worth investigating properly once this lands.
             //
-            // debug_assert!(self.items.get(&resolved).is_some(), "How?");
+            // extra_assert!(self.items.get(&resolved).is_some(), "How?");
         }
     }
 
@@ -584,12 +584,11 @@ impl<'ctx> BindgenContext<'ctx> {
         ret
     }
 
-    /// When the `testing_only_assert_no_dangling_items` feature is enabled,
-    /// this function walks the IR graph and asserts that we do not have any
-    /// edges referencing an ItemId for which we do not have an associated IR
-    /// item.
+    /// When the `testing_only_extra_assertions` feature is enabled, this
+    /// function walks the IR graph and asserts that we do not have any edges
+    /// referencing an ItemId for which we do not have an associated IR item.
     fn assert_no_dangling_references(&self) {
-        if cfg!(feature = "testing_only_assert_no_dangling_items") {
+        if cfg!(feature = "testing_only_extra_assertions") {
             for _ in self.assert_no_dangling_item_traversal() {
                 // The iterator's next method does the asserting for us.
             }
@@ -657,7 +656,7 @@ impl<'ctx> BindgenContext<'ctx> {
     // -> builtin type ItemId would be the best to improve that.
     fn add_builtin_item(&mut self, item: Item) {
         debug!("add_builtin_item: item = {:?}", item);
-        debug_assert!(item.kind().is_type());
+        extra_assert!(item.kind().is_type());
         let id = item.id();
         let old_item = self.items.insert(id, item);
         assert!(old_item.is_none(), "Inserted type twice?");
@@ -919,7 +918,7 @@ impl<'ctx> BindgenContext<'ctx> {
                         debug!("instantiate_template: inserting nested \
                                 instantiation item: {:?}",
                                sub_item);
-                        debug_assert!(sub_id == sub_item.id());
+                        extra_assert!(sub_id == sub_item.id());
                         self.items.insert(sub_id, sub_item);
                         args.push(sub_id);
                     }
@@ -964,7 +963,7 @@ impl<'ctx> BindgenContext<'ctx> {
 
         // Bypass all the validations in add_item explicitly.
         debug!("instantiate_template: inserting item: {:?}", item);
-        debug_assert!(with_id == item.id());
+        extra_assert!(with_id == item.id());
         self.items.insert(with_id, item);
         Some(with_id)
     }
@@ -1152,7 +1151,7 @@ impl<'ctx> BindgenContext<'ctx> {
 
     /// Get the currently parsed macros.
     pub fn parsed_macros(&self) -> &HashMap<Vec<u8>, cexpr::expr::EvalResult> {
-        debug_assert!(!self.in_codegen_phase());
+        extra_assert!(!self.in_codegen_phase());
         &self.parsed_macros
     }
 
@@ -1194,7 +1193,7 @@ impl<'ctx> BindgenContext<'ctx> {
     /// Is the item with the given `name` hidden? Or is the item with the given
     /// `name` and `id` replaced by another type, and effectively hidden?
     pub fn hidden_by_name(&self, path: &[String], id: ItemId) -> bool {
-        debug_assert!(self.in_codegen_phase(),
+        extra_assert!(self.in_codegen_phase(),
                       "You're not supposed to call this yet");
         self.options.hidden_types.matches(&path[1..].join("::")) ||
         self.is_replaced_type(path, id)
@@ -1211,7 +1210,7 @@ impl<'ctx> BindgenContext<'ctx> {
 
     /// Is the type with the given `name` marked as opaque?
     pub fn opaque_by_name(&self, path: &[String]) -> bool {
-        debug_assert!(self.in_codegen_phase(),
+        extra_assert!(self.in_codegen_phase(),
                       "You're not supposed to call this yet");
         self.options.opaque_types.matches(&path[1..].join("::"))
     }
@@ -1298,7 +1297,7 @@ impl<'ctx> BindgenContext<'ctx> {
     pub fn with_module<F>(&mut self, module_id: ItemId, cb: F)
         where F: FnOnce(&mut Self),
     {
-        debug_assert!(self.resolve_item(module_id).kind().is_module(), "Wat");
+        extra_assert!(self.resolve_item(module_id).kind().is_module(), "Wat");
 
         let previous_id = self.current_module;
         self.current_module = module_id;
diff --git a/src/ir/item.rs b/src/ir/item.rs
@@ -70,7 +70,7 @@ pub trait ItemAncestors {
 }
 
 cfg_if! {
-    if #[cfg(debug_assertions)] {
+    if #[cfg(testing_only_extra_assertions)] {
         type DebugOnlyItemSet = ItemSet;
     } else {
         struct DebugOnlyItemSet;
@@ -123,7 +123,7 @@ impl<'a, 'b> Iterator for ItemAncestorsIter<'a, 'b>
         } else {
             self.item = item.parent_id();
 
-            debug_assert!(!self.seen.contains(&item.id()));
+            extra_assert!(!self.seen.contains(&item.id()));
             self.seen.insert(item.id());
 
             Some(item.id())
@@ -163,7 +163,7 @@ impl AsNamed for ItemKind {
 // Pure convenience
 impl ItemCanonicalName for ItemId {
     fn canonical_name(&self, ctx: &BindgenContext) -> String {
-        debug_assert!(ctx.in_codegen_phase(),
+        extra_assert!(ctx.in_codegen_phase(),
                       "You're not supposed to call this yet");
         ctx.resolve_item(*self).canonical_name(ctx)
     }
@@ -173,13 +173,13 @@ impl ItemCanonicalPath for ItemId {
     fn namespace_aware_canonical_path(&self,
                                       ctx: &BindgenContext)
                                       -> Vec<String> {
-        debug_assert!(ctx.in_codegen_phase(),
+        extra_assert!(ctx.in_codegen_phase(),
                       "You're not supposed to call this yet");
         ctx.resolve_item(*self).namespace_aware_canonical_path(ctx)
     }
 
     fn canonical_path(&self, ctx: &BindgenContext) -> Vec<String> {
-        debug_assert!(ctx.in_codegen_phase(),
+        extra_assert!(ctx.in_codegen_phase(),
                       "You're not supposed to call this yet");
         ctx.resolve_item(*self).canonical_path(ctx)
     }
@@ -418,7 +418,7 @@ impl Item {
                parent_id: ItemId,
                kind: ItemKind)
                -> Self {
-        debug_assert!(id != parent_id || kind.is_module());
+        extra_assert!(id != parent_id || kind.is_module());
         Item {
             id: id,
             local_id: Cell::new(None),
@@ -573,15 +573,15 @@ impl Item {
     ///
     /// This may be due to either annotations or to other kind of configuration.
     pub fn is_hidden(&self, ctx: &BindgenContext) -> bool {
-        debug_assert!(ctx.in_codegen_phase(),
+        extra_assert!(ctx.in_codegen_phase(),
                       "You're not supposed to call this yet");
         self.annotations.hide() ||
         ctx.hidden_by_name(&self.canonical_path(ctx), self.id)
     }
 
     /// Is this item opaque?
     pub fn is_opaque(&self, ctx: &BindgenContext) -> bool {
-        debug_assert!(ctx.in_codegen_phase(),
+        extra_assert!(ctx.in_codegen_phase(),
                       "You're not supposed to call this yet");
         self.annotations.opaque() ||
         self.as_type().map_or(false, |ty| ty.is_opaque()) ||
@@ -614,7 +614,7 @@ impl Item {
         let mut item = self;
 
         loop {
-            debug_assert!(!targets_seen.contains(&item.id()));
+            extra_assert!(!targets_seen.contains(&item.id()));
             targets_seen.insert(item.id());
 
             if self.annotations().use_instead_of().is_some() {
@@ -1402,7 +1402,7 @@ impl ClangItemParser for Item {
 
 impl ItemCanonicalName for Item {
     fn canonical_name(&self, ctx: &BindgenContext) -> String {
-        debug_assert!(ctx.in_codegen_phase(),
+        extra_assert!(ctx.in_codegen_phase(),
                       "You're not supposed to call this yet");
         if self.canonical_name_cache.borrow().is_none() {
             let in_namespace = ctx.options().enable_cxx_namespaces ||
diff --git a/src/ir/named.rs b/src/ir/named.rs
@@ -371,7 +371,7 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> {
     fn constrain(&mut self, id: ItemId) -> bool {
         // Invariant: all hash map entries' values are `Some` upon entering and
         // exiting this method.
-        debug_assert!(self.used.values().all(|v| v.is_some()));
+        extra_assert!(self.used.values().all(|v| v.is_some()));
 
         // Take the set for this id out of the hash map while we mutate it based
         // on other hash map entries. We *must* put it back into the hash map at
@@ -437,7 +437,7 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> {
 
         // Put the set back in the hash map and restore our invariant.
         self.used.insert(id, Some(used_by_this_id));
-        debug_assert!(self.used.values().all(|v| v.is_some()));
+        extra_assert!(self.used.values().all(|v| v.is_some()));
 
         new_len != original_len
     }
diff --git a/src/ir/traversal.rs b/src/ir/traversal.rs
@@ -439,9 +439,9 @@ impl<'ctx, 'gen, Storage, Queue, Predicate> Iterator
         };
 
         let newly_discovered = self.seen.add(None, id);
-        debug_assert!(!newly_discovered,
+        extra_assert!(!newly_discovered,
                       "should have already seen anything we get out of our queue");
-        debug_assert!(self.ctx.resolve_item_fallible(id).is_some(),
+        extra_assert!(self.ctx.resolve_item_fallible(id).is_some(),
                       "should only get IDs of actual items in our context during traversal");
 
         self.currently_traversing = Some(id);
diff --git a/src/ir/ty.rs b/src/ir/ty.rs
@@ -894,7 +894,7 @@ impl Type {
     /// to comply to the C and C++ layouts, that specify that every type needs
     /// to be addressable.
     pub fn is_unsized(&self, ctx: &BindgenContext) -> bool {
-        debug_assert!(ctx.in_codegen_phase(), "Not yet");
+        extra_assert!(ctx.in_codegen_phase(), "Not yet");
 
         match self.kind {
             TypeKind::Void => true,
@@ -1122,7 +1122,7 @@ impl Type {
                                         CXCursor_TypeAliasDecl => {
                                             let current = cur.cur_type();
 
-                                            debug_assert!(current.kind() ==
+                                            extra_assert!(current.kind() ==
                                                           CXType_Typedef);
 
                                             name = current.spelling();
diff --git a/src/lib.rs b/src/lib.rs
