Skip to content

Commit aad9f3d

Browse files
committed
fix: Use global interner for resolve_string to fix cross-arena resolution
The AstArena had a mismatch between intern_string (which returns symbols from the global interner) and resolve_string (which tried to resolve from the local interner). This caused incorrect string resolution when multiple arenas existed, as symbol indices from different interners don't match. Fixed by changing resolve_string to use the global interner via resolve_global(), leaking the string to provide a stable &str reference. Added tests: - test_cross_arena_resolution: verifies symbols work across arenas - test_sequential_arena_builds: validates stdlib test pattern
1 parent 3ee71d8 commit aad9f3d

1 file changed

Lines changed: 65 additions & 3 deletions

File tree

crates/typed_ast/src/arena.rs

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,19 @@ impl AstArena {
185185

186186
global_interned
187187
}
188-
188+
189189
/// Resolve an interned string back to its value
190+
///
191+
/// Since `intern_string` returns symbols from the global interner,
192+
/// we must resolve via global interner. The string is leaked to provide
193+
/// a stable &str reference (acceptable since interned strings are never freed).
190194
pub fn resolve_string(&self, interned: InternedString) -> Option<&str> {
191-
self.string_interner.resolve(interned.symbol())
195+
// Always use global interner since intern_string returns global symbols
196+
interned.resolve_global().map(|s| {
197+
// Leak the string to provide stable &str lifetime
198+
// This is safe because interned strings live for the program duration
199+
Box::leak(s.into_boxed_str()) as &str
200+
})
192201
}
193202

194203
/// Get statistics about arena usage
@@ -398,9 +407,62 @@ mod tests {
398407
#[test]
399408
fn test_capacity_estimation() {
400409
let (nodes, strings) = utils::estimate_arena_capacity(100, 50);
401-
410+
402411
// Should add buffer
403412
assert!(nodes > 100);
404413
assert!(strings > 50);
405414
}
415+
416+
#[test]
417+
fn test_cross_arena_resolution() {
418+
// Simulate what happens when multiple tests create separate arenas
419+
// All arenas share the GLOBAL_INTERNER
420+
421+
// Arena 1 interns "std"
422+
let std_symbol = {
423+
let mut arena1 = AstArena::new();
424+
arena1.intern_string("std")
425+
};
426+
427+
// Arena 2 interns different strings, then tries to resolve "std" symbol
428+
let mut arena2 = AstArena::new();
429+
arena2.intern_string("hash_fn");
430+
arena2.intern_string("malloc");
431+
432+
// The symbol from arena1 should still resolve correctly via global interner
433+
let resolved = arena2.resolve_string(std_symbol);
434+
assert_eq!(resolved, Some("std"), "Cross-arena resolution should work via global interner");
435+
}
436+
437+
#[test]
438+
fn test_sequential_arena_builds() {
439+
// Simulates the stdlib test pattern where each test builds parts of stdlib
440+
441+
// Test 1: builds hashmap functions
442+
{
443+
let mut arena = AstArena::new();
444+
let hash_fn = arena.intern_string("hash_fn");
445+
let eq_fn = arena.intern_string("eq_fn");
446+
assert_eq!(arena.resolve_string(hash_fn), Some("hash_fn"));
447+
assert_eq!(arena.resolve_string(eq_fn), Some("eq_fn"));
448+
}
449+
450+
// Test 2: builds memory functions
451+
{
452+
let mut arena = AstArena::new();
453+
let malloc = arena.intern_string("malloc");
454+
let free = arena.intern_string("free");
455+
assert_eq!(arena.resolve_string(malloc), Some("malloc"));
456+
assert_eq!(arena.resolve_string(free), Some("free"));
457+
}
458+
459+
// Test 3: builds full stdlib with module name "std"
460+
{
461+
let mut arena = AstArena::new();
462+
let std_name = arena.intern_string("std");
463+
// The symbol should resolve correctly even though global interner
464+
// already has many other strings from previous "tests"
465+
assert_eq!(arena.resolve_string(std_name), Some("std"));
466+
}
467+
}
406468
}

0 commit comments

Comments
 (0)