Support for with empty update expression

Adds support for `for` statements with an empty or unreachable update 
expression. In both cases, reversePostorderBlocks() will remove the 
empty/unreachable update block, leaving the ForTerminal.update pointing to a 
non-existent block. We explicitly rewrite this (much like we null out 
unreachable fallthroughs after shrink). When transforming to ReactiveFunction, 
we emit the update block as null if it was the same as the test block.

compiler/forget/src/HIR/BuildHIR.ts

@@ -307,35 +307,35 @@ function lowerStatement(
         };
       });
 
-      const updateBlock = builder.enter("loop", (_blockId) => {
-        const update = stmt.get("update");
-        if (update.node == null) {
-          builder.errors.push({
-            reason: `(BuildHIR::lowerStatement) Handle empty update in ForStatement`,
-            severity: ErrorSeverity.Todo,
-            nodePath: stmt,
-          });
-          return { kind: "unsupported", id: makeInstructionId(0) };
-        }
-        lowerExpressionToTemporary(builder, update as NodePath<t.Expression>);
-        return {
-          kind: "goto",
-          block: testBlock.id,
-          variant: GotoVariant.Break,
-          id: makeInstructionId(0),
-        };
-      });
-
-      const bodyBlock = builder.enter("block", (_blockId) => {
-        return builder.loop(label, updateBlock, continuationBlock.id, () => {
-          lowerStatement(builder, stmt.get("body"));
+      let updateBlock: BlockId | null = null;
+      const update = stmt.get("update");
+      if (update.node != null) {
+        updateBlock = builder.enter("loop", (_blockId) => {
+          lowerExpressionToTemporary(builder, update as NodePath<t.Expression>);
           return {
             kind: "goto",
-            block: updateBlock,
-            variant: GotoVariant.Continue,
+            block: testBlock.id,
+            variant: GotoVariant.Break,
             id: makeInstructionId(0),
           };
         });
+      }
+
+      const bodyBlock = builder.enter("block", (_blockId) => {
+        return builder.loop(
+          label,
+          updateBlock ?? testBlock.id,
+          continuationBlock.id,
+          () => {
+            lowerStatement(builder, stmt.get("body"));
+            return {
+              kind: "goto",
+              block: updateBlock ?? testBlock.id,
+              variant: GotoVariant.Continue,
+              id: makeInstructionId(0),
+            };
+          }
+        );
       });
 
       builder.terminateWithContinuation(

compiler/forget/src/HIR/HIR.ts

@@ -347,7 +347,7 @@ export type ForTerminal = {
   loc: SourceLocation;
   init: BlockId;
   test: BlockId;
-  update: BlockId;
+  update: BlockId | null;
   loop: BlockId;
   fallthrough: BlockId;
   id: InstructionId;

compiler/forget/src/HIR/HIRBuilder.ts

@@ -282,6 +282,7 @@ export default class HIRBuilder {
     logHIR("Build (shrunk)", ir);
     // then convert to reverse postorder
     reversePostorderBlocks(ir);
+    removeUnreachableForUpdates(ir);
     removeUnreachableFallthroughs(ir);
     removeDeadDoWhileStatements(ir);
     markInstructionIds(ir);
@@ -528,6 +529,18 @@ export function shrink(func: HIR): void {
   }
 }
 
+export function removeUnreachableForUpdates(fn: HIR): void {
+  for (const [, block] of fn.blocks) {
+    if (
+      block.terminal.kind === "for" &&
+      block.terminal.update !== null &&
+      !fn.blocks.has(block.terminal.update)
+    ) {
+      block.terminal.update = null;
+    }
+  }
+}
+
 export function removeUnreachableFallthroughs(func: HIR): void {
   const visited: Set<BlockId> = new Set();
   for (const [_, block] of func.blocks) {

compiler/forget/src/HIR/visitors.ts

@@ -632,7 +632,7 @@ export function mapTerminalSuccessors(
     case "for": {
       const init = fn(terminal.init);
       const test = fn(terminal.test);
-      const update = fn(terminal.update);
+      const update = terminal.update !== null ? fn(terminal.update) : null;
       const loop = fn(terminal.loop);
       const fallthrough = fn(terminal.fallthrough);
       return {

compiler/forget/src/Optimization/ConstantPropagation.ts

@@ -24,7 +24,10 @@ import {
   validateConsistentIdentifiers,
   validateTerminalSuccessors,
 } from "../HIR";
-import { removeDeadDoWhileStatements } from "../HIR/HIRBuilder";
+import {
+  removeDeadDoWhileStatements,
+  removeUnreachableForUpdates,
+} from "../HIR/HIRBuilder";
 import { eliminateRedundantPhi } from "../SSA";
 
 /**
@@ -52,6 +55,7 @@ export function constantPropagation(fn: HIRFunction): void {
     shrink(fn.body);
     reversePostorderBlocks(fn.body);
     removeUnreachableFallthroughs(fn.body);
+    removeUnreachableForUpdates(fn.body);
     removeDeadDoWhileStatements(fn.body);
     markInstructionIds(fn.body);
     markPredecessors(fn.body);

compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts

@@ -347,7 +347,7 @@ class Driver {
 
         const scheduleId = this.cx.scheduleLoop(
           terminal.fallthrough,
-          terminal.update,
+          terminal.update ?? terminal.test,
           terminal.loop
         );
         scheduleIds.push(scheduleId);
@@ -382,10 +382,10 @@ class Driver {
           terminal.loc
         ).value;
 
-        const updateValue = this.visitValueBlock(
-          terminal.update,
-          terminal.loc
-        ).value;
+        const updateValue =
+          terminal.update !== null
+            ? this.visitValueBlock(terminal.update, terminal.loc).value
+            : null;
 
         let loopBody: ReactiveBlock;
         if (loopId) {

compiler/forget/src/SSA/LeaveSSA.ts

@@ -335,7 +335,7 @@ export function leaveSSA(fn: HIRFunction): void {
         }
       }
 
-      if (terminal.kind === "for") {
+      if (terminal.kind === "for" && terminal.update !== null) {
         const update = fn.body.blocks.get(terminal.update)!;
         pushPhis(update);
       }

compiler/forget/src/Utils/VisualizeHIRMermaid.ts

@@ -205,7 +205,9 @@ function printTerminalArrows(blockId: BlockId, terminal: Terminal): string {
     case "for": {
       buffer.push(printJumpArrow(blockId, terminal.init, "init"));
       buffer.push(printJumpArrow(blockId, terminal.test, "test"));
-      buffer.push(printJumpArrow(blockId, terminal.update, "update"));
+      if (terminal.update !== null) {
+        buffer.push(printJumpArrow(blockId, terminal.update, "update"));
+      }
       buffer.push(printJumpArrow(blockId, terminal.loop, "loop"));
       buffer.push(printJumpArrow(blockId, terminal.fallthrough, "fallthrough"));
       break;

compiler/forget/src/__tests__/fixtures/compiler/error.for-return.expect.md

@@ -1,20 +0,0 @@
-
-## Input
-
-```javascript
-function Component(props) {
-  for (let i = 0; i < props.count; i++) {
-    return;
-  }
-}
-
-```
-
-
-## Error
-
-```
-[ReactForget] Invariant: Block bb4 does not exist for terminal '[1] For init=bb3 test=bb1 loop=bb5 update=bb4 fallthrough=bb2' (2:4)
-```
-          
-      

compiler/forget/src/__tests__/fixtures/compiler/error.leave-ssa-handle-return-in-for.expect.md

@@ -1,20 +0,0 @@
-
-## Input
-
-```javascript
-function foo(props) {
-  for (let i = 0; i < 10; i += 1) {
-    return;
-  }
-}
-
-```
-
-
-## Error
-
-```
-Cannot read properties of undefined (reading 'phis')
-```
-          
-      

compiler/forget/src/__tests__/fixtures/compiler/error.leave-ssa-handle-return-in-for.js

@@ -1,5 +0,0 @@
-function foo(props) {
-  for (let i = 0; i < 10; i += 1) {
-    return;
-  }
-}

compiler/forget/src/__tests__/fixtures/compiler/error.todo-kitchensink.expect.md

@@ -144,15 +144,6 @@ let moduleLocal = false;
   25 |   }
   26 |   for (;;) {
 
-[ReactForget] TodoError: (BuildHIR::lowerStatement) Handle empty update in ForStatement
-  21 |     x.push(i);
-  22 |   }
-> 23 |   for (; i < 3; ) {
-     |   ^
-  24 |     break;
-  25 |   }
-  26 |   for (;;) {
-
 [ReactForget] TodoError: (BuildHIR::lowerStatement) Handle non-variable initialization in ForStatement
   24 |     break;
   25 |   }
@@ -162,15 +153,6 @@ let moduleLocal = false;
   28 |   }
   29 |
 
-[ReactForget] TodoError: (BuildHIR::lowerStatement) Handle empty update in ForStatement
-  24 |     break;
-  25 |   }
-> 26 |   for (;;) {
-     |   ^
-  27 |     break;
-  28 |   }
-  29 |
-
 [ReactForget] TodoError: (BuildHIR::lowerStatement) Handle empty test in ForStatement
   24 |     break;
   25 |   }

compiler/forget/src/__tests__/fixtures/compiler/for-empty-update-with-continue.expect.md

@@ -0,0 +1,30 @@
+
+## Input
+
+```javascript
+function Component(props) {
+  let x = 0;
+  for (let i = 0; i < props.count; ) {
+    x += i;
+    i += 1;
+    continue;
+  }
+  return x;
+}
+
+```
+
+## Code
+
+```javascript
+function Component(props) {
+  let x = 0;
+  for (let i = 0; i < props.count; ) {
+    x = x + i;
+    i = i + 1;
+  }
+  return x;
+}
+
+```
+      

compiler/forget/src/__tests__/fixtures/compiler/for-empty-update-with-continue.js

@@ -0,0 +1,9 @@
+function Component(props) {
+  let x = 0;
+  for (let i = 0; i < props.count; ) {
+    x += i;
+    i += 1;
+    continue;
+  }
+  return x;
+}

compiler/forget/src/__tests__/fixtures/compiler/for-empty-update.expect.md

@@ -0,0 +1,33 @@
+
+## Input
+
+```javascript
+function Component(props) {
+  let x = 0;
+  for (let i = 0; i < props.count; ) {
+    x += i;
+    if (x > 10) {
+      break;
+    }
+  }
+  return x;
+}
+
+```
+
+## Code
+
+```javascript
+function Component(props) {
+  let x = 0;
+  for (const i = 0; 0 < props.count; ) {
+    x = x + 0;
+    if (x > 10) {
+      break;
+    }
+  }
+  return x;
+}
+
+```
+      

compiler/forget/src/__tests__/fixtures/compiler/for-empty-update.js

@@ -0,0 +1,10 @@
+function Component(props) {
+  let x = 0;
+  for (let i = 0; i < props.count; ) {
+    x += i;
+    if (x > 10) {
+      break;
+    }
+  }
+  return x;
+}

compiler/forget/src/__tests__/fixtures/compiler/for-return.expect.md

@@ -0,0 +1,23 @@
+
+## Input
+
+```javascript
+function Component(props) {
+  for (let i = 0; i < props.count; i++) {
+    return;
+  }
+}
+
+```
+
+## Code
+
+```javascript
+function Component(props) {
+  for (const i = 0; 0 < props.count; ) {
+    return;
+  }
+}
+
+```
+