Skip to content

Commit deba48a

Browse files
authored
[compiler] Repro for invalid Array.map type (#32094)
See test fixture --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32094). * #32099 * #32104 * #32098 * #32097 * #32096 * #32095 * __->__ #32094 * #32093
1 parent b6b33bf commit deba48a

File tree

3 files changed

+220
-0
lines changed

3 files changed

+220
-0
lines changed
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {
6+
arrayPush,
7+
identity,
8+
makeArray,
9+
Stringify,
10+
useFragment,
11+
} from 'shared-runtime';
12+
13+
/**
14+
* Bug repro showing why it's invalid for function references to be annotated
15+
* with a `Read` effect when that reference might lead to the function being
16+
* invoked.
17+
*
18+
* Note that currently, `Array.map` is annotated to have `Read` effects on its
19+
* operands. This is incorrect as function effects must be replayed when `map`
20+
* is called
21+
* - Read: non-aliasing data dependency
22+
* - Capture: maybe-aliasing data dependency
23+
* - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke
24+
* but only if the value is mutable
25+
*
26+
* Invalid evaluator result: Found differences in evaluator results Non-forget
27+
* (expected): (kind: ok)
28+
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
29+
* <div>{"x":[2,2,2],"count":4}</div><div>{"item":1}</div>
30+
* Forget:
31+
* (kind: ok)
32+
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
33+
* <div>{"x":[2,2,2,2,2,2],"count":4}</div><div>{"item":1}</div>
34+
*/
35+
36+
function Component({extraJsx}) {
37+
const x = makeArray();
38+
const items = useFragment();
39+
const jsx = items.a.map((item, i) => {
40+
arrayPush(x, 2);
41+
return <Stringify item={item} key={i} />;
42+
});
43+
const offset = jsx.length;
44+
for (let i = 0; i < extraJsx; i++) {
45+
jsx.push(<Stringify item={0} key={i + offset} />);
46+
}
47+
const count = jsx.length;
48+
identity(count);
49+
return (
50+
<>
51+
<Stringify x={x} count={count} />
52+
{jsx[0]}
53+
</>
54+
);
55+
}
56+
export const FIXTURE_ENTRYPOINT = {
57+
fn: Component,
58+
params: [{extraJsx: 0}],
59+
sequentialRenders: [{extraJsx: 0}, {extraJsx: 1}],
60+
};
61+
62+
```
63+
64+
## Code
65+
66+
```javascript
67+
import { c as _c } from "react/compiler-runtime";
68+
import {
69+
arrayPush,
70+
identity,
71+
makeArray,
72+
Stringify,
73+
useFragment,
74+
} from "shared-runtime";
75+
76+
/**
77+
* Bug repro showing why it's invalid for function references to be annotated
78+
* with a `Read` effect when that reference might lead to the function being
79+
* invoked.
80+
*
81+
* Note that currently, `Array.map` is annotated to have `Read` effects on its
82+
* operands. This is incorrect as function effects must be replayed when `map`
83+
* is called
84+
* - Read: non-aliasing data dependency
85+
* - Capture: maybe-aliasing data dependency
86+
* - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke
87+
* but only if the value is mutable
88+
*
89+
* Invalid evaluator result: Found differences in evaluator results Non-forget
90+
* (expected): (kind: ok)
91+
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
92+
* <div>{"x":[2,2,2],"count":4}</div><div>{"item":1}</div>
93+
* Forget:
94+
* (kind: ok)
95+
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
96+
* <div>{"x":[2,2,2,2,2,2],"count":4}</div><div>{"item":1}</div>
97+
*/
98+
99+
function Component(t0) {
100+
const $ = _c(9);
101+
const { extraJsx } = t0;
102+
let t1;
103+
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
104+
t1 = makeArray();
105+
$[0] = t1;
106+
} else {
107+
t1 = $[0];
108+
}
109+
const x = t1;
110+
const items = useFragment();
111+
let jsx;
112+
if ($[1] !== extraJsx || $[2] !== items.a) {
113+
jsx = items.a.map((item, i) => {
114+
arrayPush(x, 2);
115+
return <Stringify item={item} key={i} />;
116+
});
117+
const offset = jsx.length;
118+
for (let i_0 = 0; i_0 < extraJsx; i_0++) {
119+
jsx.push(<Stringify item={0} key={i_0 + offset} />);
120+
}
121+
$[1] = extraJsx;
122+
$[2] = items.a;
123+
$[3] = jsx;
124+
} else {
125+
jsx = $[3];
126+
}
127+
128+
const count = jsx.length;
129+
identity(count);
130+
let t2;
131+
if ($[4] !== count) {
132+
t2 = <Stringify x={x} count={count} />;
133+
$[4] = count;
134+
$[5] = t2;
135+
} else {
136+
t2 = $[5];
137+
}
138+
const t3 = jsx[0];
139+
let t4;
140+
if ($[6] !== t2 || $[7] !== t3) {
141+
t4 = (
142+
<>
143+
{t2}
144+
{t3}
145+
</>
146+
);
147+
$[6] = t2;
148+
$[7] = t3;
149+
$[8] = t4;
150+
} else {
151+
t4 = $[8];
152+
}
153+
return t4;
154+
}
155+
156+
export const FIXTURE_ENTRYPOINT = {
157+
fn: Component,
158+
params: [{ extraJsx: 0 }],
159+
sequentialRenders: [{ extraJsx: 0 }, { extraJsx: 1 }],
160+
};
161+
162+
```
163+
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import {
2+
arrayPush,
3+
identity,
4+
makeArray,
5+
Stringify,
6+
useFragment,
7+
} from 'shared-runtime';
8+
9+
/**
10+
* Bug repro showing why it's invalid for function references to be annotated
11+
* with a `Read` effect when that reference might lead to the function being
12+
* invoked.
13+
*
14+
* Note that currently, `Array.map` is annotated to have `Read` effects on its
15+
* operands. This is incorrect as function effects must be replayed when `map`
16+
* is called
17+
* - Read: non-aliasing data dependency
18+
* - Capture: maybe-aliasing data dependency
19+
* - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke
20+
* but only if the value is mutable
21+
*
22+
* Invalid evaluator result: Found differences in evaluator results Non-forget
23+
* (expected): (kind: ok)
24+
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
25+
* <div>{"x":[2,2,2],"count":4}</div><div>{"item":1}</div>
26+
* Forget:
27+
* (kind: ok)
28+
* <div>{"x":[2,2,2],"count":3}</div><div>{"item":1}</div>
29+
* <div>{"x":[2,2,2,2,2,2],"count":4}</div><div>{"item":1}</div>
30+
*/
31+
32+
function Component({extraJsx}) {
33+
const x = makeArray();
34+
const items = useFragment();
35+
const jsx = items.a.map((item, i) => {
36+
arrayPush(x, 2);
37+
return <Stringify item={item} key={i} />;
38+
});
39+
const offset = jsx.length;
40+
for (let i = 0; i < extraJsx; i++) {
41+
jsx.push(<Stringify item={0} key={i + offset} />);
42+
}
43+
const count = jsx.length;
44+
identity(count);
45+
return (
46+
<>
47+
<Stringify x={x} count={count} />
48+
{jsx[0]}
49+
</>
50+
);
51+
}
52+
export const FIXTURE_ENTRYPOINT = {
53+
fn: Component,
54+
params: [{extraJsx: 0}],
55+
sequentialRenders: [{extraJsx: 0}, {extraJsx: 1}],
56+
};

compiler/packages/snap/src/SproutTodoFilter.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,7 @@ const skipFilter = new Set([
486486
'bug-aliased-capture-mutate',
487487
'bug-functiondecl-hoisting',
488488
'bug-try-catch-maybe-null-dependency',
489+
'bug-invalid-mixedreadonly-map-shape',
489490
'bug-type-inference-control-flow',
490491
'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',
491492
'bug-invalid-phi-as-dependency',

0 commit comments

Comments
 (0)