mirror of
https://github.com/roc-lang/roc.git
synced 2025-08-03 11:52:19 +00:00
Do not fixup recursion pointers in non-recursive lambda sets
If a lambda set is non-recursive, but contains naked recursion pointers, we should not fill those naked pointers in with the slot of the lambda set during interning. Such naked pointers must belong to an encompassing lambda set that is in fact recursive, and will be filled in later. For example, `LambdaSet([Foo, LambdaSet(Bar, [<rec>])] as <rec>)` should not have the inner lambda set's capture be filled in with itself. Also, during reification of recursion pointers, we do not need to traverse re-inserted lambda sets again, since they were just fixed-up. Closes #5026
This commit is contained in:
parent
2321fa7504
commit
a3de22c88a
5 changed files with 114 additions and 21 deletions
|
@ -1,4 +1,5 @@
|
|||
use crate::ir::Parens;
|
||||
use crate::layout::intern::NeedsRecursionPointerFixup;
|
||||
use bitvec::vec::BitVec;
|
||||
use bumpalo::collections::Vec;
|
||||
use bumpalo::Bump;
|
||||
|
@ -1883,12 +1884,16 @@ impl<'a> LambdaSet<'a> {
|
|||
);
|
||||
cache_criteria.and(criteria);
|
||||
|
||||
let needs_recursive_fixup = NeedsRecursionPointerFixup(
|
||||
opt_recursion_var.is_some() && set_captures_have_naked_rec_ptr,
|
||||
);
|
||||
|
||||
let lambda_set = env.cache.interner.insert_lambda_set(
|
||||
env.arena,
|
||||
fn_args,
|
||||
ret,
|
||||
env.arena.alloc(set.into_bump_slice()),
|
||||
set_captures_have_naked_rec_ptr,
|
||||
needs_recursive_fixup,
|
||||
representation,
|
||||
);
|
||||
|
||||
|
@ -1902,7 +1907,7 @@ impl<'a> LambdaSet<'a> {
|
|||
fn_args,
|
||||
ret,
|
||||
&(&[] as &[(Symbol, &[InLayout])]),
|
||||
false,
|
||||
NeedsRecursionPointerFixup(false),
|
||||
Layout::UNIT,
|
||||
);
|
||||
Cacheable(Ok(lambda_set), cache_criteria)
|
||||
|
|
|
@ -121,6 +121,13 @@ impl<'a> Layout<'a> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Whether a recursive lambda set being inserted into an interner needs fixing-up of naked
|
||||
/// recursion pointers in the capture set.
|
||||
/// Applicable only if
|
||||
/// - the lambda set is indeed recursive, and
|
||||
/// - its capture set contain naked pointer references
|
||||
pub struct NeedsRecursionPointerFixup(pub bool);
|
||||
|
||||
pub trait LayoutInterner<'a>: Sized {
|
||||
/// Interns a value, returning its interned representation.
|
||||
/// If the value has been interned before, the old interned representation will be re-used.
|
||||
|
@ -139,7 +146,7 @@ pub trait LayoutInterner<'a>: Sized {
|
|||
args: &'a &'a [InLayout<'a>],
|
||||
ret: InLayout<'a>,
|
||||
set: &'a &'a [(Symbol, &'a [InLayout<'a>])],
|
||||
set_may_have_naked_rec_ptr: bool,
|
||||
needs_recursive_fixup: NeedsRecursionPointerFixup,
|
||||
representation: InLayout<'a>,
|
||||
) -> LambdaSet<'a>;
|
||||
|
||||
|
@ -550,7 +557,7 @@ impl<'a> GlobalLayoutInterner<'a> {
|
|||
&self,
|
||||
arena: &'a Bump,
|
||||
normalized: LambdaSet<'a>,
|
||||
set_may_have_naked_rec_ptr: bool,
|
||||
needs_recursive_fixup: NeedsRecursionPointerFixup,
|
||||
normalized_hash: u64,
|
||||
) -> WrittenGlobalLambdaSet<'a> {
|
||||
let mut normalized_lambda_set_map = self.0.normalized_lambda_set_map.lock();
|
||||
|
@ -574,7 +581,7 @@ impl<'a> GlobalLayoutInterner<'a> {
|
|||
let slot = unsafe { InLayout::from_index(vec.len()) };
|
||||
vec.push(Layout::VOID_NAKED);
|
||||
|
||||
let set = if set_may_have_naked_rec_ptr {
|
||||
let set = if needs_recursive_fixup.0 {
|
||||
let mut interner = LockedGlobalInterner {
|
||||
map: &mut map,
|
||||
normalized_lambda_set_map: &mut normalized_lambda_set_map,
|
||||
|
@ -708,7 +715,7 @@ impl<'a> LayoutInterner<'a> for TLLayoutInterner<'a> {
|
|||
args: &'a &'a [InLayout<'a>],
|
||||
ret: InLayout<'a>,
|
||||
set: &'a &'a [(Symbol, &'a [InLayout<'a>])],
|
||||
set_may_have_naked_rec_ptr: bool,
|
||||
needs_recursive_fixup: NeedsRecursionPointerFixup,
|
||||
representation: InLayout<'a>,
|
||||
) -> LambdaSet<'a> {
|
||||
// The tricky bit of inserting a lambda set is we need to fill in the `full_layout` only
|
||||
|
@ -737,7 +744,7 @@ impl<'a> LayoutInterner<'a> for TLLayoutInterner<'a> {
|
|||
} = global.get_or_insert_hashed_normalized_lambda_set(
|
||||
arena,
|
||||
normalized,
|
||||
set_may_have_naked_rec_ptr,
|
||||
needs_recursive_fixup,
|
||||
normalized_hash,
|
||||
);
|
||||
|
||||
|
@ -865,7 +872,7 @@ macro_rules! st_impl {
|
|||
args: &'a &'a [InLayout<'a>],
|
||||
ret: InLayout<'a>,
|
||||
set: &'a &'a [(Symbol, &'a [InLayout<'a>])],
|
||||
set_may_have_naked_rec_ptr: bool,
|
||||
needs_recursive_fixup: NeedsRecursionPointerFixup,
|
||||
representation: InLayout<'a>,
|
||||
) -> LambdaSet<'a> {
|
||||
// IDEA:
|
||||
|
@ -884,7 +891,7 @@ macro_rules! st_impl {
|
|||
let slot = unsafe { InLayout::from_index(self.vec.len()) };
|
||||
self.vec.push(Layout::VOID_NAKED);
|
||||
|
||||
let set = if set_may_have_naked_rec_ptr {
|
||||
let set = if needs_recursive_fixup.0 {
|
||||
reify::reify_lambda_set_captures(arena, self, slot, set)
|
||||
} else {
|
||||
set
|
||||
|
@ -957,7 +964,7 @@ mod reify {
|
|||
|
||||
use crate::layout::{Builtin, LambdaSet, Layout, UnionLayout};
|
||||
|
||||
use super::{InLayout, LayoutInterner};
|
||||
use super::{InLayout, LayoutInterner, NeedsRecursionPointerFixup};
|
||||
|
||||
// TODO: if recursion becomes a problem we could make this iterative
|
||||
pub fn reify_recursive_layout<'a>(
|
||||
|
@ -1103,7 +1110,8 @@ mod reify {
|
|||
arena.alloc(args),
|
||||
ret,
|
||||
arena.alloc(set),
|
||||
true,
|
||||
// All nested recursive pointers should been fixed up, since we just did that above.
|
||||
NeedsRecursionPointerFixup(false),
|
||||
representation,
|
||||
)
|
||||
}
|
||||
|
@ -1420,7 +1428,7 @@ mod insert_lambda_set {
|
|||
|
||||
use crate::layout::{LambdaSet, Layout};
|
||||
|
||||
use super::{GlobalLayoutInterner, InLayout, LayoutInterner};
|
||||
use super::{GlobalLayoutInterner, InLayout, LayoutInterner, NeedsRecursionPointerFixup};
|
||||
|
||||
const TARGET_INFO: TargetInfo = TargetInfo::default_x86_64();
|
||||
const TEST_SET: &&[(Symbol, &[InLayout])] =
|
||||
|
@ -1428,6 +1436,8 @@ mod insert_lambda_set {
|
|||
const TEST_ARGS: &&[InLayout] = &(&[Layout::UNIT] as &[_]);
|
||||
const TEST_RET: InLayout = Layout::UNIT;
|
||||
|
||||
const FIXUP: NeedsRecursionPointerFixup = NeedsRecursionPointerFixup(true);
|
||||
|
||||
#[test]
|
||||
fn two_threads_write() {
|
||||
for _ in 0..100 {
|
||||
|
@ -1440,7 +1450,7 @@ mod insert_lambda_set {
|
|||
for arena in arenas.iter_mut() {
|
||||
let mut interner = global.fork();
|
||||
handles.push(s.spawn(move || {
|
||||
interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, true, repr)
|
||||
interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, FIXUP, repr)
|
||||
}))
|
||||
}
|
||||
let ins: Vec<LambdaSet> = handles.into_iter().map(|t| t.join().unwrap()).collect();
|
||||
|
@ -1457,7 +1467,7 @@ mod insert_lambda_set {
|
|||
let mut interner = global.fork();
|
||||
|
||||
let lambda_set =
|
||||
interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, TEST_SET, true, Layout::UNIT);
|
||||
interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, TEST_SET, FIXUP, Layout::UNIT);
|
||||
let lambda_set_layout_in = interner.insert(Layout::LambdaSet(lambda_set));
|
||||
assert_eq!(lambda_set.full_layout, lambda_set_layout_in);
|
||||
}
|
||||
|
@ -1471,12 +1481,12 @@ mod insert_lambda_set {
|
|||
|
||||
let in1 = {
|
||||
let mut interner = global.fork();
|
||||
interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, true, repr)
|
||||
interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, FIXUP, repr)
|
||||
};
|
||||
|
||||
let in2 = {
|
||||
let mut st_interner = global.unwrap().unwrap();
|
||||
st_interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, true, repr)
|
||||
st_interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, FIXUP, repr)
|
||||
};
|
||||
|
||||
assert_eq!(in1, in2);
|
||||
|
@ -1491,12 +1501,12 @@ mod insert_lambda_set {
|
|||
let set = TEST_SET;
|
||||
let repr = Layout::UNIT;
|
||||
|
||||
let in1 = st_interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, true, repr);
|
||||
let in1 = st_interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, FIXUP, repr);
|
||||
|
||||
let global = st_interner.into_global();
|
||||
let mut interner = global.fork();
|
||||
|
||||
let in2 = interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, true, repr);
|
||||
let in2 = interner.insert_lambda_set(arena, TEST_ARGS, TEST_RET, set, FIXUP, repr);
|
||||
|
||||
assert_eq!(in1, in2);
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue