From d1e044b81ab311b74427ebc58fc541a53bc3c711 Mon Sep 17 00:00:00 2001
From: Mofei Zhang <mofei.zhang@utexas.edu>
Date: Thu, 11 May 2023 16:24:37 -0400
Subject: [PATCH] [deps] Add DeclareLocal to ReactiveScope decls

---

Try to fix bug from #1589:

> If a declaration for an immutable identifier (i.e. one that is not later
re-assigned, since undefined is a primitive) is sandwiched between mutations, we
currently do not record it as an output or hoist it out of the reactive scope.

One simple fix is to add all declared (and later referenced) identifiers as
declarations of a reactive scope. This has some undesired effects (e.g.
additional instructions + memo cache slots), but in practice, this shouldn't be
happening often.

Alternatively, we could 1.) add a pass to hoist declarations, 2.) account for
this in constant propagation, or 3.) add a bailout
---
 .../PropagateScopeDependencies.ts               |  9 +++++++++
 ...zed-declaration-in-reactive-scope.expect.md} | 17 ++++++++++-------
 ...nitialized-declaration-in-reactive-scope.js} |  0
 3 files changed, 19 insertions(+), 7 deletions(-)
 rename compiler/forget/src/__tests__/fixtures/compiler/{_bug.uninitialized-declaration-in-reactive-scope.expect.md => uninitialized-declaration-in-reactive-scope.expect.md} (69%)
 rename compiler/forget/src/__tests__/fixtures/compiler/{_bug.uninitialized-declaration-in-reactive-scope.js => uninitialized-declaration-in-reactive-scope.js} (100%)

diff --git a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
index 7aaf4a980a..0f3e292b7c 100644
--- a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+++ b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
@@ -537,6 +537,15 @@ class PropagationVisitor extends ReactiveFunctionVisitor<Context> {
         id,
         scope: context.currentScope,
       });
+    } else if (value.kind === "DeclareLocal") {
+      // Some variables may be declared and never initialized. We need
+      // to retain (and hoist) these declarations if they are included
+      // in a reactive scope. One approach is to simply add all `DeclareLocal`s
+      // as scope declarations.
+      context.declare(value.lvalue.place.identifier, {
+        id,
+        scope: context.currentScope,
+      });
     } else if (value.kind === "Destructure") {
       context.visitOperand(value.value);
       for (const place of eachPatternOperand(value.lvalue.pattern)) {
diff --git a/compiler/forget/src/__tests__/fixtures/compiler/_bug.uninitialized-declaration-in-reactive-scope.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/uninitialized-declaration-in-reactive-scope.expect.md
similarity index 69%
rename from compiler/forget/src/__tests__/fixtures/compiler/_bug.uninitialized-declaration-in-reactive-scope.expect.md
rename to compiler/forget/src/__tests__/fixtures/compiler/uninitialized-declaration-in-reactive-scope.expect.md
index 014e51ac59..de9b7dee92 100644
--- a/compiler/forget/src/__tests__/fixtures/compiler/_bug.uninitialized-declaration-in-reactive-scope.expect.md
+++ b/compiler/forget/src/__tests__/fixtures/compiler/uninitialized-declaration-in-reactive-scope.expect.md
@@ -16,22 +16,25 @@ function Component(props) {
 ```javascript
 import { unstable_useMemoCache as useMemoCache } from "react";
 function Component(props) {
-  const $ = useMemoCache(2);
+  const $ = useMemoCache(3);
+  let y;
   let x;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
     x = mutate();
-    let y;
+
     foo(x);
-    $[0] = x;
+    $[0] = y;
+    $[1] = x;
   } else {
-    x = $[0];
+    y = $[0];
+    x = $[1];
   }
   let t0;
-  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
     t0 = [y, x];
-    $[1] = t0;
+    $[2] = t0;
   } else {
-    t0 = $[1];
+    t0 = $[2];
   }
   return t0;
 }
diff --git a/compiler/forget/src/__tests__/fixtures/compiler/_bug.uninitialized-declaration-in-reactive-scope.js b/compiler/forget/src/__tests__/fixtures/compiler/uninitialized-declaration-in-reactive-scope.js
similarity index 100%
rename from compiler/forget/src/__tests__/fixtures/compiler/_bug.uninitialized-declaration-in-reactive-scope.js
rename to compiler/forget/src/__tests__/fixtures/compiler/uninitialized-declaration-in-reactive-scope.js