Skip to content

Commit 79afa5e

Browse files
fitzgenKowasaki
authored andcommitted
Define extra assertion macros
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.
1 parent 9700f61 commit 79afa5e

File tree

8 files changed

+49
-14
lines changed

8 files changed

+49
-14
lines changed

Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,6 @@ static = []
7171

7272
# These features only exist for CI testing -- don't use them if you're not hacking
7373
# on bindgen!
74-
testing_only_assert_no_dangling_items = []
7574
testing_only_docs = []
75+
testing_only_extra_assertions = []
7676
testing_only_llvm_stable = []

ci/test.sh

+5-2
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ cd "$(dirname "$0")/.."
66
# Regenerate the test headers' bindings in debug and release modes, and assert
77
# that we always get the expected generated bindings.
88

9-
cargo test --features "$BINDGEN_FEATURES testing_only_assert_no_dangling_items"
9+
cargo test --features "$BINDGEN_FEATURES"
10+
./ci/assert-no-diff.sh
11+
12+
cargo test --features "$BINDGEN_FEATURES testing_only_extra_assertions"
1013
./ci/assert-no-diff.sh
1114

12-
cargo test --release --features "$BINDGEN_FEATURES testing_only_assert_no_dangling_items"
15+
cargo test --release --features "$BINDGEN_FEATURES testing_only_extra_assertions"
1316
./ci/assert-no-diff.sh
1417

1518
# Now test the expectations' size and alignment tests.

src/codegen/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2651,7 +2651,7 @@ impl TryToRustTy for TemplateInstantiation {
26512651
// This can happen if we generated an opaque type for a partial
26522652
// template specialization, and we've hit an instantiation of
26532653
// that partial specialization.
2654-
debug_assert!(ctx.resolve_type_through_type_refs(decl)
2654+
extra_assert!(ctx.resolve_type_through_type_refs(decl)
26552655
.is_opaque());
26562656
return Err(error::Error::InstantiationOfOpaqueType);
26572657
}

src/extra_assertions.rs

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
//! Macros for defining extra assertions that should only be checked in testing
2+
//! and/or CI when the `testing_only_extra_assertions` feature is enabled.
3+
4+
#[macro_export]
5+
macro_rules! extra_assert {
6+
( $cond:expr ) => {
7+
if cfg!(feature = "testing_only_extra_assertions") {
8+
assert!($cond);
9+
}
10+
};
11+
( $cond:expr , $( $arg:tt )+ ) => {
12+
if cfg!(feature = "testing_only_extra_assertions") {
13+
assert!($cond, $( $arg )* )
14+
}
15+
};
16+
}
17+
18+
#[macro_export]
19+
macro_rules! extra_assert_eq {
20+
( $lhs:expr , $rhs:expr ) => {
21+
if cfg!(feature = "testing_only_extra_assertions") {
22+
assert_eq!($lhs, $rhs);
23+
}
24+
};
25+
( $lhs:expr , $rhs:expr , $( $arg:tt )+ ) => {
26+
if cfg!(feature = "testing_only_extra_assertions") {
27+
assert!($lhs, $rhs, $( $arg )* );
28+
}
29+
};
30+
}

src/ir/context.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -584,12 +584,11 @@ impl<'ctx> BindgenContext<'ctx> {
584584
ret
585585
}
586586

587-
/// When the `testing_only_assert_no_dangling_items` feature is enabled,
588-
/// this function walks the IR graph and asserts that we do not have any
589-
/// edges referencing an ItemId for which we do not have an associated IR
590-
/// item.
587+
/// When the `testing_only_extra_assertions` feature is enabled, this
588+
/// function walks the IR graph and asserts that we do not have any edges
589+
/// referencing an ItemId for which we do not have an associated IR item.
591590
fn assert_no_dangling_references(&self) {
592-
if cfg!(feature = "testing_only_assert_no_dangling_items") {
591+
if cfg!(feature = "testing_only_extra_assertions") {
593592
for _ in self.assert_no_dangling_item_traversal() {
594593
// The iterator's next method does the asserting for us.
595594
}

src/ir/item.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ pub trait ItemAncestors {
7070
}
7171

7272
cfg_if! {
73-
if #[cfg(debug_assertions)] {
73+
if #[cfg(testing_only_extra_assertions)] {
7474
type DebugOnlyItemSet = ItemSet;
7575
} else {
7676
struct DebugOnlyItemSet;
@@ -123,7 +123,7 @@ impl<'a, 'b> Iterator for ItemAncestorsIter<'a, 'b>
123123
} else {
124124
self.item = item.parent_id();
125125

126-
debug_assert!(!self.seen.contains(&item.id()));
126+
extra_assert!(!self.seen.contains(&item.id()));
127127
self.seen.insert(item.id());
128128

129129
Some(item.id())
@@ -614,7 +614,7 @@ impl Item {
614614
let mut item = self;
615615

616616
loop {
617-
debug_assert!(!targets_seen.contains(&item.id()));
617+
extra_assert!(!targets_seen.contains(&item.id()));
618618
targets_seen.insert(item.id());
619619

620620
if self.annotations().use_instead_of().is_some() {

src/ir/named.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> {
371371
fn constrain(&mut self, id: ItemId) -> bool {
372372
// Invariant: all hash map entries' values are `Some` upon entering and
373373
// exiting this method.
374-
debug_assert!(self.used.values().all(|v| v.is_some()));
374+
extra_assert!(self.used.values().all(|v| v.is_some()));
375375

376376
// Take the set for this id out of the hash map while we mutate it based
377377
// on other hash map entries. We *must* put it back into the hash map at
@@ -446,7 +446,7 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> {
446446
// Put the set back in the hash map and restore our invariant.
447447
debug_assert!(self.used[&id].is_none());
448448
self.used.insert(id, Some(used_by_this_id));
449-
debug_assert!(self.used.values().all(|v| v.is_some()));
449+
extra_assert!(self.used.values().all(|v| v.is_some()));
450450

451451
new_len != original_len
452452
}

src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ extern crate log;
3737
#[macro_use]
3838
mod log_stubs;
3939

40+
#[macro_use]
41+
mod extra_assertions;
42+
4043
// A macro to declare an internal module for which we *must* provide
4144
// documentation for. If we are building with the "testing_only_docs" feature,
4245
// then the module is declared public, and our `#![deny(missing_docs)]` pragma

0 commit comments

Comments
 (0)