Support hoisting const variable declarations

This PR adds preliminary support for hoisting const variable declarations. We do this via BuildHIR when lowering top level statements in a BlockStatement, by first checking which bindings are in scope to be hoistable if referenced before they are declared. The declarations are then hoisted to their earliest point where they are referenced (ie the top level statement just before) as context variables. Later, prior to codegen, we restore the original source by removing the DeclareContexts and transforming their associated StoreContexts back. Support for hoisting other kinds of declarations will come in future PRs!
2025-11-01 09:12:30 +00:00 · 2023-09-21 17:00:04 -04:00
parent 36f979d9d4
commit 1ce952e687
24 changed files with 650 additions and 66 deletions
@@ -46,6 +46,7 @@ import {
  promoteUsedTemporaries,
  propagateScopeDependencies,
  pruneAllReactiveScopes,
+  pruneHoistedContexts,
  pruneNonEscapingScopes,
  pruneNonReactiveDependencies,
  pruneUnusedLValues,
@@ -299,6 +300,13 @@ export function* run(
    value: reactiveFunction,
  });

+  pruneHoistedContexts(reactiveFunction);
+  yield log({
+    kind: "reactive",
+    name: "PruneHoistedContexts",
+    value: reactiveFunction,
+  });
+
  const ast = codegenReactiveFunction(reactiveFunction).unwrap();
  yield log({ kind: "ast", name: "Codegen", value: ast });

@@ -5,7 +5,7 @@
 * LICENSE file in the root directory of this source tree.
 */

-import { NodePath, Scope } from "@babel/traverse";
+import { Binding, NodePath, Scope } from "@babel/traverse";
 import * as t from "@babel/types";
 import { Expression } from "@babel/types";
 import invariant from "invariant";
@@ -298,7 +298,108 @@ function lowerStatement(
    }
    case "BlockStatement": {
      const stmt = stmtPath as NodePath<t.BlockStatement>;
-      stmt.get("body").forEach((s) => lowerStatement(builder, s));
+      const statements = stmt.get("body");
+      const hoistableBindings: Set<Binding> = new Set();
+
+      const recordDeclaration = (lval: NodePath<t.LVal>): void => {
+        // TODO: support other kinds of declarations that might need to be hoisted
+        switch (lval.type) {
+          case "Identifier": {
+            const lv = lval as NodePath<t.Identifier>;
+            const binding = stmt.scope.getBinding(lv.node.name);
+            if (binding != null) {
+              hoistableBindings.delete(binding);
+            }
+            break;
+          }
+        }
+      };
+
+      for (const [, binding] of Object.entries(stmt.scope.bindings)) {
+        // TODO: support other kinds of bindings
+        if (binding.kind === "const") {
+          if (
+            binding.path.isVariableDeclarator() &&
+            binding.path.get("id").isIdentifier()
+          ) {
+            hoistableBindings.add(binding);
+          }
+        }
+      }
+
+      for (const s of statements) {
+        const hoistableIdentifiers = new Set<NodePath<t.Identifier>>();
+        // After visiting the declaration, hoisting is no longer required
+        // TODO: support other kinds of declarations
+        if (s.isVariableDeclaration()) {
+          for (const decl of s.get("declarations")) {
+            recordDeclaration(decl.get("id"));
+          }
+        }
+
+        // If we see a hoistable identifier before its declaration, it should be hoisted just
+        // before the statement that references it
+        s.traverse({
+          Identifier(id: NodePath<t.Identifier>) {
+            const binding = stmt.scope.getBinding(id.node.name);
+            if (binding != null && hoistableBindings.has(binding)) {
+              if (
+                id.parentPath.isVariableDeclarator() ||
+                // don't hoist MemberExpr `property`s, only their `object`
+                (id.parentPath.isMemberExpression() &&
+                  id.parentPath.get("property") === id &&
+                  id.parentPath.node.computed === false)
+              ) {
+                return;
+              }
+              hoistableIdentifiers.add(id);
+            }
+          },
+        });
+
+        // Hoist declarations that need it to the earliest point where they are needed
+        for (const id of hoistableIdentifiers) {
+          const binding = stmt.scope.getBinding(id.node.name);
+          CompilerError.invariant(binding != null, {
+            reason: "Expected to find binding for hoisted identifier",
+            description: `Could not find a binding for ${id.node.name}`,
+            suggestions: null,
+            loc: id.node.loc ?? GeneratedSource,
+          });
+          if (builder.environment.isHoistedIdentifier(binding.identifier)) {
+            // Already hoisted
+            continue;
+          }
+          if (!binding.path.isVariableDeclarator()) {
+            builder.errors.push({
+              severity: ErrorSeverity.Todo,
+              reason: "Unsupported declaration type for hoisting",
+              description: `${id.parentPath.type}`,
+              suggestions: null,
+              loc: id.parentPath.node.loc ?? GeneratedSource,
+            });
+            continue;
+          }
+          const identifier = builder.resolveIdentifier(id)!;
+          const place: Place = {
+            effect: Effect.Unknown,
+            identifier,
+            kind: "Identifier",
+            loc: id.node.loc ?? GeneratedSource,
+          };
+          lowerValueToTemporary(builder, {
+            kind: "DeclareContext",
+            lvalue: {
+              kind: InstructionKind.HoistedConst,
+              place,
+            },
+            loc: id.node.loc ?? GeneratedSource,
+          });
+          builder.environment.addHoistedIdentifier(binding.identifier);
+        }
+        lowerStatement(builder, s);
+      }
+
      return;
    }
    case "BreakStatement": {
@@ -2894,13 +2995,16 @@ function lowerAssignment(
          node: lvalue.node,
        };
      }
