ci: Replace extra_assert by debug_assert

pvdrz · pvdrz · commit 4bc7403f03b7 · 2024-12-04T16:49:22.000-05:00
This basically means that running a debug build of `bindgen` will always
have the extra assertions enabled, which should help us catch more bugs
than having them only running on CI.
diff --git a/.github/workflows/bindgen.yml b/.github/workflows/bindgen.yml
@@ -136,17 +136,8 @@ jobs:
         # FIXME: There are no pre-built static libclang libraries, so the
         # `static` feature is not testable atm.
         feature_runtime: [0, 1]
-        feature_extra_asserts: [0]
 
         include:
-          # Test with extra asserts + docs just with latest llvm versions to
-          # prevent explosion
-          - os: ubuntu-latest
-            llvm_version: "16.0"
-            release_build: 0
-            no_default_features: 0
-            feature_extra_asserts: 1
-
           # FIXME: Seems installing multiarch packages fails:
           #
           #   https://github.com/rust-lang/rust-bindgen/pull/2037/checks?check_run_id=2441799333
@@ -159,14 +150,12 @@ jobs:
           #   llvm_version: "16.0"
           #   main_tests: 0
           #   release_build: 0
-          #   feature_extra_asserts: 0
 
           # Ensure stuff works on macos too
           - os: macos-latest
             llvm_version: "16.0"
             release_build: 0
             no_default_features: 0
-            feature_extra_asserts: 0
     steps:
       - uses: actions/checkout@v4
 
@@ -207,9 +196,8 @@ jobs:
           BINDGEN_MAIN_TESTS: ${{matrix.main_tests}}
           BINDGEN_RELEASE_BUILD: ${{matrix.release_build}}
           BINDGEN_FEATURE_RUNTIME: ${{matrix.feature_runtime}}
-          BINDGEN_FEATURE_EXTRA_ASSERTS: ${{matrix.feature_extra_asserts}}
           BINDGEN_NO_DEFAULT_FEATURES: ${{matrix.no_default_features}}
-          BINDGEN_RUST_FOR_LINUX_TEST: ${{matrix.os == 'ubuntu-latest' && matrix.llvm_version == '16.0' && matrix.feature_extra_asserts == 0 && 1 || 0}}
+          BINDGEN_RUST_FOR_LINUX_TEST: ${{matrix.os == 'ubuntu-latest' && matrix.llvm_version == '16.0' && 1 || 0}}
         run: ./ci/test.sh
 
   check-cfg:
diff --git a/bindgen-cli/Cargo.toml b/bindgen-cli/Cargo.toml
@@ -38,7 +38,6 @@ prettyplease = ["bindgen/prettyplease"]
 ## The following features are for internal use and they shouldn't be used if
 ## you're not hacking on bindgen
 # Features used for CI testing 
-__testing_only_extra_assertions = ["bindgen/__testing_only_extra_assertions"]
 __testing_only_libclang_9 = ["bindgen/__testing_only_libclang_9"]
 __testing_only_libclang_16 = ["bindgen/__testing_only_libclang_16"]
 
diff --git a/bindgen-integration/Cargo.toml b/bindgen-integration/Cargo.toml
@@ -18,6 +18,5 @@ cc.workspace = true
 static = ["bindgen/static"]
 runtime = ["bindgen/runtime"]
 
-__testing_only_extra_assertions = ["bindgen/__testing_only_extra_assertions"]
 __testing_only_libclang_9 = ["bindgen/__testing_only_libclang_9"]
 __testing_only_libclang_16 = ["bindgen/__testing_only_libclang_16"]
diff --git a/bindgen-tests/Cargo.toml b/bindgen-tests/Cargo.toml
@@ -23,6 +23,5 @@ logging = ["bindgen/logging"]
 static = ["bindgen/static"]
 runtime = ["bindgen/runtime"]
 
-__testing_only_extra_assertions = ["bindgen/__testing_only_extra_assertions"]
 __testing_only_libclang_9 = ["bindgen/__testing_only_libclang_9"]
 __testing_only_libclang_16 = ["bindgen/__testing_only_libclang_16"]
diff --git a/bindgen-tests/tests/stylo_sanity.rs b/bindgen-tests/tests/stylo_sanity.rs
@@ -10,10 +10,7 @@
 /// how long bindings generation takes for Stylo. Stylo bindings generation
 /// takes too long to be a proper `#[bench]`.
 #[test]
