Repository: facebook/react
Pull request: [compiler] Fix AnalyzeFunctions to fully reset context identifiers


AnalyzeFunctions had logic to reset the mutable ranges of context variables after visiting inner function expressions. However, there was a bug in that logic: InferReactiveScopeVariables makes all the identifiers in a scope point to the same mutable range instance. That meant that it was possible for a later function expression to indirectly cause an earlier function expressions' context variables to get a non-zero mutable range.

The fix is to not just reset start/end of context var ranges, but assign a new range instance. Thanks for the help on debugging, @mofeiz!

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33500).
* #33571
* #33558
* #33547
* #33543
* #33533
* #33532
* #33530
* #33526
* #33522
* #33518
* #33514
* #33513
* #33512
* #33504
* __->__ #33500
* #33497
* #33496

Changed files:
filename: compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts
patch: @@ -42,8 +42,16 @@ export default function analyseFunctions(func: HIRFunction): void {
            * Reset mutable range for outer inferReferenceEffects
            */
           for (const operand of instr.value.loweredFunc.func.context) {
-            operand.identifier.mutableRange.start = makeInstructionId(0);
-            operand.identifier.mutableRange.end = makeInstructionId(0);
+            /**
+             * NOTE: inferReactiveScopeVariables makes identifiers in the scope
+             * point to the *same* mutableRange instance. Resetting start/end
+             * here is insufficient, because a later mutation of the range
+             * for any one identifier could affect the range for other identifiers.
+             */
+            operand.identifier.mutableRange = {
+              start: makeInstructionId(0),
+              end: makeInstructionId(0),
+            };
             operand.identifier.scope = null;
           }
           break;

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-internal-compiler-shared-mutablerange-bug.expect.md
patch: @@ -0,0 +1,80 @@
+
+## Input
+
+```javascript
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
+component Component(
+  onAsyncSubmit?: (() => void) => void,
+  onClose: (isConfirmed: boolean) => void
+) {
+  // When running inferReactiveScopeVariables,
+  // onAsyncSubmit and onClose update to share
+  // a mutableRange instance.
+  const onSubmit = useCallback(() => {
+    if (onAsyncSubmit) {
+      onAsyncSubmit(() => {
+        onClose(true);
+      });
+      return;
+    }
+  }, [onAsyncSubmit, onClose]);
+  // When running inferReactiveScopeVariables here,
+  // first the existing range gets updated (affecting
+  // onAsyncSubmit) and then onClose gets assigned a
+  // different mutable range instance, which is the
+  // one reset after AnalyzeFunctions.
+  // The fix is to fully reset mutable ranges *instances*
+  // after AnalyzeFunctions visit a function expression
+  return <Dialog onSubmit={onSubmit} onClose={() => onClose(false)} />;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component(t0) {
+  const $ = _c(8);
+  const { onAsyncSubmit, onClose } = t0;
+  let t1;
+  if ($[0] !== onAsyncSubmit || $[1] !== onClose) {
+    t1 = () => {
+      if (onAsyncSubmit) {
+        onAsyncSubmit(() => {
+          onClose(true);
+        });
+        return;
+      }
+    };
+    $[0] = onAsyncSubmit;
+    $[1] = onClose;
+    $[2] = t1;
+  } else {
+    t1 = $[2];
+  }
+  const onSubmit = t1;
+  let t2;
+  if ($[3] !== onClose) {
+    t2 = () => onClose(false);
+    $[3] = onClose;
+    $[4] = t2;
+  } else {
+    t2 = $[4];
+  }
+  let t3;
+  if ($[5] !== onSubmit || $[6] !== t2) {
+    t3 = <Dialog onSubmit={onSubmit} onClose={t2} />;
+    $[5] = onSubmit;
+    $[6] = t2;
+    $[7] = t3;
+  } else {
+    t3 = $[7];
+  }
+  return t3;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented
\ No newline at end of file

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-internal-compiler-shared-mutablerange-bug.js
patch: @@ -0,0 +1,25 @@
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
+component Component(
+  onAsyncSubmit?: (() => void) => void,
+  onClose: (isConfirmed: boolean) => void
+) {
+  // When running inferReactiveScopeVariables,
+  // onAsyncSubmit and onClose update to share
+  // a mutableRange instance.
+  const onSubmit = useCallback(() => {
+    if (onAsyncSubmit) {
+      onAsyncSubmit(() => {
+        onClose(true);
+      });
+      return;
+    }
+  }, [onAsyncSubmit, onClose]);
+  // When running inferReactiveScopeVariables here,
+  // first the existing range gets updated (affecting
+  // onAsyncSubmit) and then onClose gets assigned a
+  // different mutable range instance, which is the
+  // one reset after AnalyzeFunctions.
+  // The fix is to fully reset mutable ranges *instances*
+  // after AnalyzeFunctions visit a function expression
+  return <Dialog onSubmit={onSubmit} onClose={() => onClose(false)} />;
+}