From 2683c0a18c1d0c2834f371d9dc6ef25495a208de Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Fri, 12 Jul 2024 13:00:51 +0900
Subject: [PATCH] [compiler] Fix mode for generating scopes for reassignments

We have an experimental mode where we generate scopes for simple phi values, even if they aren't subsequently mutated. This mode was incorrectly generating scope ranges, leaving the start at 0 which is invalid. The fix is to allow non-zero identifier ranges to overwrite the scope start (rather than taking the min) if the scope start is still zero.

ghstack-source-id: ecbb04c96ed4de62f781e48cda46309c42aa07e0
Pull Request resolved: https://github.com/facebook/react/pull/30321
---
 .../InferReactiveScopeVariables.ts            | 10 ++-
 ...signed-loop-force-scopes-enabled.expect.md | 68 +++++++++++++++++++
 ...ve-reassigned-loop-force-scopes-enabled.js | 19 ++++++
 3 files changed, 94 insertions(+), 3 deletions(-)
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/primitive-reassigned-loop-force-scopes-enabled.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/primitive-reassigned-loop-force-scopes-enabled.js

diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
index 2c9e67646b155..3e344d1bfa34c 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts
@@ -114,9 +114,13 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void {
       };
       scopes.set(groupIdentifier, scope);
     } else {
-      scope.range.start = makeInstructionId(
-        Math.min(scope.range.start, identifier.mutableRange.start)
-      );
+      if (scope.range.start === 0) {
+        scope.range.start = identifier.mutableRange.start;
+      } else if (identifier.mutableRange.start !== 0) {
+        scope.range.start = makeInstructionId(
+          Math.min(scope.range.start, identifier.mutableRange.start)
+        );
+      }
       scope.range.end = makeInstructionId(
         Math.max(scope.range.end, identifier.mutableRange.end)
       );
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/primitive-reassigned-loop-force-scopes-enabled.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/primitive-reassigned-loop-force-scopes-enabled.expect.md
new file mode 100644
index 0000000000000..a40c7452bb973
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/primitive-reassigned-loop-force-scopes-enabled.expect.md
@@ -0,0 +1,68 @@
+
+## Input
+
+```javascript
+// @enableForest
+function Component({ base, start, increment, test }) {
+  let value = base;
+  for (let i = start; i < test; i += increment) {
+    value += i;
+  }
+  return <div>{value}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ base: 0, start: 0, test: 10, increment: 1 }],
+  sequentialRenders: [
+    { base: 0, start: 1, test: 10, increment: 1 },
+    { base: 0, start: 0, test: 10, increment: 2 },
+    { base: 2, start: 0, test: 10, increment: 2 },
+    { base: 0, start: 0, test: 11, increment: 2 },
+  ],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @enableForest
+function Component(t0) {
+  const $ = _c(5);
+  const { base, start, increment, test } = t0;
+  let value;
+  if ($[0] !== base || $[1] !== start || $[2] !== test || $[3] !== increment) {
+    value = base;
+    for (let i = start; i < test; i = i + increment, i) {
+      value = value + i;
+    }
+    $[0] = base;
+    $[1] = start;
+    $[2] = test;
+    $[3] = increment;
+    $[4] = value;
+  } else {
+    value = $[4];
+  }
+  return <div>{value}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ base: 0, start: 0, test: 10, increment: 1 }],
+  sequentialRenders: [
+    { base: 0, start: 1, test: 10, increment: 1 },
+    { base: 0, start: 0, test: 10, increment: 2 },
+    { base: 2, start: 0, test: 10, increment: 2 },
+    { base: 0, start: 0, test: 11, increment: 2 },
+  ],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>45</div>
+<div>20</div>
+<div>22</div>
+<div>30</div>
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/primitive-reassigned-loop-force-scopes-enabled.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/primitive-reassigned-loop-force-scopes-enabled.js
new file mode 100644
index 0000000000000..2cbf8ee58a8dd
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/primitive-reassigned-loop-force-scopes-enabled.js
@@ -0,0 +1,19 @@
+// @enableForest
+function Component({ base, start, increment, test }) {
+  let value = base;
+  for (let i = start; i < test; i += increment) {
+    value += i;
+  }
+  return <div>{value}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ base: 0, start: 0, test: 10, increment: 1 }],
+  sequentialRenders: [
+    { base: 0, start: 1, test: 10, increment: 1 },
+    { base: 0, start: 0, test: 10, increment: 2 },
+    { base: 2, start: 0, test: 10, increment: 2 },
+    { base: 0, start: 0, test: 11, increment: 2 },
+  ],
+};