-#[cfg(not(any(
-    debug_assertions,
-    feature = "__testing_only_extra_assertions",
-)))]
+#[cfg(not(any(debug_assertions, debug_assertions,)))]
 #[cfg(any(
     feature = "__testing_only_libclang_9",
     feature = "__testing_only_libclang_16"
diff --git a/bindgen/Cargo.toml b/bindgen/Cargo.toml
@@ -56,7 +56,6 @@ experimental = ["dep:annotate-snippets"]
 # Features used by `bindgen-cli`
 __cli = ["dep:clap", "dep:clap_complete"]
 # Features used for CI testing
-__testing_only_extra_assertions = []
 __testing_only_libclang_9 = []
 __testing_only_libclang_16 = []
 
diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs
@@ -4446,7 +4446,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.
-            extra_assert!(def.is_opaque(ctx, &()));
+            debug_assert!(def.is_opaque(ctx, &()));
             return Err(Error::InstantiationOfOpaqueType);
         }
 
diff --git a/bindgen/extra_assertions.rs b/bindgen/extra_assertions.rs
diff --git a/bindgen/ir/analysis/derive.rs b/bindgen/ir/analysis/derive.rs
@@ -698,7 +698,7 @@ impl<'ctx> MonotoneFramework for CannotDerive<'ctx> {
 
 impl<'ctx> From<CannotDerive<'ctx>> for HashMap<ItemId, CanDerive> {
     fn from(analysis: CannotDerive<'ctx>) -> Self {
-        extra_assert!(analysis
+        debug_assert!(analysis
             .can_derive
             .values()
             .all(|v| *v != CanDerive::Yes));
diff --git a/bindgen/ir/analysis/has_vtable.rs b/bindgen/ir/analysis/has_vtable.rs
@@ -210,7 +210,7 @@ impl<'ctx> MonotoneFramework for HasVtableAnalysis<'ctx> {
 impl<'ctx> From<HasVtableAnalysis<'ctx>> for HashMap<ItemId, HasVtableResult> {
     fn from(analysis: HasVtableAnalysis<'ctx>) -> Self {
         // We let the lack of an entry mean "No" to save space.
-        extra_assert!(analysis
+        debug_assert!(analysis
             .have_vtable
             .values()
             .all(|v| { *v != HasVtableResult::No }));
diff --git a/bindgen/ir/analysis/sizedness.rs b/bindgen/ir/analysis/sizedness.rs
@@ -329,7 +329,7 @@ impl<'ctx> MonotoneFramework for SizednessAnalysis<'ctx> {
 impl<'ctx> From<SizednessAnalysis<'ctx>> for HashMap<TypeId, SizednessResult> {
     fn from(analysis: SizednessAnalysis<'ctx>) -> Self {
         // We let the lack of an entry mean "ZeroSized" to save space.
-        extra_assert!(analysis
+        debug_assert!(analysis
             .sized
             .values()
             .all(|v| { *v != SizednessResult::ZeroSized }));
diff --git a/bindgen/ir/analysis/template_params.rs b/bindgen/ir/analysis/template_params.rs
@@ -453,7 +453,7 @@ impl<'ctx> MonotoneFramework for UsedTemplateParameters<'ctx> {
             }
         }
 
-        if cfg!(feature = "__testing_only_extra_assertions") {
+        if cfg!(debug_assertions) {
             // Invariant: The `used` map has an entry for every allowlisted
             // item, as well as all explicitly blocklisted items that are
             // reachable from allowlisted items.
@@ -465,13 +465,13 @@ impl<'ctx> MonotoneFramework for UsedTemplateParameters<'ctx> {
             // to have a set of template parameters, and we can allow
             // blocklisted templates to use all of their parameters).
             for item in &allowlisted_items {
-                extra_assert!(used.contains_key(item));
-                extra_assert!(dependencies.contains_key(item));
+                debug_assert!(used.contains_key(item));
+                debug_assert!(dependencies.contains_key(item));
                 item.trace(
                     ctx,
                     &mut |sub_item, _| {
-                        extra_assert!(used.contains_key(&sub_item));
-                        extra_assert!(dependencies.contains_key(&sub_item));
+                        debug_assert!(used.contains_key(&sub_item));
+                        debug_assert!(dependencies.contains_key(&sub_item));
                     },
                     &(),
                 );
@@ -510,7 +510,7 @@ impl<'ctx> MonotoneFramework for UsedTemplateParameters<'ctx> {
     fn constrain(&mut self, id: ItemId) -> ConstrainResult {
         // Invariant: all hash map entries' values are `Some` upon entering and
         // exiting this method.
-        extra_assert!(self.used.values().all(|v| v.is_some()));
+        debug_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
@@ -568,7 +568,7 @@ impl<'ctx> MonotoneFramework for UsedTemplateParameters<'ctx> {
         // Put the set back in the hash map and restore our invariant.
         debug_assert!(self.used[&id].is_none());
         self.used.insert(id, Some(used_by_this_id));
-        extra_assert!(self.used.values().all(|v| v.is_some()));
+        debug_assert!(self.used.values().all(|v| v.is_some()));
 
         if new_len == original_len {
             ConstrainResult::Same
diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs
@@ -1221,7 +1221,7 @@ If you encounter an error missing from this list, please file an issue or a PR!"
     /// 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_extra_assertions") {
+        if cfg!(debug_assertions) {
             for _ in self.assert_no_dangling_item_traversal() {
                 // The iterator's next method does the asserting for us.
             }
@@ -1246,7 +1246,7 @@ If you encounter an error missing from this list, please file an issue or a PR!"
     /// every item and ensure that it is in the children set of one of its
     /// module ancestors.
     fn assert_every_item_in_a_module(&self) {
-        if cfg!(feature = "__testing_only_extra_assertions") {
+        if cfg!(debug_assertions) {
             assert!(self.in_codegen_phase());
             assert!(self.current_module == self.root_module);
 
diff --git a/bindgen/ir/item.rs b/bindgen/ir/item.rs
@@ -91,13 +91,13 @@ pub(crate) trait ItemAncestors {
     fn ancestors<'a>(&self, ctx: &'a BindgenContext) -> ItemAncestorsIter<'a>;
 }
 
-#[cfg(feature = "__testing_only_extra_assertions")]
+#[cfg(debug_assertions)]
 type DebugOnlyItemSet = ItemSet;
 
-#[cfg(not(feature = "__testing_only_extra_assertions"))]
+#[cfg(not(debug_assertions))]
 struct DebugOnlyItemSet;
 
-#[cfg(not(feature = "__testing_only_extra_assertions"))]
+#[cfg(not(debug_assertions))]
 impl DebugOnlyItemSet {
     fn new() -> Self {
         DebugOnlyItemSet
@@ -138,7 +138,7 @@ impl Iterator for ItemAncestorsIter<'_> {
         } else {
             self.item = item.parent_id();
 
-            extra_assert!(!self.seen.contains(&item.id()));
+            debug_assert!(!self.seen.contains(&item.id()));
             self.seen.insert(item.id());
 
             Some(item.id())
@@ -682,7 +682,7 @@ impl Item {
         let mut item = self;
 
         loop {
-            extra_assert!(!targets_seen.contains(&item.id()));
+            debug_assert!(!targets_seen.contains(&item.id()));
             targets_seen.insert(item.id());
 
             if self.annotations().use_instead_of().is_some() {
diff --git a/bindgen/lib.rs b/bindgen/lib.rs
@@ -29,9 +29,6 @@ extern crate log;
 #[macro_use]
 mod log_stubs;
 
-#[macro_use]
-mod extra_assertions;
-
 mod codegen;
 mod deps;
 mod options;
diff --git a/ci/test.sh b/ci/test.sh
@@ -40,9 +40,6 @@ get_cargo_args() {
   if [ "$BINDGEN_FEATURE_RUNTIME" == "1"  ]; then
     features+="runtime"
   fi
-  if [ "$BINDGEN_FEATURE_EXTRA_ASSERTS" == "1"  ]; then
-    features+=" __testing_only_extra_assertions"
-  fi
   if [ ! -z "$features" ]; then
     args+=" --features $(echo $features | tr ' ' ',')"
   fi

Original file line number	Diff line number	Diff line change
`@@ -4446,7 +4446,7 @@ impl TryToRustTy for TemplateInstantiation {`
`4446`	`4446`	`// This can happen if we generated an opaque type for a partial`
`4447`	`4447`	`// template specialization, and we've hit an instantiation of`
`4448`	`4448`	`// that partial specialization.`
`4449`		`- extra_assert!(def.is_opaque(ctx, &()));`
	`4449`	`+ debug_assert!(def.is_opaque(ctx, &()));`
`4450`	`4450`	`return Err(Error::InstantiationOfOpaqueType);`
`4451`	`4451`	`}`
`4452`	`4452`