Support assignment in for statement's update clause

Per the title, this PR adds support for assignment expressions in update 
clauses. This was mostly fixed by the previous diff to improve value block 
handling, and there's only a bit more to do here to allow a "value block" that 
doesn't produce a value (we need a better name).

compiler/forget/src/HIR/BuildHIR.ts

@@ -340,6 +340,7 @@ function lowerStatement(
 
       const updateBlock = builder.enter((blockId) => {
         const update = stmt.get("update");
+        todoInvariant(update.hasNode(), "Handle empty for updater");
         if (update.hasNode()) {
           lowerExpressionToVoid(builder, update);
         }
@@ -1275,20 +1276,21 @@ function lowerExpressionToPlace(
   return place;
 }
 
+/**
+ * Lowers an expression to an instruction with no lvalue
+ */
 function lowerExpressionToVoid(
   builder: HIRBuilder,
   exprPath: NodePath<t.Expression>
 ): void {
   const instr = lowerExpression(builder, exprPath);
-  if (instr.kind !== "Identifier") {
-    const exprLoc = exprPath.node.loc ?? GeneratedSource;
-    builder.push({
-      id: makeInstructionId(0),
-      value: instr,
-      loc: exprLoc,
-      lvalue: null,
-    });
-  }
+  const exprLoc = exprPath.node.loc ?? GeneratedSource;
+  builder.push({
+    id: makeInstructionId(0),
+    value: instr,
+    loc: exprLoc,
+    lvalue: null,
+  });
 }
 
 function lowerLVal(builder: HIRBuilder, exprPath: NodePath<t.LVal>): Place {

compiler/forget/src/HIR/Codegen.ts

@@ -137,9 +137,13 @@ class CodegenVisitor
   appendValueBlock(block: t.Statement[], item: t.Statement): void {
     this.appendBlock(block, item);
   }
-  leaveValueBlock(block: t.Statement[], place: t.Expression): t.Expression {
+  leaveValueBlock(
+    block: t.Statement[],
+    place: t.Expression | null
+  ): t.Expression {
     this.depth--;
     if (block.length === 0) {
+      invariant(place !== null, "Unexpected empty value block");
       return place;
     }
     const expressions = block.map((stmt) => {
@@ -153,7 +157,9 @@ class CodegenVisitor
           );
       }
     });
-    expressions.push(place);
+    if (place !== null) {
+      expressions.push(place);
+    }
     return t.sequenceExpression(expressions);
   }
 

compiler/forget/src/HIR/HIRTreeVisitor.ts

@@ -451,24 +451,27 @@ class Driver<TBlock, TInit, TValueBlock, TValue, TStatement, TCase> {
   ): TValue {
     const valueBlock = this.visitor.enterValueBlock(parent);
     const instructions = [...block.instructions];
-    let lastValue: { value: InstructionValue; id: InstructionId };
+    let lastValue: { value: InstructionValue; id: InstructionId } | null = null;
     if (terminalValue != null) {
       lastValue = terminalValue;
     } else {
-      invariant(instructions.length > 0, "Value block may not be empty");
-      const last = instructions.pop()!;
-      invariant(
-        last.lvalue === null,
-        "Expected value block to end in a value, not an assignment"
-      );
-      lastValue = { value: last.value, id: last.id };
+      if (
+        instructions.length &&
+        instructions[instructions.length - 1].lvalue === null
+      ) {
+        const last = instructions.pop()!;
+        lastValue = { value: last.value, id: last.id };
+      }
     }
     for (const instr of instructions) {
       const value = this.visitor.visitValue(instr.value, instr.id);
       const item = this.visitor.visitInstruction(instr, value);
       this.visitor.appendValueBlock(valueBlock, item);
     }
-    const value = this.visitor.visitValue(lastValue.value, lastValue.id);
+    const value =
+      lastValue !== null
+        ? this.visitor.visitValue(lastValue.value, lastValue.id)
+        : null;
     return this.visitor.leaveValueBlock(valueBlock, value);
   }
 
@@ -817,7 +820,7 @@ export interface Visitor<
    * Converts the visitor's value block (and final value) to the visitor's
    * value representation.
    */
-  leaveValueBlock(block: TValueBlock, value: TValue): TValue;
+  leaveValueBlock(block: TValueBlock, value: TValue | null): TValue;
 
   enterInitBlock(block: TBlock): TValueBlock;
 

compiler/forget/src/HIR/InferReactiveScopes.ts

@@ -336,7 +336,10 @@ class AlignReactiveScopesToBlockScopeRangeVisitor
 {
   // For each block scope (outer array) stores a list of ReactiveScopes that start
   // in that block scope.
-  blockScopes: Array<Array<PendingReactiveScope>> = [];
+  blockScopes: Array<{
+    kind: "block" | "value";
+    scopes: Array<PendingReactiveScope>;
+  }> = [];
 
   // ReactiveScopes whose declaring block scope has ended but may still need to
   // be "closed" (ie have their range.end be updated). A given scope can be in
@@ -349,7 +352,10 @@ class AlignReactiveScopesToBlockScopeRangeVisitor
 
   visitId(id: InstructionId) {
     const currentScopes = this.blockScopes[this.blockScopes.length - 1]!;
-    const scopes = [...currentScopes, ...this.unclosedScopes];
+    if (currentScopes.kind === "value") {
+      return;
+    }
+    const scopes = [...currentScopes.scopes, ...this.unclosedScopes];
     for (const pending of scopes) {
       if (!pending.active) {
         continue;
@@ -362,7 +368,7 @@ class AlignReactiveScopesToBlockScopeRangeVisitor
   }
 
   enterBlock(): void {
-    this.blockScopes.push([]);
+    this.blockScopes.push({ kind: "block", scopes: [] });
   }
 
   appendBlock(block: void, item: void, label?: BlockId | undefined): void {}
@@ -370,10 +376,10 @@ class AlignReactiveScopesToBlockScopeRangeVisitor
   leaveBlock(block: void): void {
     const lastScope = this.blockScopes.pop();
     invariant(
-      lastScope !== undefined,
+      lastScope !== undefined && lastScope.kind === "block",
       "Expected enterBlock/leaveBlock to be called 1:1"
     );
-    for (const scope of lastScope) {
+    for (const scope of lastScope.scopes) {
       if (scope.active) {
         this.unclosedScopes.push(scope);
       }
@@ -381,19 +387,30 @@ class AlignReactiveScopesToBlockScopeRangeVisitor
   }
 
   enterValueBlock(): void {
-    this.enterBlock();
+    this.blockScopes.push({ kind: "value", scopes: [] });
   }
   appendValueBlock(block: void, item: void): void {}
   leaveValueBlock(block: void, value: void): void {
-    this.leaveBlock(block);
+    const lastScope = this.blockScopes.pop();
+    invariant(
+      lastScope !== undefined && lastScope.kind === "value",
+      "Expected enterValueBlock/leaveValueBlock to be called 1:1"
+    );
+    for (const scope of lastScope.scopes) {
+      invariant(
+        scope.active,
+        "Value scopes cannot be closed separately from the parent block"
+      );
+      this.unclosedScopes.push(scope);
+    }
   }
 
   enterInitBlock(block: void): void {
-    this.enterBlock();
+    this.enterValueBlock();
   }
   appendInitBlock(block: void, item: void): void {}
   leaveInitBlock(block: void): void {
-    this.leaveBlock(block);
+    this.leaveValueBlock(block);
   }
 
   visitInstruction(instruction: Instruction, value: void): void {
@@ -403,7 +420,7 @@ class AlignReactiveScopesToBlockScopeRangeVisitor
       if (!this.seenScopes.has(scope.id)) {
         const currentScopes = this.blockScopes[this.blockScopes.length - 1]!;
         this.seenScopes.add(scope.id);
-        currentScopes.push({
+        currentScopes.scopes.push({
           active: true,
           scope,
         });

compiler/forget/src/HIR/LeaveSSA.ts

@@ -62,8 +62,17 @@ export function leaveSSA(fn: HIRFunction) {
       phis.push(...loop.phis);
     }
     if (terminal.kind === "for") {
+      const init = fn.body.blocks.get(terminal.init)!;
+      phis.push(...init.phis);
       const update = fn.body.blocks.get(terminal.update)!;
       phis.push(...update.phis);
+
+      // find declarations in the for init
+      for (const instr of init.instructions) {
+        if (instr.lvalue !== null && instr.lvalue.place.memberPath === null) {
+          hasDeclaration.add(instr.lvalue.place.identifier);
+        }
+      }
     }
 
     // For each phi, determine a canonical identifier to use for versions of the variable

compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.expect.md

@@ -0,0 +1,113 @@
+
+## Input
+
+```javascript
+function foo() {
+  let x = 1;
+  for (let i = 0; i < 10; /* update is intentally a single identifier */ i) {
+    x += 1;
+  }
+  return x;
+}
+
+```
+
+## HIR
+
+```
+bb0:
+  [1] Let mutate x$6_@1[1:13] = 1
+  [2] For init=bb3 test=bb1 loop=bb5 update=bb4 fallthrough=bb2
+bb3:
+  predecessor blocks: bb0
+  [3] Const mutate i$7_@1[1:13] = 0
+  [4] Goto bb1
+bb1:
+  predecessor blocks: bb3 bb4
+  [5] Const mutate $8_@1[1:13] = 10
+  [6] Const mutate $10_@3[6:8] = Binary read i$7_@1 < read $8_@1
+  [7] If (read $10_@3) then:bb5 else:bb2 fallthrough=bb2
+bb5:
+  predecessor blocks: bb1
+  [8] Const mutate $11_@4 = 1
+  [9] Reassign mutate x$6_@1[1:13] = Binary read x$6_@1 + read $11_@4
+  [10] Goto(Continue) bb4
+bb4:
+  predecessor blocks: bb5
+  [11] read i$7_@1
+  [12] Goto bb1
+bb2:
+  predecessor blocks: bb1
+  [13] Return read x$6_@1
+
+```
+
+### CFG
+
+```mermaid
+flowchart TB
+  %% Basic Blocks
+  subgraph bb0
+    bb0_instrs["
+      [1] Let mutate x$6_@1[1:13] = 1
+    "]
+    bb0_instrs --> bb0_terminal(["For"])
+  end
+  subgraph bb3
+    bb3_instrs["
+      [3] Const mutate i$7_@1[1:13] = 0
+    "]
+    bb3_instrs --> bb3_terminal(["Goto"])
+  end
+  subgraph bb1
+    bb1_instrs["
+      [5] Const mutate $8_@1[1:13] = 10
+      [6] Const mutate $10_@3[6:8] = Binary read i$7_@1 < read $8_@1
+    "]
+    bb1_instrs --> bb1_terminal(["If (read $10_@3)"])
+  end
+  subgraph bb5
+    bb5_instrs["
+      [8] Const mutate $11_@4 = 1
+      [9] Reassign mutate x$6_@1[1:13] = Binary read x$6_@1 + read $11_@4
+    "]
+    bb5_instrs --> bb5_terminal(["Goto"])
+  end
+  subgraph bb4
+    bb4_instrs["
+      [11] read i$7_@1
+    "]
+    bb4_instrs --> bb4_terminal(["Goto"])
+  end
+  subgraph bb2
+    bb2_terminal(["Return read x$6_@1"])
+  end
+
+  %% Jumps
+  bb0_terminal -- "init" --> bb3
+  bb0_terminal -- "test" --> bb1
+  bb0_terminal -- "update" --> bb4
+  bb0_terminal -- "loop" --> bb5
+  bb0_terminal -- "fallthrough" --> bb2
+  bb3_terminal --> bb1
+  bb1_terminal -- "then" --> bb5
+  bb1_terminal -- "else" --> bb2
+  bb5_terminal --> bb4
+  bb4_terminal --> bb1
+
+```
+
+## Code
+
+```javascript
+function foo$0() {
+  let x$6 = 1;
+  bb2: for (const i$7 = 0; i$7 < 10; i$7) {
+    x$6 = x$6 + 1;
+  }
+
+  return x$6;
+}
+
+```
+      

compiler/forget/src/__tests__/fixtures/hir/ssa-for-trivial-update.js

@@ -0,0 +1,7 @@
+function foo() {
+  let x = 1;
+  for (let i = 0; i < 10; /* update is intentally a single identifier */ i) {
+    x += 1;
+  }
+  return x;
+}

compiler/forget/src/__tests__/fixtures/hir/ssa-for.expect.md

@@ -4,7 +4,7 @@
 ```javascript
 function foo() {
   let x = 1;
-  for (let i = 0; i < 10; update()) {
+  for (let i = 0; i < 10; i += 1) {
     x += 1;
   }
   return x;
@@ -16,32 +16,32 @@ function foo() {
 
 ```
 bb0:
-  [1] Let mutate x$7_@0[1:13] = 1
+  [1] Let mutate x$7_@1[1:15] = 1
   [2] For init=bb3 test=bb1 loop=bb5 update=bb4 fallthrough=bb2
 bb3:
   predecessor blocks: bb0
-  [3] Const mutate i$8_@1[3:5] = 0
+  [3] Let mutate i$8_@1[1:15] = 0
   [4] Goto bb1
 bb1:
   predecessor blocks: bb3 bb4
-  [5] Const mutate $9_@2 = 10
-  [6] Const mutate $11_@3[6:8] = Binary read i$8_@1 < read $9_@2
-  [7] If (read $11_@3) then:bb5 else:bb2 fallthrough=bb2
+  [5] Const mutate $9_@1[1:15] = 10
+  [6] Const mutate $11_@1[1:15] = Binary read i$8_@1 < read $9_@1
+  [7] If (read $11_@1) then:bb5 else:bb2 fallthrough=bb2
 bb5:
   predecessor blocks: bb1
-  [8] Const mutate $12_@4 = 1
-  [9] Reassign mutate x$7_@0[1:13] = Binary read x$7_@0 + read $12_@4
+  [8] Const mutate $12_@3 = 1
+  [9] Reassign mutate x$7_@1[1:15] = Binary read x$7_@1 + read $12_@3
   [10] Goto(Continue) bb4
 bb4:
   predecessor blocks: bb5
-  [11] Call mutate update$3_@5()
-  [12] Goto bb1
+  [11] Const mutate $15_@1[1:15] = 1
+  [12] Reassign mutate i$8_@1[1:15] = Binary read i$8_@1 + read $15_@1
+  [13] read i$8_@1
+  [14] Goto bb1
 bb2:
   predecessor blocks: bb1
-  [13] Return read x$7_@0
-scope3 [6:8]:
-  - dependency: read i$8_@1
-  - dependency: read $9_@2
+  [15] Return read x$7_@1
+
 ```
 
 ### CFG
@@ -51,38 +51,40 @@ flowchart TB
   %% Basic Blocks
   subgraph bb0
     bb0_instrs["
-      [1] Let mutate x$7_@0[1:13] = 1
+      [1] Let mutate x$7_@1[1:15] = 1
     "]
     bb0_instrs --> bb0_terminal(["For"])
   end
   subgraph bb3
     bb3_instrs["
-      [3] Const mutate i$8_@1[3:5] = 0
+      [3] Let mutate i$8_@1[1:15] = 0
     "]
     bb3_instrs --> bb3_terminal(["Goto"])
   end
   subgraph bb1
     bb1_instrs["
-      [5] Const mutate $9_@2 = 10
-      [6] Const mutate $11_@3[6:8] = Binary read i$8_@1 < read $9_@2
+      [5] Const mutate $9_@1[1:15] = 10
+      [6] Const mutate $11_@1[1:15] = Binary read i$8_@1 < read $9_@1
     "]
-    bb1_instrs --> bb1_terminal(["If (read $11_@3)"])
+    bb1_instrs --> bb1_terminal(["If (read $11_@1)"])
   end
   subgraph bb5
     bb5_instrs["
-      [8] Const mutate $12_@4 = 1
-      [9] Reassign mutate x$7_@0[1:13] = Binary read x$7_@0 + read $12_@4
+      [8] Const mutate $12_@3 = 1
+      [9] Reassign mutate x$7_@1[1:15] = Binary read x$7_@1 + read $12_@3
     "]
     bb5_instrs --> bb5_terminal(["Goto"])
   end
   subgraph bb4
     bb4_instrs["
-      [11] Call mutate update$3_@5()
+      [11] Const mutate $15_@1[1:15] = 1
+      [12] Reassign mutate i$8_@1[1:15] = Binary read i$8_@1 + read $15_@1
+      [13] read i$8_@1
     "]
     bb4_instrs --> bb4_terminal(["Goto"])
   end
   subgraph bb2
-    bb2_terminal(["Return read x$7_@0"])
+    bb2_terminal(["Return read x$7_@1"])
   end
 
   %% Jumps
@@ -104,7 +106,7 @@ flowchart TB
 ```javascript
 function foo$0() {
   let x$7 = 1;
-  bb2: for (const i$8 = 0; i$8 < 10; update$3()) {
+  bb2: for (let i$8 = 0; i$8 < 10; i$8 = i$8 + 1, i$8) {
     x$7 = x$7 + 1;
   }
 

compiler/forget/src/__tests__/fixtures/hir/ssa-for.js

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

compiler/forget/src/__tests__/fixtures/hir/ssa-while-no-reassign.expect.md

@@ -21,7 +21,7 @@ bb0:
   [2] While test=bb1 loop=bb3 fallthrough=bb2
 bb1:
   predecessor blocks: bb0 bb3
-  [3] Const mutate $6_@1 = 10
+  [3] Const mutate $6_@1[3:6] = 10
   [4] Const mutate $8_@2[4:6] = Binary read x$5_@0 < read $6_@1
   [5] If (read $8_@2) then:bb3 else:bb2 fallthrough=bb2
 bb3:
@@ -34,7 +34,6 @@ bb2:
   [9] Return read x$5_@0
 scope2 [4:6]:
   - dependency: read x$5_@0
-  - dependency: read $6_@1
 scope3 [6:7]:
   - dependency: read x$5_@0
 ```
@@ -52,7 +51,7 @@ flowchart TB
   end
   subgraph bb1
     bb1_instrs["
-      [3] Const mutate $6_@1 = 10
+      [3] Const mutate $6_@1[3:6] = 10
       [4] Const mutate $8_@2[4:6] = Binary read x$5_@0 < read $6_@1
     "]
     bb1_instrs --> bb1_terminal(["If (read $8_@2)"])

compiler/forget/src/__tests__/fixtures/hir/ssa-while.expect.md

@@ -21,8 +21,8 @@ bb0:
   [2] While test=bb1 loop=bb3 fallthrough=bb2
 bb1:
   predecessor blocks: bb0 bb3
-  [3] Const mutate $6_@1 = 10
-  [4] Const mutate $8_@0[1:9] = Binary read x$5_@0 < read $6_@1
+  [3] Const mutate $6_@0[1:9] = 10
+  [4] Const mutate $8_@0[1:9] = Binary read x$5_@0 < read $6_@0
   [5] If (read $8_@0) then:bb3 else:bb2 fallthrough=bb2
 bb3:
   predecessor blocks: bb1
@@ -48,8 +48,8 @@ flowchart TB
   end
   subgraph bb1
     bb1_instrs["
-      [3] Const mutate $6_@1 = 10
-      [4] Const mutate $8_@0[1:9] = Binary read x$5_@0 < read $6_@1
+      [3] Const mutate $6_@0[1:9] = 10
+      [4] Const mutate $8_@0[1:9] = Binary read x$5_@0 < read $6_@0
     "]
     bb1_instrs --> bb1_terminal(["If (read $8_@0)"])
   end