@samitouri / QOS-React-2 / commits / 80b81fe563

[compiler] Repro for aliased captures within inner function expressions (#31770)

see fixture

mofeiZ committed Dec 16, 2024 at 14:43 UTC 80b81fe56353f7419bd07ecfc3534d274a413fca
3 files changed +178
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md new
+129
@@ -0,0 +1,129 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {makeArray, mutate} from 'shared-runtime';
6 +
7 +/**
8 + * Bug repro:
9 + * Found differences in evaluator results
10 + * Non-forget (expected):
11 + * (kind: ok)
12 + * {"bar":4,"x":{"foo":3,"wat0":"joe"}}
13 + * {"bar":5,"x":{"foo":3,"wat0":"joe"}}
14 + * Forget:
15 + * (kind: ok)
16 + * {"bar":4,"x":{"foo":3,"wat0":"joe"}}
17 + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
18 + *
19 + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly
20 + * aliasing `y` via `[y]`, we make an opaque call.
21 + *
22 + * Note that the bug here is that we don't infer that `a = makeArray(y)`
23 + * potentially captures a context variable into a local variable. As a result,
24 + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
25 + * currently inferring that this lambda captures `y` (for a potential later
26 + * mutation) and simply reads `x`.
27 + *
28 + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
29 + * used when we analyze CallExpressions.
30 + */
31 +function Component({foo, bar}: {foo: number; bar: number}) {
32 + let x = {foo};
33 + let y: {bar: number; x?: {foo: number}} = {bar};
34 + const f0 = function () {
35 + let a = makeArray(y); // a = [y]
36 + let b = x;
37 + // this writes y.x = x
38 + a[0].x = b;
39 + };
40 + f0();
41 + mutate(y.x);
42 + return y;
43 +}
44 +
45 +export const FIXTURE_ENTRYPOINT = {
46 + fn: Component,
47 + params: [{foo: 3, bar: 4}],
48 + sequentialRenders: [
49 + {foo: 3, bar: 4},
50 + {foo: 3, bar: 5},
51 + ],
52 +};
53 +
54 +```
55 +
56 +## Code
57 +
58 +```javascript
59 +import { c as _c } from "react/compiler-runtime";
60 +import { makeArray, mutate } from "shared-runtime";
61 +
62 +/**
63 + * Bug repro:
64 + * Found differences in evaluator results
65 + * Non-forget (expected):
66 + * (kind: ok)
67 + * {"bar":4,"x":{"foo":3,"wat0":"joe"}}
68 + * {"bar":5,"x":{"foo":3,"wat0":"joe"}}
69 + * Forget:
70 + * (kind: ok)
71 + * {"bar":4,"x":{"foo":3,"wat0":"joe"}}
72 + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
73 + *
74 + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly
75 + * aliasing `y` via `[y]`, we make an opaque call.
76 + *
77 + * Note that the bug here is that we don't infer that `a = makeArray(y)`
78 + * potentially captures a context variable into a local variable. As a result,
79 + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
80 + * currently inferring that this lambda captures `y` (for a potential later
81 + * mutation) and simply reads `x`.
82 + *
83 + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
84 + * used when we analyze CallExpressions.
85 + */
86 +function Component(t0) {
87 + const $ = _c(5);
88 + const { foo, bar } = t0;
89 + let t1;
90 + if ($[0] !== foo) {
91 + t1 = { foo };
92 + $[0] = foo;
93 + $[1] = t1;
94 + } else {
95 + t1 = $[1];
96 + }
97 + const x = t1;
98 + let y;
99 + if ($[2] !== bar || $[3] !== x) {
100 + y = { bar };
101 + const f0 = function () {
102 + const a = makeArray(y);
103 + const b = x;
104 +
105 + a[0].x = b;
106 + };
107 +
108 + f0();
109 + mutate(y.x);
110 + $[2] = bar;
111 + $[3] = x;
112 + $[4] = y;
113 + } else {
114 + y = $[4];
115 + }
116 + return y;
117 +}
118 +
119 +export const FIXTURE_ENTRYPOINT = {
120 + fn: Component,
121 + params: [{ foo: 3, bar: 4 }],
122 + sequentialRenders: [
123 + { foo: 3, bar: 4 },
124 + { foo: 3, bar: 5 },
125 + ],
126 +};
127 +
128 +```
129 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts new
+48
@@ -0,0 +1,48 @@
1 +import {makeArray, mutate} from 'shared-runtime';
2 +
3 +/**
4 + * Bug repro:
5 + * Found differences in evaluator results
6 + * Non-forget (expected):
7 + * (kind: ok)
8 + * {"bar":4,"x":{"foo":3,"wat0":"joe"}}
9 + * {"bar":5,"x":{"foo":3,"wat0":"joe"}}
10 + * Forget:
11 + * (kind: ok)
12 + * {"bar":4,"x":{"foo":3,"wat0":"joe"}}
13 + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}}
14 + *
15 + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly
16 + * aliasing `y` via `[y]`, we make an opaque call.
17 + *
18 + * Note that the bug here is that we don't infer that `a = makeArray(y)`
19 + * potentially captures a context variable into a local variable. As a result,
20 + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're
21 + * currently inferring that this lambda captures `y` (for a potential later
22 + * mutation) and simply reads `x`.
23 + *
24 + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not
25 + * used when we analyze CallExpressions.
26 + */
27 +function Component({foo, bar}: {foo: number; bar: number}) {
28 + let x = {foo};
29 + let y: {bar: number; x?: {foo: number}} = {bar};
30 + const f0 = function () {
31 + let a = makeArray(y); // a = [y]
32 + let b = x;
33 + // this writes y.x = x
34 + a[0].x = b;
35 + };
36 + f0();
37 + mutate(y.x);
38 + return y;
39 +}
40 +
41 +export const FIXTURE_ENTRYPOINT = {
42 + fn: Component,
43 + params: [{foo: 3, bar: 4}],
44 + sequentialRenders: [
45 + {foo: 3, bar: 4},
46 + {foo: 3, bar: 5},
47 + ],
48 +};
compiler/packages/snap/src/SproutTodoFilter.ts
+1
@@ -479,6 +479,7 @@ const skipFilter = new Set([
479 // bugs
480 'fbt/bug-fbt-plural-multiple-function-calls',
481 'fbt/bug-fbt-plural-multiple-mixed-call-tag',
482 + `bug-capturing-func-maybealias-captured-mutate`,
483 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr',
484 'bug-invalid-hoisting-functionexpr',
485 'bug-aliased-capture-aliased-mutate',