Bailout on assigning to module-scope variables

compiler/forget/src/HIR/BuildHIR.ts

@@ -20,6 +20,7 @@ import {
   GeneratedSource,
   GotoVariant,
   HIRFunction,
+  Identifier,
   IfTerminal,
   InstructionKind,
   InstructionValue,
@@ -52,10 +53,12 @@ import HIRBuilder from "./HIRBuilder";
 export function lower(
   func: NodePath<t.Function>,
   options: EnvironmentOptions | null,
-  capturedRefs: t.Identifier[] = []
+  capturedRefs: t.Identifier[] = [],
+  // the outermost function being compiled, in case lower() is called recursively (for lambdas)
+  parent: NodePath<t.Function> | null = null
 ): Result<HIRFunction, CompilerError> {
   const env = new Environment(options);
-  const builder = new HIRBuilder(env, capturedRefs);
+  const builder = new HIRBuilder(env, parent ?? func, capturedRefs);
   const context: Place[] = [];
 
   for (const ref of capturedRefs ?? []) {
@@ -70,11 +73,10 @@ export function lower(
   // Internal babel is on an older version that does not have hasNode (v7.17)
   // See https://github.com/babel/babel/pull/13940/files for impl
   // TODO: write helper function for NodePath.node != null
-  const id =
-    func.isFunctionDeclaration() && func.get("id").node != null
-      ? builder.resolveIdentifier(func.get("id") as NodePath<t.Identifier>)
-      : null;
-
+  let id: Identifier | null = null;
+  if (func.isFunctionDeclaration() && func.get("id").node != null) {
+    id = builder.resolveIdentifier(func.get("id") as NodePath<t.Identifier>);
+  }
   const params: Array<Place> = [];
   func.get("params").forEach((param) => {
     if (param.isIdentifier()) {
@@ -1343,10 +1345,12 @@ function lowerExpression(
       //
       // This isn't a problem in practice because use Babel's scope analysis to
       // identify the correct references.
-      const lowering = lower(expr, builder.environment.options, [
-        ...builder.context,
-        ...captured.identifiers,
-      ]);
+      const lowering = lower(
+        expr,
+        builder.environment.options,
+        [...builder.context, ...captured.identifiers],
+        builder.parentFunction
+      );
       let loweredFunc: HIRFunction;
       if (lowering.isErr()) {
         lowering

compiler/forget/src/HIR/HIRBuilder.ts

@@ -83,6 +83,7 @@ export default class HIRBuilder {
   #bindings: Map<string, { node: t.Identifier; identifier: Identifier }> =
     new Map();
   #env: Environment;
+  parentFunction: NodePath<t.Function>;
   errors: CompilerError = new CompilerError();
 
   get nextIdentifierId() {
@@ -97,8 +98,13 @@ export default class HIRBuilder {
     return this.#env;
   }
 
-  constructor(env: Environment, context: t.Identifier[]) {
+  constructor(
+    env: Environment,
+    parentFunction: NodePath<t.Function>, // the outermost function being compiled
+    context: t.Identifier[]
+  ) {
     this.#env = env;
+    this.parentFunction = parentFunction;
     this.#context = context;
   }
 
@@ -174,11 +180,30 @@ export default class HIRBuilder {
     path: NodePath<t.Identifier | t.JSXIdentifier>
   ): Identifier | null {
     const originalName = path.node.name;
-    const node = path.scope.getBindingIdentifier(originalName);
-    if (node == null) {
+    const binding = path.scope.getBinding(originalName);
+    if (binding == null) {
       return null;
     }
-    return this.resolveBinding(node);
+    // If the binding is from the parent function's outer scope, then
+    // we treat it equivalently to a global.
+    //
+    // TODO: remove the exception that resolves references to the
+    // parent function itself. We don't need to support self-recursion,
+    // so we can treat such references as globals.
+    const outerBinding =
+      this.parentFunction.scope.parent.getBinding(originalName);
+    if (binding === outerBinding) {
+      const func = this.parentFunction;
+      const isParentFunctionReference =
+        func.isFunctionDeclaration() &&
+        func.get("id").node != null &&
+        func.get("id").node!.name === originalName;
+      if (!isParentFunctionReference) {
+        return null;
+      }
+    }
+
+    return this.resolveBinding(binding.identifier);
   }
 
   resolveBinding(node: t.Identifier): Identifier {

compiler/forget/src/__tests__/fixtures/hir/destructure-capture-global.expect.md

@@ -15,15 +15,18 @@ function component(a) {
 ```javascript
 let someGlobal = {};
 function component(a) {
-  const $ = React.unstable_useMemoCache(2);
+  const $ = React.unstable_useMemoCache(3);
+  const t0 = someGlobal;
   const c_0 = $[0] !== a;
+  const c_1 = $[1] !== t0;
   let x;
-  if (c_0) {
-    x = { a: a, someGlobal: someGlobal };
+  if (c_0 || c_1) {
+    x = { a: a, someGlobal: t0 };
     $[0] = a;
-    $[1] = x;
+    $[1] = t0;
+    $[2] = x;
   } else {
-    x = $[1];
+    x = $[2];
   }
   return x;
 }

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

@@ -71,8 +71,11 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
 
   // Cannot assign to globals
   someUnknownGlobal = true;
+  moduleLocal = true;
 }
 
+let moduleLocal = false;
+
 ```
 
 
@@ -409,8 +412,18 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
   68 |   // Cannot assign to globals
 > 69 |   someUnknownGlobal = true;
      |   ^^^^^^^^^^^^^^^^^
-  70 | }
-  71 |
+  70 |   moduleLocal = true;
+  71 | }
+  72 |
+
+[ReactForget] InvalidInputError: (BuildHIR::lowerAssignment) Assigning to an identifier defined outside the function scope is not supported.
+  68 |   // Cannot assign to globals
+  69 |   someUnknownGlobal = true;
+> 70 |   moduleLocal = true;
+     |   ^^^^^^^^^^^
+  71 | }
+  72 |
+  73 | let moduleLocal = false;
 ```
           
       

compiler/forget/src/__tests__/fixtures/hir/error.todo-kitchensink.js

@@ -67,4 +67,7 @@ function foo([a, b], { c, d, e = "e" }, f = "f", ...args) {
 
   // Cannot assign to globals
   someUnknownGlobal = true;
+  moduleLocal = true;
 }
+
+let moduleLocal = false;

compiler/forget/src/__tests__/fixtures/hir/trivial.expect.md

@@ -0,0 +1,19 @@
+
+## Input
+
+```javascript
+function foo(x) {
+  return x;
+}
+
+```
+
+## Code
+
+```javascript
+function foo(x) {
+  return x;
+}
+
+```
+      

compiler/forget/src/__tests__/fixtures/hir/trivial.js

@@ -0,0 +1,3 @@
+function foo(x) {
+  return x;
+}