[compiler] Repro for mutable range edge case (#31479)

See test fixtures

Author: mofeiZ

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md

@@ -0,0 +1,107 @@
+
+## Input
+
+```javascript
+// @flow @enableTransitivelyFreezeFunctionExpressions:false
+import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime';
+
+/**
+ * 1. `InferMutableRanges` derives the mutable range of identifiers and their
+ *     aliases from `LoadLocal`, `PropertyLoad`, etc
+ *   - After this pass, y's mutable range only extends to `arrayPush(x, y)`
+ *   - We avoid assigning mutable ranges to loads after y's mutable range, as
+ *     these are working with an immutable value. As a result, `LoadLocal y` and
+ *     `PropertyLoad y` do not get mutable ranges
+ * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes,
+ *    as according to the 'co-mutation' of different values
+ *   - Here, we infer that
+ *     - `arrayPush(y, x)` might alias `x` and `y` to each other
+ *     - `setPropertyKey(x, ...)` may mutate both `x` and `y`
+ *   - This pass correctly extends the mutable range of `y`
+ *   - Since we didn't run `InferMutableRange` logic again, the LoadLocal /
+ *     PropertyLoads still don't have a mutable range
+ *
+ * Note that the this bug is an edge case. Compiler output is only invalid for:
+ *  - function expressions with
+ *    `enableTransitivelyFreezeFunctionExpressions:false`
+ *  - functions that throw and get retried without clearing the memocache
+ *
+ * Found differences in evaluator results
+ * Non-forget (expected):
+ *   (kind: ok)
+ *   <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
+ *   <div>{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}</div>
+ * Forget:
+ *   (kind: ok)
+ *   <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
+ *   <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
+ */
+function useFoo({a, b}: {a: number, b: number}) {
+  const x = [];
+  const y = {value: a};
+
+  arrayPush(x, y); // x and y co-mutate
+  const y_alias = y;
+  const cb = () => y_alias.value;
+  setPropertyByKey(x[0], 'value', b); // might overwrite y.value
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{a: 2, b: 10}],
+  sequentialRenders: [
+    {a: 2, b: 10},
+    {a: 2, b: 11},
+  ],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { arrayPush, setPropertyByKey, Stringify } from "shared-runtime";
+
+function useFoo(t0) {
+  const $ = _c(5);
+  const { a, b } = t0;
+  let t1;
+  if ($[0] !== a || $[1] !== b) {
+    const x = [];
+    const y = { value: a };
+
+    arrayPush(x, y);
+    const y_alias = y;
+    let t2;
+    if ($[3] !== y_alias.value) {
+      t2 = () => y_alias.value;
+      $[3] = y_alias.value;
+      $[4] = t2;
+    } else {
+      t2 = $[4];
+    }
+    const cb = t2;
+    setPropertyByKey(x[0], "value", b);
+    t1 = <Stringify cb={cb} shouldInvokeFns={true} />;
+    $[0] = a;
+    $[1] = b;
+    $[2] = t1;
+  } else {
+    t1 = $[2];
+  }
+  return t1;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{ a: 2, b: 10 }],
+  sequentialRenders: [
+    { a: 2, b: 10 },
+    { a: 2, b: 11 },
+  ],
+};
+
+```
+      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js

@@ -0,0 +1,53 @@
+// @flow @enableTransitivelyFreezeFunctionExpressions:false
+import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime';
+
+/**
+ * 1. `InferMutableRanges` derives the mutable range of identifiers and their
+ *     aliases from `LoadLocal`, `PropertyLoad`, etc
+ *   - After this pass, y's mutable range only extends to `arrayPush(x, y)`
+ *   - We avoid assigning mutable ranges to loads after y's mutable range, as
+ *     these are working with an immutable value. As a result, `LoadLocal y` and
+ *     `PropertyLoad y` do not get mutable ranges
+ * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes,
+ *    as according to the 'co-mutation' of different values
+ *   - Here, we infer that
+ *     - `arrayPush(y, x)` might alias `x` and `y` to each other
+ *     - `setPropertyKey(x, ...)` may mutate both `x` and `y`
+ *   - This pass correctly extends the mutable range of `y`
+ *   - Since we didn't run `InferMutableRange` logic again, the LoadLocal /
+ *     PropertyLoads still don't have a mutable range
+ *
+ * Note that the this bug is an edge case. Compiler output is only invalid for:
+ *  - function expressions with
+ *    `enableTransitivelyFreezeFunctionExpressions:false`
+ *  - functions that throw and get retried without clearing the memocache
+ *
+ * Found differences in evaluator results
+ * Non-forget (expected):
+ *   (kind: ok)
+ *   <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
+ *   <div>{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}</div>
+ * Forget:
+ *   (kind: ok)
+ *   <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
+ *   <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
+ */
+function useFoo({a, b}: {a: number, b: number}) {
+  const x = [];
+  const y = {value: a};
+
+  arrayPush(x, y); // x and y co-mutate
+  const y_alias = y;
+  const cb = () => y_alias.value;
+  setPropertyByKey(x[0], 'value', b); // might overwrite y.value
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{a: 2, b: 10}],
+  sequentialRenders: [
+    {a: 2, b: 10},
+    {a: 2, b: 11},
+  ],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md

@@ -0,0 +1,87 @@
+
+## Input
+
+```javascript
+// @flow @enableTransitivelyFreezeFunctionExpressions:false
+import {setPropertyByKey, Stringify} from 'shared-runtime';
+
+/**
+ * Variation of bug in `bug-aliased-capture-aliased-mutate`
+ * Found differences in evaluator results
+ * Non-forget (expected):
+ *   (kind: ok)
+ *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
+ *   <div>{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}</div>
+ * Forget:
+ *   (kind: ok)
+ *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
+ *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
+ */
+
+function useFoo({a}: {a: number, b: number}) {
+  const arr = [];
+  const obj = {value: a};
+
+  setPropertyByKey(obj, 'arr', arr);
+  const obj_alias = obj;
+  const cb = () => obj_alias.arr.length;
+  for (let i = 0; i < a; i++) {
+    arr.push(i);
+  }
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{a: 2}],
+  sequentialRenders: [{a: 2}, {a: 3}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { setPropertyByKey, Stringify } from "shared-runtime";
+
+function useFoo(t0) {
+  const $ = _c(4);
+  const { a } = t0;
+  let t1;
+  if ($[0] !== a) {
+    const arr = [];
+    const obj = { value: a };
+
+    setPropertyByKey(obj, "arr", arr);
+    const obj_alias = obj;
+    let t2;
+    if ($[2] !== obj_alias.arr.length) {
+      t2 = () => obj_alias.arr.length;
+      $[2] = obj_alias.arr.length;
+      $[3] = t2;
+    } else {
+      t2 = $[3];
+    }
+    const cb = t2;
+    for (let i = 0; i < a; i++) {
+      arr.push(i);
+    }
+
+    t1 = <Stringify cb={cb} shouldInvokeFns={true} />;
+    $[0] = a;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  return t1;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{ a: 2 }],
+  sequentialRenders: [{ a: 2 }, { a: 3 }],
+};
+
+```
+      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js

@@ -0,0 +1,34 @@
+// @flow @enableTransitivelyFreezeFunctionExpressions:false
+import {setPropertyByKey, Stringify} from 'shared-runtime';
+
+/**
+ * Variation of bug in `bug-aliased-capture-aliased-mutate`
+ * Found differences in evaluator results
+ * Non-forget (expected):
+ *   (kind: ok)
+ *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
+ *   <div>{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}</div>
+ * Forget:
+ *   (kind: ok)
+ *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
+ *   <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
+ */
+
+function useFoo({a}: {a: number, b: number}) {
+  const arr = [];
+  const obj = {value: a};
+
+  setPropertyByKey(obj, 'arr', arr);
+  const obj_alias = obj;
+  const cb = () => obj_alias.arr.length;
+  for (let i = 0; i < a; i++) {
+    arr.push(i);
+  }
+  return <Stringify cb={cb} shouldInvokeFns={true} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{a: 2}],
+  sequentialRenders: [{a: 2}, {a: 3}],
+};

compiler/packages/snap/src/SproutTodoFilter.ts

@@ -480,6 +480,8 @@ const skipFilter = new Set([
   'fbt/bug-fbt-plural-multiple-mixed-call-tag',
   'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr',
   'bug-invalid-hoisting-functionexpr',
+  'bug-aliased-capture-aliased-mutate',
+  'bug-aliased-capture-mutate',
   'bug-functiondecl-hoisting',
   'bug-try-catch-maybe-null-dependency',
   'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',