Skip to content

Commit

Permalink
Add a test to ensure that interners have no dangling pointers
Browse files Browse the repository at this point in the history
  • Loading branch information
lo48576 committed Aug 27, 2019
1 parent 9ca8921 commit eb54ae0
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 0 deletions.
58 changes: 58 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,64 @@ where
self.map.reserve(additional);
self.values.reserve(additional);
}

/// Checks whether the all `InternalStrRef`s refer the strigs owned by `self`.
///
/// For testing purpose only.
///
/// # Panics
///
/// Panics if the interner has wrong state. That is:
///
/// * when `InternalStrRef` refers the address which is not owned by the interner, or
/// * when there are `Box<str>` not referred by any `InternalStrRef` owned by the interner.
#[cfg(test)]
pub(crate) fn assert_internal_str_refs_validity(&self)
where
S: std::fmt::Debug,
{
// Collect `InternalStrRef` pointers.
let mut referred_ptrs = self
.map
.keys()
.map(|s| s.0)
.collect::<std::collections::HashSet<_>>();
// Remove owned pointers.
for (owned_str, owned_ptr) in self.values.iter().map(|v| (&**v, (&**v) as *const str)) {
if !referred_ptrs.remove(&owned_ptr) {
// `owned` is not in `referred_ptrs`.
// It means the `Box<str>` is not found by `get()` and `get_or_intern()`.
panic!(
"String {:?} at {:?} is not registered to `map`: self={:#?}",
owned_str, owned_ptr, self
);
}
}
if !referred_ptrs.is_empty() {
// `self.map` has some dangling pointers.
let values_ptrs = self
.values
.iter()
.map(|v| (&**v, (&**v) as *const str))
.collect::<Vec<_>>();
panic!(
"Dangling pointers found: pointers {:?} are not stored in `values`: \
self={:#?}, values_ptrs = {:?}",
referred_ptrs, self, values_ptrs
);
}
}

/// Returns the maximum capacity of the internal storages.
///
/// Storing `self.max_capacity() + 1` elements in total will cause all storages to be
/// reallocated at least once.
///
/// For testing purpose only.
#[cfg(test)]
pub(crate) fn max_capacity(&self) -> usize {
std::cmp::max(self.map.capacity(), self.values.capacity())
}
}

impl<S, H> StringInterner<S, H>
Expand Down
49 changes: 49 additions & 0 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,3 +458,52 @@ mod clone_and_drop {
}
}
}

/// Tests safety invariants of `StringInterner`.
mod internal_str_refs_validity {
use super::*;

/// Tests for interning, reallocation, and cloning.
#[test]
fn intern_reallocate_clone() {
let mut old = DefaultStringInterner::new();
old.assert_internal_str_refs_validity();
let mut syms_old = Vec::new();

// Cause allocation to `old`.
syms_old.push(old.get_or_intern("0"));
old.assert_internal_str_refs_validity();
// Fill storage with some elements.
for i in 1..old.capacity() {
syms_old.push(old.get_or_intern(i.to_string()));
old.assert_internal_str_refs_validity();
}
// Lookup all values.
for (i, sym) in syms_old.iter().enumerate() {
assert_eq!(old.resolve(*sym), Some(i.to_string().as_str()));
}

// Clone the interner.
let mut new = old.clone();
let mut syms_new = syms_old.clone();

// Cause reallocation to `old`.
for i in old.len()..=old.max_capacity() {
syms_old.push(old.get_or_intern(i.to_string()));
old.assert_internal_str_refs_validity();
}
// Cause reallocation to `new`.
for i in new.len()..=new.max_capacity() {
syms_new.push(new.get_or_intern(i.to_string()));
new.assert_internal_str_refs_validity();
}

// Lookup all values.
for (i, sym) in syms_old.iter().enumerate() {
assert_eq!(old.resolve(*sym), Some(i.to_string().as_str()));
}
for (i, sym) in syms_new.iter().enumerate() {
assert_eq!(new.resolve(*sym), Some(i.to_string().as_str()));
}
}
}

0 comments on commit eb54ae0

Please sign in to comment.