Skip to content

Commit df16174

Browse files
committed
fix: ensure table tests pass with miri
We previously assumed that &'static str would always have the same address, but with miri it seems like this address is not actually stable (see rust-lang/miri#1955). So refactor the tests to let it use the StringInterner instead of constructing InternedString by itself.
1 parent 4de7ada commit df16174

File tree

2 files changed

+41
-34
lines changed

2 files changed

+41
-34
lines changed

lib/strings/src/string_interner.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::table::StringTable;
44

55
#[derive(Eq, Clone, Copy)]
66
pub struct InternedString {
7-
ptr: *const str,
7+
pub(crate) ptr: *const str,
88
pub(crate) hash: usize,
99
}
1010

@@ -13,10 +13,6 @@ impl InternedString {
1313
s.bytes().fold(2166136261, |hash, byte| (hash ^ byte as usize).wrapping_mul(16777619))
1414
}
1515

16-
pub fn new(s: &str) -> Self {
17-
Self { ptr: s as *const str, hash: Self::hash(s) }
18-
}
19-
2016
#[allow(clippy::op_ref)]
2117
/// Compare actual underlying strings, instead of just comparing pointers.
2218
/// This is only needed when interning new strings.

lib/strings/src/table.rs

+40-29
Original file line numberDiff line numberDiff line change
@@ -168,48 +168,59 @@ impl<V> Entry<V> {
168168

169169
#[cfg(test)]
170170
mod tests {
171+
use crate::string_interner::StringInterner;
172+
171173
use super::*;
172174

173175
#[test]
174176
fn table() {
175177
let mut table = StringTable::new();
176-
table.insert(InternedString::new("hello"), 1);
177-
table.insert(InternedString::new("world"), 2);
178-
assert_eq!(table.get(InternedString::new("world")), Some(&2));
179-
table.insert(InternedString::new("a"), 3);
180-
table.insert(InternedString::new("b"), 4);
181-
table.insert(InternedString::new("c"), 5);
182-
table.insert(InternedString::new("d"), 6);
183-
table.insert(InternedString::new("e"), 7);
184-
table.insert(InternedString::new("f"), 8);
178+
let mut interner = StringInterner::with_capacity(8);
179+
table.insert(interner.intern("hello"), 1);
180+
table.insert(interner.intern("world"), 2);
181+
assert_eq!(table.get(interner.intern("world")), Some(&2));
182+
table.insert(interner.intern("a"), 3);
183+
table.insert(interner.intern("b"), 4);
184+
table.insert(interner.intern("c"), 5);
185+
table.insert(interner.intern("d"), 6);
186+
table.insert(interner.intern("e"), 7);
187+
table.insert(interner.intern("f"), 8);
185188

186-
assert_eq!(table.get(InternedString::new("world")), Some(&2));
187-
assert_eq!(table.get(InternedString::new("nope")), None);
188-
assert_eq!(table.get_mut(InternedString::new("world")), Some(&mut 2));
189-
assert_eq!(table.get_mut(InternedString::new("nope")), None);
189+
assert_eq!(table.get(interner.intern("world")), Some(&2));
190+
assert_eq!(table.get(interner.intern("nope")), None);
191+
assert_eq!(table.get_mut(interner.intern("world")), Some(&mut 2));
192+
assert_eq!(table.get_mut(interner.intern("nope")), None);
190193

191-
table.insert(InternedString::new("world"), -123);
192-
assert_eq!(table.get(InternedString::new("world")), Some(&-123));
194+
table.insert(interner.intern("world"), -123);
195+
assert_eq!(table.get(interner.intern("world")), Some(&-123));
193196

194-
*table.get_mut(InternedString::new("e")).unwrap() = 123;
195-
assert_eq!(table.get(InternedString::new("e")), Some(&123));
197+
*table.get_mut(interner.intern("e")).unwrap() = 123;
198+
assert_eq!(table.get(interner.intern("e")), Some(&123));
196199
}
197200

198201
#[test]
199202
fn get_str_eq() {
200203
let s = "s".to_string();
201204
let mut table = StringTable::new();
202-
table.insert(InternedString::new(s.as_str()), 123);
203-
204-
assert_eq!(table.get(InternedString::new(s.as_str())), Some(&123));
205-
206-
#[allow(clippy::redundant_clone)]
207-
let s2 = s.clone();
208-
assert_eq!(table.get(InternedString::new(s2.as_str())), None);
209-
assert_eq!(table.get_str_eq(InternedString::new("hkjhkjhkj")), None);
210-
assert_eq!(
211-
table.get_str_eq(InternedString::new(s2.as_str())),
212-
Some(&InternedString::new(s.as_str()))
213-
);
205+
let mut interner = StringInterner::with_capacity(16);
206+
207+
let s_internered = interner.intern(s.as_str());
208+
table.insert(s_internered, 123);
209+
210+
assert_eq!(table.get(s_internered), Some(&123));
211+
assert_eq!(table.get_str_eq(s_internered), Some(&s_internered));
212+
assert_eq!(table.get_str_eq(interner.intern("hkjhkjhkj")), None);
213+
214+
let mut interner2 = StringInterner::with_capacity(16);
215+
216+
// Clone the string to give it a different address, and then intern it in a second interner,
217+
// such that the resulting interned string has a different address as well.
218+
let s_cloned_interned = interner2.intern(&s.clone());
219+
assert_ne!(s_internered, s_cloned_interned);
220+
221+
// Now, normal get() returns None because it only compares the address, but get_str_eq()
222+
// findfs the original interned string since it should do full string comparison.
223+
assert_eq!(table.get(s_cloned_interned), None);
224+
assert_eq!(table.get_str_eq(s_cloned_interned), Some(&s_internered));
214225
}
215226
}

0 commit comments

Comments
 (0)