-
Notifications
You must be signed in to change notification settings - Fork 174
Description
After days of debugging I found a shocking, but terribly subtle, issue.
Having the reference to the current frame be k , I ensured that k has a binding to a FnValue , which has an enclosing environment of k. This is what I expected. This is tested in the console.
However, for the bottom FnValue,
This shows that despite the frame looking identical, there are actually duplicate Frame objects in memory, so the FnValue does not actually always reference its enclosing Frame.
Suspected Cause:
The above means that somewhere in the code, new Frame objects are needlessly re-created instead of referring to the existing one, so environment objects aren't being properly shared/reused, even though they represent the same environment (as indicated by the same ID).
A possible cause would be in deepCopyTree:
export function deepCopyTree(value: EnvTree): EnvTree {
const clone = cloneDeep(value);
copyOwnPropertyDescriptors(value, clone);
return clone;
}
And in copyOwnPropertyDescriptors:
} else if (isEnvironment(source) && isEnvironment(destination)) {
// TODO: revisit this approach of copying the raw values and descriptors from source,
// as this defeats the purpose of deep cloning by referencing the original values again.
// Perhaps, there should be a new deep cloning function that also clones property descriptors.
// copy descriptors from source frame to destination frame
Object.defineProperties(destination.head, Object.getOwnPropertyDescriptors(source.head));
// copy heap from source frame to destination frame as well, to preserve references
destination.heap = source.heap;
// recurse on tail
copyOwnPropertyDescriptors(source.tail, destination.tail);
}
There even seems to be a TODO comment left behind that points out the exact issue! The problem is:
- deepCopyTree uses cloneDeep from lodash to create a deep copy
- But then it calls copyOwnPropertyDescriptors which:
- Copies the raw property descriptors from source to destination
- Directly assigns the source heap to the destination (destination.heap = source.heap)
This means we end up with a mix of cloned and referenced objects.
This explains why:
The environment IDs are the same (they're copied)
But the environment objects are different (some parts are cloned, others referenced)
To replicate:
In CseMachineLayout.tsx, have a reference to current frame by insert const k = FrameComponent.envFrameMap.get(CseMachine.getCurrentEnvId()); after // initialize control and stash Layout.initializeControlStash(chapter);
Run this in CSE Machine:
function test_func() {
const x = [];
const a = d => d + 1;
return a;
}
const result = test_func();
result(5);
Use Chrome Console and set breakpoint in CseMachineLayout.tsx. Step through the CSE machine from step 31 to 33, then the Chrome Console debugger to CseMachineLayout.tsx. Test the above in screenshots.



