[compiler] Repro for mutable range edge case (#31479)
See test fixtures
mofeiZ committed
Nov 11, 2024 at 18:04 UTC
2ec26bc4323673d1f2035191d3aaf0a18b20d488
5 files changed
+283
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md
new
+107
@@ -0,0 +1,107 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @flow @enableTransitivelyFreezeFunctionExpressions:false
6
+import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime';
7
+
8
+/**
9
+ * 1. `InferMutableRanges` derives the mutable range of identifiers and their
10
+ * aliases from `LoadLocal`, `PropertyLoad`, etc
11
+ * - After this pass, y's mutable range only extends to `arrayPush(x, y)`
12
+ * - We avoid assigning mutable ranges to loads after y's mutable range, as
13
+ * these are working with an immutable value. As a result, `LoadLocal y` and
14
+ * `PropertyLoad y` do not get mutable ranges
15
+ * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes,
16
+ * as according to the 'co-mutation' of different values
17
+ * - Here, we infer that
18
+ * - `arrayPush(y, x)` might alias `x` and `y` to each other
19
+ * - `setPropertyKey(x, ...)` may mutate both `x` and `y`
20
+ * - This pass correctly extends the mutable range of `y`
21
+ * - Since we didn't run `InferMutableRange` logic again, the LoadLocal /
22
+ * PropertyLoads still don't have a mutable range
23
+ *
24
+ * Note that the this bug is an edge case. Compiler output is only invalid for:
25
+ * - function expressions with
26
+ * `enableTransitivelyFreezeFunctionExpressions:false`
27
+ * - functions that throw and get retried without clearing the memocache
28
+ *
29
+ * Found differences in evaluator results
30
+ * Non-forget (expected):
31
+ * (kind: ok)
32
+ * <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
33
+ * <div>{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}</div>
34
+ * Forget:
35
+ * (kind: ok)
36
+ * <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
37
+ * <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
38
+ */
39
+function useFoo({a, b}: {a: number, b: number}) {
40
+ const x = [];
41
+ const y = {value: a};
42
+
43
+ arrayPush(x, y); // x and y co-mutate
44
+ const y_alias = y;
45
+ const cb = () => y_alias.value;
46
+ setPropertyByKey(x[0], 'value', b); // might overwrite y.value
47
+ return <Stringify cb={cb} shouldInvokeFns={true} />;
48
+}
49
+
50
+export const FIXTURE_ENTRYPOINT = {
51
+ fn: useFoo,
52
+ params: [{a: 2, b: 10}],
53
+ sequentialRenders: [
54
+ {a: 2, b: 10},
55
+ {a: 2, b: 11},
56
+ ],
57
+};
58
+
59
+```
60
+
61
+## Code
62
+
63
+```javascript
64
+import { c as _c } from "react/compiler-runtime";
65
+import { arrayPush, setPropertyByKey, Stringify } from "shared-runtime";
66
+
67
+function useFoo(t0) {
68
+ const $ = _c(5);
69
+ const { a, b } = t0;
70
+ let t1;
71
+ if ($[0] !== a || $[1] !== b) {
72
+ const x = [];
73
+ const y = { value: a };
74
+
75
+ arrayPush(x, y);
76
+ const y_alias = y;
77
+ let t2;
78
+ if ($[3] !== y_alias.value) {
79
+ t2 = () => y_alias.value;
80
+ $[3] = y_alias.value;
81
+ $[4] = t2;
82
+ } else {
83
+ t2 = $[4];
84
+ }
85
+ const cb = t2;
86
+ setPropertyByKey(x[0], "value", b);
87
+ t1 = <Stringify cb={cb} shouldInvokeFns={true} />;
88
+ $[0] = a;
89
+ $[1] = b;
90
+ $[2] = t1;
91
+ } else {
92
+ t1 = $[2];
93
+ }
94
+ return t1;
95
+}
96
+
97
+export const FIXTURE_ENTRYPOINT = {
98
+ fn: useFoo,
99
+ params: [{ a: 2, b: 10 }],
100
+ sequentialRenders: [
101
+ { a: 2, b: 10 },
102
+ { a: 2, b: 11 },
103
+ ],
104
+};
105
+
106
+```
107
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js
new
+53
@@ -0,0 +1,53 @@
1
+// @flow @enableTransitivelyFreezeFunctionExpressions:false
2
+import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime';
3
+
4
+/**
5
+ * 1. `InferMutableRanges` derives the mutable range of identifiers and their
6
+ * aliases from `LoadLocal`, `PropertyLoad`, etc
7
+ * - After this pass, y's mutable range only extends to `arrayPush(x, y)`
8
+ * - We avoid assigning mutable ranges to loads after y's mutable range, as
9
+ * these are working with an immutable value. As a result, `LoadLocal y` and
10
+ * `PropertyLoad y` do not get mutable ranges
11
+ * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes,
12
+ * as according to the 'co-mutation' of different values
13
+ * - Here, we infer that
14
+ * - `arrayPush(y, x)` might alias `x` and `y` to each other
15
+ * - `setPropertyKey(x, ...)` may mutate both `x` and `y`
16
+ * - This pass correctly extends the mutable range of `y`
17
+ * - Since we didn't run `InferMutableRange` logic again, the LoadLocal /
18
+ * PropertyLoads still don't have a mutable range
19
+ *
20
+ * Note that the this bug is an edge case. Compiler output is only invalid for:
21
+ * - function expressions with
22
+ * `enableTransitivelyFreezeFunctionExpressions:false`
23
+ * - functions that throw and get retried without clearing the memocache
24
+ *
25
+ * Found differences in evaluator results
26
+ * Non-forget (expected):
27
+ * (kind: ok)
28
+ * <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
29
+ * <div>{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}</div>
30
+ * Forget:
31
+ * (kind: ok)
32
+ * <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
33
+ * <div>{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}</div>
34
+ */
35
+function useFoo({a, b}: {a: number, b: number}) {
36
+ const x = [];
37
+ const y = {value: a};
38
+
39
+ arrayPush(x, y); // x and y co-mutate
40
+ const y_alias = y;
41
+ const cb = () => y_alias.value;
42
+ setPropertyByKey(x[0], 'value', b); // might overwrite y.value
43
+ return <Stringify cb={cb} shouldInvokeFns={true} />;
44
+}
45
+
46
+export const FIXTURE_ENTRYPOINT = {
47
+ fn: useFoo,
48
+ params: [{a: 2, b: 10}],
49
+ sequentialRenders: [
50
+ {a: 2, b: 10},
51
+ {a: 2, b: 11},
52
+ ],
53
+};
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md
new
+87
@@ -0,0 +1,87 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+// @flow @enableTransitivelyFreezeFunctionExpressions:false
6
+import {setPropertyByKey, Stringify} from 'shared-runtime';
7
+
8
+/**
9
+ * Variation of bug in `bug-aliased-capture-aliased-mutate`
10
+ * Found differences in evaluator results
11
+ * Non-forget (expected):
12
+ * (kind: ok)
13
+ * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
14
+ * <div>{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}</div>
15
+ * Forget:
16
+ * (kind: ok)
17
+ * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
18
+ * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
19
+ */
20
+
21
+function useFoo({a}: {a: number, b: number}) {
22
+ const arr = [];
23
+ const obj = {value: a};
24
+
25
+ setPropertyByKey(obj, 'arr', arr);
26
+ const obj_alias = obj;
27
+ const cb = () => obj_alias.arr.length;
28
+ for (let i = 0; i < a; i++) {
29
+ arr.push(i);
30
+ }
31
+ return <Stringify cb={cb} shouldInvokeFns={true} />;
32
+}
33
+
34
+export const FIXTURE_ENTRYPOINT = {
35
+ fn: useFoo,
36
+ params: [{a: 2}],
37
+ sequentialRenders: [{a: 2}, {a: 3}],
38
+};
39
+
40
+```
41
+
42
+## Code
43
+
44
+```javascript
45
+import { c as _c } from "react/compiler-runtime";
46
+import { setPropertyByKey, Stringify } from "shared-runtime";
47
+
48
+function useFoo(t0) {
49
+ const $ = _c(4);
50
+ const { a } = t0;
51
+ let t1;
52
+ if ($[0] !== a) {
53
+ const arr = [];
54
+ const obj = { value: a };
55
+
56
+ setPropertyByKey(obj, "arr", arr);
57
+ const obj_alias = obj;
58
+ let t2;
59
+ if ($[2] !== obj_alias.arr.length) {
60
+ t2 = () => obj_alias.arr.length;
61
+ $[2] = obj_alias.arr.length;
62
+ $[3] = t2;
63
+ } else {
64
+ t2 = $[3];
65
+ }
66
+ const cb = t2;
67
+ for (let i = 0; i < a; i++) {
68
+ arr.push(i);
69
+ }
70
+
71
+ t1 = <Stringify cb={cb} shouldInvokeFns={true} />;
72
+ $[0] = a;
73
+ $[1] = t1;
74
+ } else {
75
+ t1 = $[1];
76
+ }
77
+ return t1;
78
+}
79
+
80
+export const FIXTURE_ENTRYPOINT = {
81
+ fn: useFoo,
82
+ params: [{ a: 2 }],
83
+ sequentialRenders: [{ a: 2 }, { a: 3 }],
84
+};
85
+
86
+```
87
+
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js
new
+34
@@ -0,0 +1,34 @@
1
+// @flow @enableTransitivelyFreezeFunctionExpressions:false
2
+import {setPropertyByKey, Stringify} from 'shared-runtime';
3
+
4
+/**
5
+ * Variation of bug in `bug-aliased-capture-aliased-mutate`
6
+ * Found differences in evaluator results
7
+ * Non-forget (expected):
8
+ * (kind: ok)
9
+ * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
10
+ * <div>{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}</div>
11
+ * Forget:
12
+ * (kind: ok)
13
+ * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
14
+ * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
15
+ */
16
+
17
+function useFoo({a}: {a: number, b: number}) {
18
+ const arr = [];
19
+ const obj = {value: a};
20
+
21
+ setPropertyByKey(obj, 'arr', arr);
22
+ const obj_alias = obj;
23
+ const cb = () => obj_alias.arr.length;
24
+ for (let i = 0; i < a; i++) {
25
+ arr.push(i);
26
+ }
27
+ return <Stringify cb={cb} shouldInvokeFns={true} />;
28
+}
29
+
30
+export const FIXTURE_ENTRYPOINT = {
31
+ fn: useFoo,
32
+ params: [{a: 2}],
33
+ sequentialRenders: [{a: 2}, {a: 3}],
34
+};
compiler/packages/snap/src/SproutTodoFilter.ts
+2
@@ -480,6 +480,8 @@ const skipFilter = new Set([
480
'fbt/bug-fbt-plural-multiple-mixed-call-tag',
481
'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr',
482
'bug-invalid-hoisting-functionexpr',
483
+ 'bug-aliased-capture-aliased-mutate',
484
+ 'bug-aliased-capture-mutate',
485
'bug-functiondecl-hoisting',
486
'bug-try-catch-maybe-null-dependency',
487
'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',