Repository: facebook/react
Pull request: [compiler] Repro for aliased captures within inner function expressions


see fixture


Changed files:
filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md
patch: @@ -0,0 +1,129 @@
+
+## Input
+
+```javascript
+import {makeArray, mutate} from 'shared-runtime';
+
+/**
+ * Bug repro:
+ * Found differences in evaluator results
+ *   Non-forget (expected):
+ *   (kind: ok)
+ *   {"bar":4,"x":{"foo":3,"wat0":"joe"}}
+ *   {"bar":5,"x":{"foo":3,"wat0":"joe"}}
+ *   Forget:
+ *   (kind: ok)
+ *   {"bar":4,"x":{"foo":3,"wat0":"joe"}}
+ *   {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
+ *
+ * Fork of `capturing-func-alias-captured-mutate`, but instead of directly
+ * aliasing `y` via `[y]`, we make an opaque call.
+ *
+ * Note that the bug here is that we don't infer that `a = makeArray(y)`
+ * potentially captures a context variable into a local variable. As a result,
+ * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
+ * currently inferring that this lambda captures `y` (for a potential later
+ * mutation) and simply reads `x`.
+ *
+ * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
+ * used when we analyze CallExpressions.
+ */
+function Component({foo, bar}: {foo: number; bar: number}) {
+  let x = {foo};
+  let y: {bar: number; x?: {foo: number}} = {bar};
+  const f0 = function () {
+    let a = makeArray(y); // a = [y]
+    let b = x;
+    // this writes y.x = x
+    a[0].x = b;
+  };
+  f0();
+  mutate(y.x);
+  return y;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{foo: 3, bar: 4}],
+  sequentialRenders: [
+    {foo: 3, bar: 4},
+    {foo: 3, bar: 5},
+  ],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { makeArray, mutate } from "shared-runtime";
+
+/**
+ * Bug repro:
+ * Found differences in evaluator results
+ *   Non-forget (expected):
+ *   (kind: ok)
+ *   {"bar":4,"x":{"foo":3,"wat0":"joe"}}
+ *   {"bar":5,"x":{"foo":3,"wat0":"joe"}}
+ *   Forget:
+ *   (kind: ok)
+ *   {"bar":4,"x":{"foo":3,"wat0":"joe"}}
+ *   {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
+ *
+ * Fork of `capturing-func-alias-captured-mutate`, but instead of directly
+ * aliasing `y` via `[y]`, we make an opaque call.
+ *
+ * Note that the bug here is that we don't infer that `a = makeArray(y)`
+ * potentially captures a context variable into a local variable. As a result,
+ * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
+ * currently inferring that this lambda captures `y` (for a potential later
+ * mutation) and simply reads `x`.
+ *
+ * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
+ * used when we analyze CallExpressions.
+ */
+function Component(t0) {
+  const $ = _c(5);
+  const { foo, bar } = t0;
+  let t1;
+  if ($[0] !== foo) {
+    t1 = { foo };
+    $[0] = foo;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  const x = t1;
+  let y;
+  if ($[2] !== bar || $[3] !== x) {
+    y = { bar };
+    const f0 = function () {
+      const a = makeArray(y);
+      const b = x;
+
+      a[0].x = b;
+    };
+
+    f0();
+    mutate(y.x);
+    $[2] = bar;
+    $[3] = x;
+    $[4] = y;
+  } else {
+    y = $[4];
+  }
+  return y;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ foo: 3, bar: 4 }],
+  sequentialRenders: [
+    { foo: 3, bar: 4 },
+    { foo: 3, bar: 5 },
+  ],
+};
+
+```
+      
\ No newline at end of file

filename: compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts
patch: @@ -0,0 +1,48 @@
+import {makeArray, mutate} from 'shared-runtime';
+
+/**
+ * Bug repro:
+ * Found differences in evaluator results
+ *   Non-forget (expected):
+ *   (kind: ok)
+ *   {"bar":4,"x":{"foo":3,"wat0":"joe"}}
+ *   {"bar":5,"x":{"foo":3,"wat0":"joe"}}
+ *   Forget:
+ *   (kind: ok)
+ *   {"bar":4,"x":{"foo":3,"wat0":"joe"}}
+ *   {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
+ *
+ * Fork of `capturing-func-alias-captured-mutate`, but instead of directly
+ * aliasing `y` via `[y]`, we make an opaque call.
+ *
+ * Note that the bug here is that we don't infer that `a = makeArray(y)`
+ * potentially captures a context variable into a local variable. As a result,
+ * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
+ * currently inferring that this lambda captures `y` (for a potential later
+ * mutation) and simply reads `x`.
+ *
+ * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
+ * used when we analyze CallExpressions.
+ */
+function Component({foo, bar}: {foo: number; bar: number}) {
+  let x = {foo};
+  let y: {bar: number; x?: {foo: number}} = {bar};
+  const f0 = function () {
+    let a = makeArray(y); // a = [y]
+    let b = x;
+    // this writes y.x = x
+    a[0].x = b;
+  };
+  f0();
+  mutate(y.x);
+  return y;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{foo: 3, bar: 4}],
+  sequentialRenders: [
+    {foo: 3, bar: 4},
+    {foo: 3, bar: 5},
+  ],
+};

filename: compiler/packages/snap/src/SproutTodoFilter.ts
patch: @@ -479,6 +479,7 @@ const skipFilter = new Set([
   // bugs
   'fbt/bug-fbt-plural-multiple-function-calls',
   'fbt/bug-fbt-plural-multiple-mixed-call-tag',
+  `bug-capturing-func-maybealias-captured-mutate`,
   'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr',
   'bug-invalid-hoisting-functionexpr',
   'bug-aliased-capture-aliased-mutate',