From 0cb2e7ba07b3eae3c1b5372c90ecfaf1918e98bf Mon Sep 17 00:00:00 2001 From: johba Date: Fri, 20 Mar 2026 08:42:34 +0100 Subject: [PATCH] fix: EXEC.IF branch reconciliation still injects synthetic zeros (#618) (#1033) Fixes #618 ## Changes Add stack depth validation in processExecIf() so asymmetric EXEC.IF branches (where one branch pushes more values than the other) throw an explicit error instead of silently padding with '0'. Error messages identify both branch depths for DYADIC and BOOLEAN stacks. Removed dead-code '0'/'false' fallbacks in buildAssignments and reconstruction. Updated existing unbalanced-branch tests to expect errors; added regression tests for error message content and BOOLEAN mismatch. All existing seed files (optimizer_v3.push3, optimizer_seed.push3) continue to transpile. Co-authored-by: openhands Reviewed-on: https://codeberg.org/johba/harb/pulls/1033 Reviewed-by: Disinto_bot --- tools/push3-transpiler/src/transpiler.ts | 28 ++++++++--- .../push3-transpiler/test/transpiler.test.ts | 49 +++++++++++++------ 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/tools/push3-transpiler/src/transpiler.ts b/tools/push3-transpiler/src/transpiler.ts index 198561a..e1625cd 100644 --- a/tools/push3-transpiler/src/transpiler.ts +++ b/tools/push3-transpiler/src/transpiler.ts @@ -255,8 +255,24 @@ function processExecIf( processItems(toItems(falseBranch), falseState); state.varCounter = falseState.varCounter; + // --- Validate branch stack depths match --- + if (trueState.dStack.length !== falseState.dStack.length) { + throw new Error( + `EXEC.IF: DYADIC stack depth mismatch — ` + + `true branch produces ${trueState.dStack.length} values, ` + + `false branch produces ${falseState.dStack.length}` + ); + } + if (trueState.bStack.length !== falseState.bStack.length) { + throw new Error( + `EXEC.IF: BOOLEAN stack depth mismatch — ` + + `true branch produces ${trueState.bStack.length} values, ` + + `false branch produces ${falseState.bStack.length}` + ); + } + // --- Compare dStacks elementwise --- - const maxDLen = Math.max(trueState.dStack.length, falseState.dStack.length); + const maxDLen = trueState.dStack.length; // both are equal after validation // result var for each position that changed or is new const dResultMap = new Map(); // position → result var name for (let k = 0; k < maxDLen; k++) { @@ -272,7 +288,7 @@ function processExecIf( } // --- Compare bStacks elementwise --- - const maxBLen = Math.max(trueState.bStack.length, falseState.bStack.length); + const maxBLen = trueState.bStack.length; // both are equal after validation const bResultMap = new Map(); for (let k = 0; k < maxBLen; k++) { const tv = trueState.bStack[k]; @@ -296,11 +312,11 @@ function processExecIf( const pad = ' '.repeat(indentLevel); const assignments: string[] = []; for (const [k, rv] of dMap) { - const val = branchState.dStack[k] ?? '0'; + const val = branchState.dStack[k]!; assignments.push(`${pad}${rv} = uint256(${val});`); } for (const [k, rv] of bMap) { - const val = branchState.bStack[k] ?? 'false'; + const val = branchState.bStack[k]!; assignments.push(`${pad}${rv} = ${val};`); } return assignments; @@ -326,12 +342,12 @@ function processExecIf( const newDStack: string[] = []; for (let k = 0; k < maxDLen; k++) { const rv = dResultMap.get(k); - newDStack.push(rv ?? (dBefore[k] ?? trueState.dStack[k] ?? '0')); + newDStack.push(rv ?? (dBefore[k] ?? trueState.dStack[k]!)); } const newBStack: string[] = []; for (let k = 0; k < maxBLen; k++) { const rv = bResultMap.get(k); - newBStack.push(rv ?? (bBefore[k] ?? trueState.bStack[k] ?? 'false')); + newBStack.push(rv ?? (bBefore[k] ?? trueState.bStack[k]!)); } state.dStack = newDStack; diff --git a/tools/push3-transpiler/test/transpiler.test.ts b/tools/push3-transpiler/test/transpiler.test.ts index 4a53f44..045c272 100644 --- a/tools/push3-transpiler/test/transpiler.test.ts +++ b/tools/push3-transpiler/test/transpiler.test.ts @@ -188,8 +188,7 @@ describe('EXEC.IF balanced branches', () => { // --------------------------------------------------------------------------- describe('EXEC.IF unbalanced branches', () => { - it('true branch pushes more values than false branch', () => { - // True branch pushes 2 values, false branch pushes 1 + it('true branch pushes more DYADIC values than false branch → error', () => { const src = `( DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP @@ -198,13 +197,10 @@ describe('EXEC.IF unbalanced branches', () => { (10 20) (10) )`; - const result = run(src); - const body = result.functionBody.join('\n'); - assert.ok(body.includes('if ('), 'should emit if/else'); - // The missing value in false branch defaults to '0' + assert.throws(() => run(src), /DYADIC stack depth mismatch/); }); - it('false branch pushes more values than true branch', () => { + it('false branch pushes more DYADIC values than true branch → error', () => { const src = `( DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP @@ -213,24 +209,33 @@ describe('EXEC.IF unbalanced branches', () => { (10) (10 20) )`; - const result = run(src); - const body = result.functionBody.join('\n'); - assert.ok(body.includes('if ('), 'should emit if/else'); + assert.throws(() => run(src), /DYADIC stack depth mismatch/); }); - it('one branch is empty (no-op)', () => { + it('error message includes both branch depths', () => { + const src = `( + DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP + DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP + 100 50 DYADIC.> + EXEC.IF + (10 20 30) + (10) + )`; + assert.throws(() => run(src), /true branch produces 3.*false branch produces 1/); + }); + + it('one branch pushes extra, the other is no-op → depth mismatch error', () => { + // True branch pushes 2 extra values, false branch leaves stack unchanged const src = `( DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP 50 100 50 DYADIC.> EXEC.IF - (DYADIC.POP 99) + (99 88) () )`; - const result = run(src); - const body = result.functionBody.join('\n'); - assert.ok(body.includes('if ('), 'should emit if/else'); + assert.throws(() => run(src), /DYADIC stack depth mismatch/); }); it('EXEC.IF with missing branches throws', () => { @@ -240,6 +245,20 @@ describe('EXEC.IF unbalanced branches', () => { )`; assert.throws(() => run(src), /EXEC\.IF: missing branches/); }); + + it('BOOLEAN stack depth mismatch between branches → error', () => { + // true branch pushes a boolean comparison, false branch does not + const src = `( + DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP + DYADIC.POP DYADIC.POP DYADIC.POP DYADIC.POP + 10 20 30 40 + TRUE + EXEC.IF + (10 5 DYADIC.>) + () + )`; + assert.throws(() => run(src), /BOOLEAN stack depth mismatch/); + }); }); // ---------------------------------------------------------------------------