From 008cebd633d5f28955a3ddd358c9e177d256ddc4 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 14 Feb 2023 16:22:58 -0800
Subject: [PATCH] [facepalm] Fix bug w missing deps

While reviewing @poteto's PR I noticed that there were some cases of missing
dependencies. I tracked it down to a bug I introduced
[here](https://github.com/facebook/react-forget/commit/5b827eb85ce0b09a72e620449d1d676071c2e0b9#r100646304).
Decl.id is meant to be the id of the instruction that declares the variable. We
then test to see if a dependency is later than that. If the Decl.id is
incorrectly too high, then we miss some dependencies thinking they aren't
defined yet.
---
 .../src/ReactiveScopes/PropagateScopeDependencies.ts |  3 ++-
 .../fixtures/hir/reassignment-conditional.expect.md  | 12 +++++++-----
 .../hir/reassignment-separate-scopes.expect.md       |  8 +++++---
 .../__tests__/fixtures/hir/ssa-leave-case.expect.md  |  8 +++++---
 .../fixtures/hir/switch-non-final-default.expect.md  | 12 +++++++-----
 .../src/__tests__/fixtures/hir/switch.expect.md      | 12 +++++++-----
 6 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
index 7a95f7e1c9..5b96f742dd 100644
--- a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+++ b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
@@ -151,6 +151,7 @@ class Context {
     }
 
     const decl = this.#declarations.get(maybeDependency.place.identifier.id);
+
     // if decl is undefined here, then this is a free var
     //  (all other decls e.g. `let x;` should be initialized in BuildHIR)
 
@@ -376,7 +377,7 @@ function visitInstruction(context: Context, instr: ReactiveInstruction): void {
   } else {
     context.declare(lvalue.place.identifier, {
       kind: DeclKind.Dynamic,
-      id: lvalue.place.identifier.mutableRange.start,
+      id: instr.id,
       scope: context.currentScope,
     });
   }
diff --git a/compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md b/compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md
index eb87414d0c..6b70c68314 100644
--- a/compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md
+++ b/compiler/forget/src/__tests__/fixtures/hir/reassignment-conditional.expect.md
@@ -48,14 +48,16 @@ function Component(props) {
   }
 
   y.push(props.p2);
-  const c_3 = $[3] !== y;
+  const c_3 = $[3] !== x$0;
+  const c_4 = $[4] !== y;
   let t0;
-  if (c_3) {
+  if (c_3 || c_4) {
     t0 = <Component x={x$0} y={y}></Component>;
-    $[3] = y;
-    $[4] = t0;
+    $[3] = x$0;
+    $[4] = y;
+    $[5] = t0;
   } else {
-    t0 = $[4];
+    t0 = $[5];
   }
   return t0;
 }
diff --git a/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md b/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md
index f65319c67a..f3304c07cd 100644
--- a/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md
+++ b/compiler/forget/src/__tests__/fixtures/hir/reassignment-separate-scopes.expect.md
@@ -87,8 +87,9 @@ function foo(a, b, c) {
     }
   }
   const c_8 = $[8] !== y;
+  const c_9 = $[9] !== x$0;
   let t0;
-  if (c_8) {
+  if (c_8 || c_9) {
     t0 = (
       <div>
         {y}
@@ -96,9 +97,10 @@ function foo(a, b, c) {
       </div>
     );
     $[8] = y;
-    $[9] = t0;
+    $[9] = x$0;
+    $[10] = t0;
   } else {
-    t0 = $[9];
+    t0 = $[10];
   }
   return t0;
 }
diff --git a/compiler/forget/src/__tests__/fixtures/hir/ssa-leave-case.expect.md b/compiler/forget/src/__tests__/fixtures/hir/ssa-leave-case.expect.md
index 9359a8826e..4c7225c6d8 100644
--- a/compiler/forget/src/__tests__/fixtures/hir/ssa-leave-case.expect.md
+++ b/compiler/forget/src/__tests__/fixtures/hir/ssa-leave-case.expect.md
@@ -46,8 +46,9 @@ function Component(props) {
     y$0 = $[3];
   }
   const c_4 = $[4] !== x;
+  const c_5 = $[5] !== y$0;
   let t0;
-  if (c_4) {
+  if (c_4 || c_5) {
     t0 = (
       <Component>
         {x}
@@ -55,9 +56,10 @@ function Component(props) {
       </Component>
     );
     $[4] = x;
-    $[5] = t0;
+    $[5] = y$0;
+    $[6] = t0;
   } else {
-    t0 = $[5];
+    t0 = $[6];
   }
   return t0;
 }
diff --git a/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md b/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md
index 8b34a04ff3..9ddb49f74d 100644
--- a/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md
+++ b/compiler/forget/src/__tests__/fixtures/hir/switch-non-final-default.expect.md
@@ -83,14 +83,16 @@ function Component(props) {
     child = $[6];
   }
   y$0.push(props.p4);
-  const c_7 = $[7] !== child;
+  const c_7 = $[7] !== y$0;
+  const c_8 = $[8] !== child;
   let t0;
-  if (c_7) {
+  if (c_7 || c_8) {
     t0 = <Component data={y$0}>{child}</Component>;
-    $[7] = child;
-    $[8] = t0;
+    $[7] = y$0;
+    $[8] = child;
+    $[9] = t0;
   } else {
-    t0 = $[8];
+    t0 = $[9];
   }
   return t0;
 }
diff --git a/compiler/forget/src/__tests__/fixtures/hir/switch.expect.md b/compiler/forget/src/__tests__/fixtures/hir/switch.expect.md
index 1d81a1ef5e..fa5f40ea80 100644
--- a/compiler/forget/src/__tests__/fixtures/hir/switch.expect.md
+++ b/compiler/forget/src/__tests__/fixtures/hir/switch.expect.md
@@ -66,14 +66,16 @@ function Component(props) {
     child = $[6];
   }
   y$0.push(props.p4);
-  const c_7 = $[7] !== child;
+  const c_7 = $[7] !== y$0;
+  const c_8 = $[8] !== child;
   let t0;
-  if (c_7) {
+  if (c_7 || c_8) {
     t0 = <Component data={y$0}>{child}</Component>;
-    $[7] = child;
-    $[8] = t0;
+    $[7] = y$0;
+    $[8] = child;
+    $[9] = t0;
   } else {
-    t0 = $[8];
+    t0 = $[9];
   }
   return t0;
 }