+      const isHoistedIdentifier = builder.environment.isHoistedIdentifier(
+        lvalue.node
+      );

      let temporary;
      if (builder.isContextIdentifier(lvalue)) {
-        if (kind !== InstructionKind.Reassign) {
+        if (kind !== InstructionKind.Reassign && !isHoistedIdentifier) {
          if (kind === InstructionKind.Const) {
            builder.errors.push({
-              reason: `Invalid declaration kind (const), this variable is reassigned later`,
+              reason: `[lowerAssignment] Invalid declaration kind (const), this variable is reassigned later`,
              severity: ErrorSeverity.InvalidJS,
              loc: lvalue.node.loc ?? null,
              suggestions: null,
@@ -235,6 +235,7 @@ export class Environment {
  enableForest: boolean;

  #contextIdentifiers: Set<t.Identifier>;
+  #hoistedIdentifiers: Set<t.Identifier>;

  constructor(
    config: EnvironmentConfig | null,
@@ -289,6 +290,7 @@ export class Environment {
    this.enableForest = config?.enableForest ?? false;

    this.#contextIdentifiers = contextIdentifiers;
+    this.#hoistedIdentifiers = new Set();
  }

  get nextIdentifierId(): IdentifierId {
@@ -298,10 +300,15 @@ export class Environment {
  get nextBlockId(): BlockId {
    return makeBlockId(this.#nextBlock++);
  }
+
  isContextIdentifier(node: t.Identifier): boolean {
    return this.#contextIdentifiers.has(node);
  }

+  isHoistedIdentifier(node: t.Identifier): boolean {
+    return this.#hoistedIdentifiers.has(node);
+  }
+
  getGlobalDeclaration(name: string): Global | null {
    let resolvedGlobal: Global | null = this.#globals.get(name) ?? null;
    if (resolvedGlobal === null) {
@@ -356,6 +363,11 @@ export class Environment {
    }
    return null;
  }
+
+  addHoistedIdentifier(node: t.Identifier): void {
+    this.#contextIdentifiers.add(node);
+    this.#hoistedIdentifiers.add(node);
+  }
 }

 // From https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js#LL18C1-L23C2
@@ -586,6 +586,11 @@ export enum InstructionKind {
   * catch clause binding
   */
  Catch = "Catch",
+
+  /**
+   * hoisted const declarations
+   */
+  HoistedConst = "HoistedConst",
 }

 function _staticInvariantInstructionValueHasLocation(
@@ -641,6 +646,7 @@ export type CallExpression = {
 *
 * Operands are therefore always a Place.
 */
+
 export type InstructionValue =
  | {
      kind: "LoadLocal";
@@ -660,7 +666,7 @@ export type InstructionValue =
  | {
      kind: "DeclareContext";
      lvalue: {
-        kind: InstructionKind.Let;
+        kind: InstructionKind.Let | InstructionKind.HoistedConst;
        place: Place;
      };
      loc: SourceLocation;
@@ -602,6 +602,9 @@ export function printLValue(lval: LValue): string {
    case InstructionKind.Catch: {
      return `Catch ${lvalue}`;
    }
+    case InstructionKind.HoistedConst: {
+      return `HoistedConst ${lvalue}$`;
+    }
    default: {
      assertExhaustive(lval.kind, `Unexpected lvalue kind '${lval.kind}'`);
    }
@@ -467,6 +467,13 @@ function codegenTerminal(
            loc: iterableItem.loc,
            suggestions: null,
          });
+        case InstructionKind.HoistedConst:
+          CompilerError.invariant(false, {
+            reason: "Unexpected HoistedConst variable in for-of collection",
+            description: null,
+            loc: iterableItem.loc,
+            suggestions: null,
+          });
        default:
          assertExhaustive(
            iterableItem.value.lvalue.kind,
@@ -671,6 +678,15 @@ function codegenInstructionNullable(
      case InstructionKind.Catch: {
        return t.emptyStatement();
      }
+      case InstructionKind.HoistedConst: {
+        CompilerError.invariant(false, {
+          reason:
+            "Expected HoistedConsts to have been pruned in PruneHoistedContexts",
+          description: null,
+          loc: instr.loc,
+          suggestions: null,
+        });
+      }
      default: {
        assertExhaustive(kind, `Unexpected instruction kind '${kind}'`);
      }
@@ -0,0 +1,64 @@
+import {
+  Identifier,
+  InstructionKind,
+  ReactiveFunction,
+  ReactiveInstruction,
+  ReactiveStatement,
+} from "../HIR";
+import {
+  ReactiveFunctionTransform,
+  Transformed,
+  visitReactiveFunction,
+} from "./visitors";
+
+/**
+ * Prunes DeclareContexts lowered for HoistedConsts, and transforms any references back to its
+ * original instruction kind.
+ */
+export function pruneHoistedContexts(fn: ReactiveFunction): void {
+  const hoistedIdentifiers: HoistedIdentifiers = new Set();
+  visitReactiveFunction(fn, new Visitor(), hoistedIdentifiers);
+}
+
+type HoistedIdentifiers = Set<Identifier>;
+
+class Visitor extends ReactiveFunctionTransform<HoistedIdentifiers> {
+  override transformInstruction(
+    instruction: ReactiveInstruction,
+    state: HoistedIdentifiers
+  ): Transformed<ReactiveStatement> {
+    this.visitInstruction(instruction, state);
+    if (
+      instruction.value.kind === "DeclareContext" &&
+      instruction.value.lvalue.kind === "HoistedConst"
+    ) {
+      state.add(instruction.value.lvalue.place.identifier);
+      return { kind: "remove" };
+    }
+
+    if (
+      instruction.value.kind === "StoreContext" &&
+      state.has(instruction.value.lvalue.place.identifier)
+    ) {
+      return {
+        kind: "replace",
+        value: {
+          kind: "instruction",
+          instruction: {
+            ...instruction,
+            value: {
+              ...instruction.value,
+              lvalue: {
+                ...instruction.value.lvalue,
+                kind: InstructionKind.Const,
+              },
+              kind: "StoreLocal",
+            },
+          },
+        },
+      };
+    }
+
+    return { kind: "keep" };
+  }
+}
@@ -22,6 +22,7 @@ export { printReactiveFunction } from "./PrintReactiveFunction";
 export { promoteUsedTemporaries } from "./PromoteUsedTemporaries";
 export { propagateScopeDependencies } from "./PropagateScopeDependencies";
 export { pruneAllReactiveScopes } from "./PruneAllReactiveScopes";
+export { pruneHoistedContexts } from "./PruneHoistedContexts";
 export { pruneNonEscapingScopes } from "./PruneNonEscapingScopes";
 export { pruneNonReactiveDependencies } from "./PruneNonReactiveDependencies";
 export { pruneTemporaryLValues as pruneUnusedLValues } from "./PruneTemporaryLValues";
@@ -1,31 +0,0 @@
-
-## Input
-
-```javascript
-function hoisting() {
-  const foo = () => {
-    return bar();
-  };
-  const bar = () => {
-    return 1;
-  };
-
-  return foo(); // OK: bar's value is only accessed outside of its TDZ
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: hoisting,
-  params: [],
-  isComponent: false,
-};
-
-```
-
-
-## Error
-
-```
-[ReactForget] Todo: EnterSSA: Expected identifier to be defined before being used. Identifier bar$0 is undefined (5:7)
-```
-          
-      
@@ -1,29 +0,0 @@
-
-## Input
-
-```javascript
-function hoisting() {
-  const foo = () => {
-    return bar + baz;
-  };
-  let bar = 3;
-  const baz = 2;
-  return foo(); // OK: called outside of TDZ for bar/baz
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: hoisting,
-  params: [],
-  isComponent: false,
-};
-
-```
-
-
-## Error
-
-```
-[ReactForget] Todo: EnterSSA: Expected identifier to be defined before being used. Identifier bar$0 is undefined (5:5)
-```
-          
-      
@@ -0,0 +1,67 @@
+
+## Input
+
+```javascript
+function hoisting() {
+  function onClick(x) {
+    return x + bar["baz"];
+  }
+  function onClick2(x) {
+    return x + bar[baz];
+  }
+  const baz = "baz";
+  const bar = { baz: 1 };
+
+  return <Button onClick={onClick} onClick2={onClick2} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+
+## Code
+
+```javascript
+import { unstable_useMemoCache as useMemoCache } from "react";
+function hoisting() {
+  const $ = useMemoCache(3);
+  let onClick;
+  let onClick2;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    onClick = function onClick(x) {
+      return x + bar.baz;
+    };
+    onClick2 = function onClick2(x_0) {
+      return x_0 + bar[baz];
+    };
+
+    const baz = "baz";
+    const bar = { baz: 1 };
+    $[0] = onClick;
+    $[1] = onClick2;
+  } else {
+    onClick = $[0];
+    onClick2 = $[1];
+  }
+  let t0;
+  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = <Button onClick={onClick} onClick2={onClick2} />;
+    $[2] = t0;
+  } else {
+    t0 = $[2];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+      
@@ -0,0 +1,18 @@
+function hoisting() {
+  function onClick(x) {
+    return x + bar["baz"];
+  }
+  function onClick2(x) {
+    return x + bar[baz];
+  }
+  const baz = "baz";
+  const bar = { baz: 1 };
+
+  return <Button onClick={onClick} onClick2={onClick2} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
@@ -0,0 +1,56 @@
+
+## Input
+
+```javascript
+function hoisting() {
+  function onClick(x) {
+    return x + bar.baz;
+  }
+  const bar = { baz: 1 };
+
+  return <Button onClick={onClick} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+
+## Code
+
+```javascript
+import { unstable_useMemoCache as useMemoCache } from "react";
+function hoisting() {
+  const $ = useMemoCache(2);
+  let onClick;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    onClick = function onClick(x) {
+      return x + bar.baz;
+    };
+
+    const bar = { baz: 1 };
+    $[0] = onClick;
+  } else {
+    onClick = $[0];
+  }
+  let t0;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = <Button onClick={onClick} />;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+      
@@ -0,0 +1,14 @@
+function hoisting() {
+  function onClick(x) {
+    return x + bar.baz;
+  }
+  const bar = { baz: 1 };
+
+  return <Button onClick={onClick} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
@@ -0,0 +1,64 @@
+
+## Input
+
+```javascript
+function hoisting(cond) {
+  let items = [];
+  if (cond) {
+    const foo = () => {
+      items.push(bar());
+    };
+    const bar = () => true;
+    foo();
+  }
+  return items;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [true],
+  isComponent: false,
+};
+
+```
+
+## Code
+
+```javascript
+import { unstable_useMemoCache as useMemoCache } from "react";
+function hoisting(cond) {
+  const $ = useMemoCache(3);
+  const c_0 = $[0] !== cond;
+  let items;
+  if (c_0) {
+    items = [];
+    if (cond) {
+      const foo = () => {
+        items.push(bar());
+      };
+      let t0;
+      if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+        t0 = () => true;
+        $[2] = t0;
+      } else {
+        t0 = $[2];
+      }
+      const bar = t0;
+      foo();
+    }
+    $[0] = cond;
+    $[1] = items;
+  } else {
+    items = $[1];
+  }
+  return items;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [true],
+  isComponent: false,
+};
+
+```
+      
@@ -0,0 +1,17 @@
+function hoisting(cond) {
+  let items = [];
+  if (cond) {
+    const foo = () => {
+      items.push(bar());
+    };
+    const bar = () => true;
+    foo();
+  }
+  return items;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [true],
+  isComponent: false,
+};
@@ -0,0 +1,61 @@
+
+## Input
+
+```javascript
+function hoisting() {
+  const qux = () => {
+    let result;
+    {
+      result = foo();
+    }
+    return result;
+  };
+  const foo = () => {
+    return bar + baz;
+  };
+  const bar = 3;
+  const baz = 2;
+  return qux(); // OK: called outside of TDZ
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+
+## Code
+
+```javascript
+import { unstable_useMemoCache as useMemoCache } from "react";
+function hoisting() {
+  const $ = useMemoCache(1);
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    const qux = () => {
+      const result = foo();
+      return result;
+    };
+
+    const foo = () => bar + baz;
+
+    const bar = 3;
+    const baz = 2;
+    t0 = qux();
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+      
@@ -0,0 +1,21 @@
+function hoisting() {
+  const qux = () => {
+    let result;
+    {
+      result = foo();
+    }
+    return result;
+  };
+  const foo = () => {
+    return bar + baz;
+  };
+  const bar = 3;
+  const baz = 2;
+  return qux(); // OK: called outside of TDZ
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
@@ -0,0 +1,55 @@
+
+## Input
+
+```javascript
+function hoisting() {
+  const foo = () => {
+    return bar + baz;
+  };
+  const bar = 3;
+  const baz = 2;
+  return foo(); // OK: called outside of TDZ for bar/baz
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+
+## Code
+
+```javascript
+import { unstable_useMemoCache as useMemoCache } from "react";
+function hoisting() {
+  const $ = useMemoCache(2);
+  let foo;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    foo = () => bar + baz;
+
+    const bar = 3;
+    const baz = 2;
+    $[0] = foo;
+  } else {
+    foo = $[0];
+  }
+  let t0;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = foo();
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+      
@@ -2,7 +2,7 @@ function hoisting() {
  const foo = () => {
    return bar + baz;
  };
-  let bar = 3;
+  const bar = 3;
  const baz = 2;
  return foo(); // OK: called outside of TDZ for bar/baz
 }
@@ -0,0 +1,57 @@
+
+## Input
+
+```javascript
+function hoisting() {
+  const foo = () => {
+    return bar();
+  };
+  const bar = () => {
+    return 1;
+  };
+
+  return foo(); // OK: bar's value is only accessed outside of its TDZ
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+
+## Code
+
+```javascript
+import { unstable_useMemoCache as useMemoCache } from "react";
+function hoisting() {
+  const $ = useMemoCache(2);
+  let t1;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    const foo = () => bar();
+    let t0;
+    if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+      t0 = () => 1;
+      $[1] = t0;
+    } else {
+      t0 = $[1];
+    }
+    const bar = t0;
+
+    t1 = foo();
+    $[0] = t1;
+  } else {
+    t1 = $[0];
+  }
+  return t1;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: hoisting,
+  params: [],
+  isComponent: false,
+};
+
+```
+