diff options
| -rw-r--r-- | CONTRIBUTING.md | 8 | ||||
| -rw-r--r-- | Cargo.toml | 10 | ||||
| -rwxr-xr-x | ci/assert-docs.sh | 2 | ||||
| -rwxr-xr-x | ci/test.sh | 7 | ||||
| -rw-r--r-- | src/codegen/mod.rs | 2 | ||||
| -rw-r--r-- | src/extra_assertions.rs | 30 | ||||
| -rw-r--r-- | src/ir/context.rs | 6 | ||||
| -rw-r--r-- | src/ir/item.rs | 6 | ||||
| -rw-r--r-- | src/ir/named.rs | 29 | ||||
| -rw-r--r-- | src/lib.rs | 11 |
10 files changed, 80 insertions, 31 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0aad4f2c..1d78497e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -65,12 +65,12 @@ $ export LD_LIBRARY_PATH=path/to/clang-3.9/lib # for Linux $ export DYLD_LIBRARY_PATH=path/to/clang-3.9/lib # for macOS ``` -Additionally, you may want to build and test with the `docs_` feature to ensure -that you aren't forgetting to document types and functions. CI will catch it if -you forget, but the turn around will be a lot slower ;) +Additionally, you may want to build and test with the `testing_only_docs` +feature to ensure that you aren't forgetting to document types and functions. CI +will catch it if you forget, but the turn around will be a lot slower ;) ``` -$ cargo build --features docs_ +$ cargo build --features testing_only_docs ``` ## Testing @@ -65,10 +65,12 @@ features = ["with-syntex"] version = "0.29" [features] -assert_no_dangling_items = [] default = ["logging"] -testing_only_llvm_stable = [] logging = ["env_logger", "log"] static = [] -# This feature only exists for CI -- don't use it! -docs_ = [] + +# These features only exist for CI testing -- don't use them if you're not hacking +# on bindgen! +testing_only_docs = [] +testing_only_extra_assertions = [] +testing_only_llvm_stable = [] diff --git a/ci/assert-docs.sh b/ci/assert-docs.sh index aa4b90ac..d5757403 100755 --- a/ci/assert-docs.sh +++ b/ci/assert-docs.sh @@ -3,4 +3,4 @@ set -xeu cd "$(dirname "$0")/.." -cargo build --features "$BINDGEN_FEATURES docs_" +cargo build --features "$BINDGEN_FEATURES testing_only_docs" @@ -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 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 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/codegen/mod.rs b/src/codegen/mod.rs index c203795a..579e9de1 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -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 new file mode 100644 index 00000000..b89c718a --- /dev/null +++ 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/context.rs b/src/ir/context.rs index 32ee5bd0..9b9ad8bd 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -584,9 +584,11 @@ impl<'ctx> BindgenContext<'ctx> { ret } - /// This function trying to find any dangling references inside of `items` + /// 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 = "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. } diff --git a/src/ir/item.rs b/src/ir/item.rs index 5477dee9..5e806de9 100644 --- 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()) @@ -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() { diff --git a/src/ir/named.rs b/src/ir/named.rs index 7cae195b..67b36915 100644 --- a/src/ir/named.rs +++ b/src/ir/named.rs @@ -371,14 +371,19 @@ 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 // the end of this method. This allows us to side-step HashMap's lack of // an analog to slice::split_at_mut. - let mut used_by_this_id = - self.used.get_mut(&id).unwrap().take().unwrap(); + let mut used_by_this_id = self.used + .get_mut(&id) + .expect("Should have a set of used template params for every item \ + id") + .take() + .expect("Should maintain the invariant that all used template param \ + sets are `Some` upon entry of `constrain`"); let original_len = used_by_this_id.len(); @@ -415,29 +420,33 @@ impl<'ctx, 'gen> MonotoneFramework for UsedTemplateParameters<'ctx, 'gen> { // Otherwise, add the union of each of its referent item's template // parameter usage. _ => { - item.trace(self.ctx, - &mut |sub_id, edge_kind| { + item.trace(self.ctx, &mut |sub_id, edge_kind| { if sub_id == id || !Self::consider_edge(edge_kind) { return; } let used_by_sub_id = self.used[&sub_id] .as_ref() - .unwrap() + .expect("Because sub_id != id, and all used template \ + param sets other than id's are `Some`, \ + sub_id's used template param set should be \ + `Some`") .iter() .cloned(); used_by_this_id.extend(used_by_sub_id); - }, - &()); + }, &()); } } let new_len = used_by_this_id.len(); - assert!(new_len >= original_len); + assert!(new_len >= original_len, + "This is the property that ensures this function is monotone -- \ + if it doesn't hold, the analysis might never terminate!"); // 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)); - debug_assert!(self.used.values().all(|v| v.is_some())); + extra_assert!(self.used.values().all(|v| v.is_some())); new_len != original_len } @@ -37,16 +37,19 @@ extern crate log; #[macro_use] mod log_stubs; +#[macro_use] +mod extra_assertions; + // A macro to declare an internal module for which we *must* provide -// documentation for. If we are building with the "docs_" feature, then the -// module is declared public, and our `#![deny(missing_docs)]` pragma applies to -// it. This feature is used in CI, so we won't let anything slip by +// documentation for. If we are building with the "testing_only_docs" feature, +// then the module is declared public, and our `#![deny(missing_docs)]` pragma +// applies to it. This feature is used in CI, so we won't let anything slip by // undocumented. Normal builds, however, will leave the module private, so that // we don't expose internals to library consumers. macro_rules! doc_mod { ($m:ident, $doc_mod_name:ident) => { cfg_if! { - if #[cfg(feature = "docs_")] { + if #[cfg(feature = "testing_only_docs")] { pub mod $doc_mod_name { //! Autogenerated documentation module. pub use super::$m::*; |