diff --git a/src/bundles/binary_tree/src/__tests__/index.test.ts b/src/bundles/binary_tree/src/__tests__/index.test.ts index 25198d48d5..2c368a2819 100644 --- a/src/bundles/binary_tree/src/__tests__/index.test.ts +++ b/src/bundles/binary_tree/src/__tests__/index.test.ts @@ -15,6 +15,19 @@ async function rawTree( ); } +async function flatTree( + handler: TestDataHandler, + value: number, + left: TypedValue, + right: TypedValue +) { + const tree = await handler.array_make(DataType.ANY, 3, emptyListValue()); + await handler.array_set(tree, 0, numberValue(value)); + await handler.array_set(tree, 1, left); + await handler.array_set(tree, 2, right); + return tree; +} + describe(funcs.is_tree, () => { it('returns false when argument is not a list', async () => { const handler = new TestDataHandler(); @@ -59,24 +72,30 @@ describe(funcs.is_tree, () => { await expect(funcs.is_tree(handler, tree)).resolves.toEqual(true); }); - it('returns true for a tree round-tripped back in as DataType.ARRAY, not just DataType.PAIR', async () => { - // Per py-slang: pythonToModule now builds every Python list as a flat DataType.ARRAY, never a - // DataType.PAIR chain - so a tree round-tripped out to Python (via moduleToPython) and passed - // back into e.g. entry()/left_branch() arrives tagged ARRAY, not PAIR. is_tree (and - // assertNonEmptyTree) must accept either shape - this is the regression test for that. + it('returns true for a tree passed back in as a flat 3-element DataType.ARRAY', async () => { + // py-slang and js-slang both flatten a proper list into one ARRAY when a program passes it to a + // module, so a tree that make_tree returned comes back as [entry, left, right], not as a PAIR chain. const handler = new TestDataHandler(); - const rightPair = await handler.array_make(DataType.ANY, 2, emptyListValue()); - await handler.array_set(rightPair, 0, emptyListValue()); - await handler.array_set(rightPair, 1, emptyListValue()); - const leftPair = await handler.array_make(DataType.ANY, 2, emptyListValue()); - await handler.array_set(leftPair, 0, emptyListValue()); - await handler.array_set(leftPair, 1, rightPair); - const tree = await handler.array_make(DataType.ANY, 2, emptyListValue()); - await handler.array_set(tree, 0, numberValue(0)); - await handler.array_set(tree, 1, leftPair); + const leaf = await flatTree(handler, 4, emptyListValue(), emptyListValue()); + const tree = await flatTree(handler, 5, leaf, emptyListValue()); + await expect(funcs.is_tree(handler, leaf)).resolves.toEqual(true); await expect(funcs.is_tree(handler, tree)).resolves.toEqual(true); - expect(await funcs.entry(handler, tree)).toEqual(numberValue(0)); + }); + + it('returns false for a DataType.ARRAY that does not have exactly 3 elements', async () => { + const handler = new TestDataHandler(); + const two = await handler.array_make(DataType.ANY, 2, emptyListValue()); + const four = await handler.array_make(DataType.ANY, 4, emptyListValue()); + + await expect(funcs.is_tree(handler, two)).resolves.toEqual(false); + await expect(funcs.is_tree(handler, four)).resolves.toEqual(false); + }); + + it('returns false for a flat DataType.ARRAY whose branches are not trees', async () => { + const handler = new TestDataHandler(); + const tree = await flatTree(handler, 0, numberValue(1), emptyListValue()); + await expect(funcs.is_tree(handler, tree)).resolves.toEqual(false); }); }); @@ -94,6 +113,18 @@ describe(funcs.make_tree, () => { funcs.make_tree(handler, numberValue(0), funcs.make_empty_tree(), numberValue(0) as unknown as TypedValue) ).rejects.toThrowError('make_tree expects binary tree for right'); }); + + it('accepts a branch passed back in as a flat 3-element DataType.ARRAY', async () => { + // Regression: make_tree(5, make_tree(4, None, None), None) failed with + // "make_tree expects binary tree for left (expected binary tree, got ARRAY)". + const handler = new TestDataHandler(); + const leaf = await flatTree(handler, 4, emptyListValue(), emptyListValue()); + const tree = await funcs.make_tree(handler, numberValue(5), leaf as unknown as TypedValue, funcs.make_empty_tree()); + + await expect(funcs.entry(handler, tree)).resolves.toEqual(numberValue(5)); + const left = await funcs.left_branch(handler, tree); + await expect(funcs.entry(handler, left)).resolves.toEqual(numberValue(4)); + }); }); describe(funcs.entry, () => { @@ -169,6 +200,19 @@ describe(funcs.left_branch, () => { }); }); +describe('accessors on a flat DataType.ARRAY tree', () => { + it('read entry, left_branch and right_branch by position', async () => { + const handler = new TestDataHandler(); + const left = await flatTree(handler, 1, emptyListValue(), emptyListValue()); + const right = await flatTree(handler, 2, emptyListValue(), emptyListValue()); + const tree = await flatTree(handler, 0, left, right); + + await expect(funcs.entry(handler, tree)).resolves.toEqual(numberValue(0)); + await expect(funcs.left_branch(handler, tree)).resolves.toBe(left); + await expect(funcs.right_branch(handler, tree)).resolves.toBe(right); + }); +}); + describe(funcs.right_branch, () => { it('throws when argument is not a tree', async () => { const handler = new TestDataHandler(); diff --git a/src/bundles/binary_tree/src/functions.ts b/src/bundles/binary_tree/src/functions.ts index f00cdea936..224ae7f347 100644 --- a/src/bundles/binary_tree/src/functions.ts +++ b/src/bundles/binary_tree/src/functions.ts @@ -61,34 +61,44 @@ export async function make_tree( export async function is_tree(evaluator: IDataHandler, value: TypedValue): Promise { if (!value) return false; if (value.type === DataType.EMPTY_LIST) return true; - // A tree node is a pair - per conductor's "a pair is just an array of length 2" model, that may - // arrive tagged DataType.PAIR (built directly by make_tree) or DataType.ARRAY (round-tripped - // back in through a Python list, since py-slang's pythonToModule builds every list as an ARRAY - // now, not a PAIR chain). pair_head/pair_tail already read either shape the same way. - if (!isPairLike(value)) return false; - const rest = await evaluator.pair_tail(value as TypedValue); - if (!isPairLike(rest)) return false; + const node = await readTreeNode(evaluator, value); + if (!node) return false; - const left = await evaluator.pair_head(rest as TypedValue); - if (!await is_tree(evaluator, left)) return false; - - const rightRest = await evaluator.pair_tail(rest as TypedValue); - if (!isPairLike(rightRest)) return false; - - const right = await evaluator.pair_head(rightRest as TypedValue); - if (!await is_tree(evaluator, right)) return false; - - const tail = await evaluator.pair_tail(rightRest as TypedValue); - return tail.type === DataType.EMPTY_LIST; + const [, left, right] = node; + return await is_tree(evaluator, left) && await is_tree(evaluator, right); } -/** A pair is just an array of length 2 (no distinct "pair" representation) - a tree node may be - * tagged either DataType.PAIR (built directly by make_tree's own pair_make calls) or - * DataType.ARRAY (round-tripped back in through Python, since pythonToModule builds every list as - * an ARRAY now). Both are equally valid; pair_head/pair_tail read either the same way. */ -function isPairLike(value: TypedValue): boolean { - return value.type === DataType.PAIR || value.type === DataType.ARRAY; +/** + * Reads a non-empty tree node, the list `[entry, left, right]`, into its three elements, or returns + * `undefined` if the value is not a list of exactly three elements. The branches themselves are not + * checked. + * + * A node built by make_tree is a chain of DataType.PAIRs. The same node passed back into this module + * from a program arrives as a flat 3-element DataType.ARRAY instead: py-slang and js-slang both + * flatten a proper list into one ARRAY on the way in. Walking that ARRAY with pair_head/pair_tail + * would read its elements 0 and 1 as head and tail, so an ARRAY is read by index instead. + */ +async function readTreeNode( + evaluator: IDataHandler, + value: TypedValue +): Promise<[TypedValue, TypedValue, TypedValue] | undefined> { + const elements: TypedValue[] = []; + if (value.type === DataType.ARRAY) { + const length = await evaluator.array_length(value); + if (length !== 3) return undefined; + for (let i = 0; i < length; i += 1) { + elements.push(await evaluator.array_get(value, i)); + } + } else { + let current: TypedValue = value; + while (current.type === DataType.PAIR && elements.length < 4) { + elements.push(await evaluator.pair_head(current)); + current = await evaluator.pair_tail(current); + } + if (current.type !== DataType.EMPTY_LIST || elements.length !== 3) return undefined; + } + return elements as [TypedValue, TypedValue, TypedValue]; } /** @@ -110,19 +120,16 @@ async function assertNonEmptyTree( evaluator: IDataHandler, value: TypedValue, funcName: string -): Promise { +): Promise<[TypedValue, TypedValue, TypedValue]> { if (!value || !await is_tree(evaluator, value)) { throw new EvaluatorTypeError(`${funcName} expects binary tree`, 'binary tree', value ? DataType[value.type] : 'undefined'); } - if (!isPairLike(value)) { + const node = await readTreeNode(evaluator, value); + if (!node) { throw new EvaluatorRuntimeError(`${funcName} received an empty binary tree!`); } - - // NonEmptyBinaryTree is declared DataType.PAIR, but the runtime value may genuinely be - // DataType.ARRAY (round-tripped back in through Python) - pair_head/pair_tail read either the - // same way (see isPairLike's doc comment), so this is a safe, documented cast, not a lie. - return value as NonEmptyBinaryTree; + return node; } /** @@ -137,8 +144,8 @@ async function assertNonEmptyTree( * @returns Value */ export async function entry(evaluator: IDataHandler, t: TypedValue): Promise> { - const tree = await assertNonEmptyTree(evaluator, t, entry.name); - return evaluator.pair_head(tree); + const [value] = await assertNonEmptyTree(evaluator, t, entry.name); + return value; } /** @@ -153,9 +160,8 @@ export async function entry(evaluator: IDataHandler, t: TypedValue): P * @returns BinaryTree */ export async function left_branch(evaluator: IDataHandler, t: TypedValue): Promise { - const tree = await assertNonEmptyTree(evaluator, t, left_branch.name); - const rest = await evaluator.pair_tail(tree); - return (await evaluator.pair_head(rest as TypedValue)) as BinaryTree; + const [, left] = await assertNonEmptyTree(evaluator, t, left_branch.name); + return left as BinaryTree; } /** @@ -170,8 +176,6 @@ export async function left_branch(evaluator: IDataHandler, t: TypedValue): Promise { - const tree = await assertNonEmptyTree(evaluator, t, right_branch.name); - const rest = await evaluator.pair_tail(tree); - const rightRest = await evaluator.pair_tail(rest as TypedValue); - return (await evaluator.pair_head(rightRest as TypedValue)) as BinaryTree; + const [, , right] = await assertNonEmptyTree(evaluator, t, right_branch.name); + return right as BinaryTree; }