fix: fix init-reassign for-loop bug

packages/cashc/src/generation/GenerateTargetTraversal.ts

@@ -751,7 +751,13 @@ export default class GenerateTargetTraversal extends AstTraversal {
 
   visitFor(node: ForNode): Node {
     const forScopeStackDepth = this.stack.length;
+
+    // If the init is a re-assignment, we need to increment the scope depth so it gets treated as a scoped reassignment
+    // (and therefore the stack value gets properly replaced)
+    const isReassignment = node.init instanceof AssignNode;
+    if (isReassignment) this.scopeDepth += 1;
     node.init = this.visit(node.init) as VariableDefinitionNode | AssignNode;
+    if (isReassignment) this.scopeDepth -= 1;
 
     this.scopeDepth += 1;
     this.emit(Op.OP_BEGIN, { location: node.location, positionHint: PositionHint.START });

packages/cashc/test/generation/fixtures/valid-contract-files/for_loop_reassigned_init.ts

@@ -0,0 +1,39 @@
+import { Fixture } from '../../fixture-utils.js';
+
+export const fixtures: Fixture[] = [
+  {
+    // A for-loop init that reassigns an existing variable replaces that variable's stack slot, so the loop's
+    // final value is what the require after the loop reads (the loop's cleanup does not drop it)
+    artifact: {
+      contractName: 'ForLoopReassignedInit',
+      constructorInputs: [],
+      abi: [{ name: 'spend', inputs: [{ name: 'n', type: 'int' }] }],
+      bytecode:
+        // int i = 0;
+        'OP_0 '
+        // for (i = 0; i < n; i++) {
+        + 'OP_DROP OP_0 OP_BEGIN OP_2DUP OP_GREATERTHAN OP_DUP OP_TOALTSTACK OP_IF '
+        // require(tx.outputs[i].value >= 1000);
+        + 'OP_DUP OP_OUTPUTVALUE e803 OP_GREATERTHANOREQUAL OP_VERIFY '
+        // For loop update
+        + 'OP_1ADD '
+        // Loop condition
+        + 'OP_ENDIF OP_FROMALTSTACK OP_NOT OP_UNTIL '
+        // require(i == tx.outputs.length);
+        + 'OP_TXOUTPUTCOUNT OP_NUMEQUAL '
+        // Cleanup
+        + 'OP_NIP',
+      fingerprint: 'efe2107d45ab15018a2d5a2bc2cb0311d86ec7c8ba5e01c8503f25619ecec621',
+      debug: {
+        bytecode: '007500656ea0766b6376cc02e803a2698b686c9166c49c77',
+        sourceMap: '3:16:3:17;5:13:5:18:1;;:8:7:9:0;:20:5:25;::::1;;;:32:7:9:0;6:31:6:32;:20::39:1;:43::47:0;:20:::1;:12::49;5:27:5:30;:32:7:9;;:8;;9:21:9:38:0;:8::40:1;2:26:10:5',
+        logs: [],
+        requires: [
+          { ip: 13, line: 6 },
+          { ip: 21, line: 9 },
+        ],
+        sourceTags: '14:14:fu;15:18:lc;21:21:sc',
+      },
+    },
+  },
+];

packages/cashc/test/generation/fixtures/valid-contract-files/global_function_for_loop_reassigned_init.ts

@@ -0,0 +1,90 @@
+import { Fixture } from '../../fixture-utils.js';
+
+export const fixtures: Fixture[] = [
+  {
+    // A global function returning a variable that a for-loop init reassigned returns the loop's final value
+    artifact: {
+      contractName: 'GlobalFunctionForLoopReassignedInit',
+      constructorInputs: [],
+      abi: [{ name: 'spend', inputs: [{ name: 'n', type: 'int' }] }],
+      bytecode:
+        // require(count(n) == n), with count(n) spliced in:
+        'OP_DUP '
+        // int i = 0;
+        + 'OP_0 '
+        // for (i = 0; i < n; i++) {
+        + 'OP_DROP OP_0 OP_BEGIN OP_2DUP OP_GREATERTHAN OP_DUP OP_TOALTSTACK OP_IF '
+        // require(i < 10);
+        + 'OP_DUP OP_10 OP_LESSTHAN OP_VERIFY '
+        // For loop update
+        + 'OP_1ADD '
+        // Loop condition
+        + 'OP_ENDIF OP_FROMALTSTACK OP_NOT OP_UNTIL '
+        // return i;
+        + 'OP_NIP '
+        // == n
+        + 'OP_NUMEQUAL',
+      fingerprint: '9643acfe0e75c3bfacc39e2ff86267dc6d707f341818b2c3a83795038141d11e',
+      debug: {
+        bytecode: '76007500656ea0766b63765a9f698b686c9166779c',
+        sourceMap: '13:22:13:23;:16::24:1;;;;;;;;;;;;;;;;;;;:8::31',
+        logs: [],
+        requires: [
+          { ip: 13, line: 13 },
+          { ip: 21, line: 13 },
+        ],
+        sourceTags: '14:14:fu;15:18:lc;19:19:sc',
+        functions: [
+          {
+            name: 'count',
+            inputs: [{ name: 'n', type: 'int' }],
+            bytecode: '007500656ea0766b63765a9f698b686c916677',
+            sourceMap: '2:12:2:13;4:9:4:14:1;;:4:6:5:0;:16:4:21;::::1;;;:28:6:5:0;5:16:5:17;:20::22;:16:::1;:8::24;4:23:4:26;:28:6:5;;:4;;1:36:9:1',
+            sourceTags: '13:13:fu;14:17:lc;18:18:sc',
+            logs: [],
+            requires: [
+              { ip: 12, line: 5 },
+            ],
+          },
+        ],
+        inlineRanges: '1:19:count',
+      },
+    },
+  },
+  {
+    compilerOptions: { disableInlining: true },
+    artifact: {
+      contractName: 'GlobalFunctionForLoopReassignedInit',
+      constructorInputs: [],
+      abi: [{ name: 'spend', inputs: [{ name: 'n', type: 'int' }] }],
+      bytecode:
+        // OP_DEFINE count (id 0): the same body as the inlined variant, ending in return i (OP_NIP)
+        '007500656ea0766b63765a9f698b686c916677 OP_0 OP_DEFINE '
+        // require(count(n) == n)
+        + 'OP_DUP OP_0 OP_INVOKE OP_NUMEQUAL',
+      fingerprint: '33bcf111dab3f5359532cf1f0d54027b49f5f7a708614569d905143b953894f4',
+      debug: {
+        bytecode: '13007500656ea0766b63765a9f698b686c916677008976008a9c',
+        sourceMap: '1::9:1;;::::1;13:22:13:23:0;:16::24:1;;:8::31',
+        logs: [],
+        requires: [
+          { ip: 7, line: 13 },
+        ],
+        functions: [
+          {
+            id: 0,
+            name: 'count',
+            inputs: [{ name: 'n', type: 'int' }],
+            bytecode: '007500656ea0766b63765a9f698b686c916677',
+            sourceMap: '2:12:2:13;4:9:4:14:1;;:4:6:5:0;:16:4:21;::::1;;;:28:6:5:0;5:16:5:17;:20::22;:16:::1;:8::24;4:23:4:26;:28:6:5;;:4;;1:36:9:1',
+            sourceTags: '13:13:fu;14:17:lc;18:18:sc',
+            logs: [],
+            requires: [
+              { ip: 12, line: 5 },
+            ],
+          },
+        ],
+      },
+    },
+  },
+];

packages/cashc/test/valid-contract-files/for_loop_reassigned_init.cash

@@ -0,0 +1,11 @@
+contract ForLoopReassignedInit() {
+    function spend(int n) {
+        int i = 0;
+
+        for (i = 0; i < n; i++) {
+            require(tx.outputs[i].value >= 1000);
+        }
+
+        require(i == tx.outputs.length);
+    }
+}

packages/cashc/test/valid-contract-files/global_function_for_loop_reassigned_init.cash

@@ -0,0 +1,15 @@
+function count(int n) returns (int) {
+    int i = 0;
+
+    for (i = 0; i < n; i++) {
+        require(i < 10);
+    }
+
+    return i;
+}
+
+contract GlobalFunctionForLoopReassignedInit() {
+    function spend(int n) {
+        require(count(n) == n);
+    }
+}

website/docs/releases/release-notes.md

@@ -18,6 +18,7 @@ This release contains several breaking changes, please refer to the [migration n
 - :bug: Fix bug where date literal parsing was different per locale, it now uses UTC.
 - :bug: Fix bug where `LockingBytecodeNullData` used an incorrect push opcode for empty chunks and chunks of 128-255 bytes, so it did not match the SDK's `addOpReturnOutput()`. Chunks with a value or length that is known at compile time now use a precomputed push opcode, which makes them smaller. This changes the bytecode of all contracts that use `LockingBytecodeNullData`.
 - :bug: Fix bug where hex literals with an odd number of digits (e.g. `0x123`) compiled to a different value (`0x1203`), they now cause a compile error.
+- :bug: Fix bug where a `for` loop whose init reassigns an existing variable did not update the stack value after the loop.
 - :racehorse: Add new optimisations for loop counter updates, reassignments, scope cleanup, stack shuffles, and order-independent operations.
 - :racehorse: Greatly improve compiler speed for very large contracts.