Skip to content

Commit 9df5788

Browse files
committed
[compiler] Copy fixtures affected by new inference
1 parent d58c07b commit 9df5788

File tree

46 files changed

+1912
-0
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+1912
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
2+
## Input
3+
4+
```javascript
5+
import {
6+
Stringify,
7+
mutate,
8+
identity,
9+
shallowCopy,
10+
setPropertyByKey,
11+
} from 'shared-runtime';
12+
13+
/**
14+
* This fixture is similar to `bug-aliased-capture-aliased-mutate` and
15+
* `nonmutating-capture-in-unsplittable-memo-block`, but with a focus on
16+
* dependency extraction.
17+
*
18+
* NOTE: this fixture is currently valid, but will break with optimizations:
19+
* - Scope and mutable-range based reordering may move the array creation
20+
* *after* the `mutate(aliasedObj)` call. This is invalid if mutate
21+
* reassigns inner properties.
22+
* - RecycleInto or other deeper-equality optimizations may produce invalid
23+
* output -- it may compare the array's contents / dependencies too early.
24+
* - Runtime validation for immutable values will break if `mutate` does
25+
* interior mutation of the value captured into the array.
26+
*
27+
* Before scope block creation, HIR looks like this:
28+
* //
29+
* // $1 is unscoped as obj's mutable range will be
30+
* // extended in a later pass
31+
* //
32+
* $1 = LoadLocal obj@0[0:12]
33+
* $2 = PropertyLoad $1.id
34+
* //
35+
* // $3 gets assigned a scope as Array is an allocating
36+
* // instruction, but this does *not* get extended or
37+
* // merged into the later mutation site.
38+
* // (explained in `bug-aliased-capture-aliased-mutate`)
39+
* //
40+
* $3@1 = Array[$2]
41+
* ...
42+
* $10@0 = LoadLocal shallowCopy@0[0, 12]
43+
* $11 = LoadGlobal mutate
44+
* $12 = $11($10@0[0, 12])
45+
*
46+
* When filling in scope dependencies, we find that it's incorrect to depend on
47+
* PropertyLoads from obj as it hasn't completed its mutable range. Following
48+
* the immutable / mutable-new typing system, we check the identity of obj to
49+
* detect whether it was newly created (and thus mutable) in this render pass.
50+
*
51+
* HIR with scopes looks like this.
52+
* bb0:
53+
* $1 = LoadLocal obj@0[0:12]
54+
* $2 = PropertyLoad $1.id
55+
* scopeTerminal deps=[obj@0] block=bb1 fallt=bb2
56+
* bb1:
57+
* $3@1 = Array[$2]
58+
* goto bb2
59+
* bb2:
60+
* ...
61+
*
62+
* This is surprising as deps now is entirely decoupled from temporaries used
63+
* by the block itself. scope @1's instructions now reference a value (1)
64+
* produced outside its scope range and (2) not represented in its dependencies
65+
*
66+
* The right thing to do is to ensure that all Loads from a value get assigned
67+
* the value's reactive scope. This also requires track mutating and aliasing
68+
* separately from scope range. In this example, that would correctly merge
69+
* the scopes of $3 with obj.
70+
* Runtime validation and optimizations such as ReactiveGraph-based reordering
71+
* require this as well.
72+
*
73+
* A tempting fix is to instead extend $3's ReactiveScope range up to include
74+
* $2 (the PropertyLoad). This fixes dependency deduping but not reordering
75+
* and mutability.
76+
*/
77+
function Component({prop}) {
78+
let obj = shallowCopy(prop);
79+
const aliasedObj = identity(obj);
80+
81+
// [obj.id] currently is assigned its own reactive scope
82+
const id = [obj.id];
83+
84+
// Writing to the alias may reassign to previously captured references.
85+
// The compiler currently produces valid output, but this breaks with
86+
// reordering, recycleInto, and other potential optimizations.
87+
mutate(aliasedObj);
88+
setPropertyByKey(aliasedObj, 'id', prop.id + 1);
89+
90+
return <Stringify id={id} />;
91+
}
92+
93+
export const FIXTURE_ENTRYPOINT = {
94+
fn: Component,
95+
params: [{prop: {id: 1}}],
96+
sequentialRenders: [{prop: {id: 1}}, {prop: {id: 1}}, {prop: {id: 2}}],
97+
};
98+
99+
```
100+
101+
## Code
102+
103+
```javascript
104+
import { c as _c } from "react/compiler-runtime";
105+
import {
106+
Stringify,
107+
mutate,
108+
identity,
109+
shallowCopy,
110+
setPropertyByKey,
111+
} from "shared-runtime";
112+
113+
/**
114+
* This fixture is similar to `bug-aliased-capture-aliased-mutate` and
115+
* `nonmutating-capture-in-unsplittable-memo-block`, but with a focus on
116+
* dependency extraction.
117+
*
118+
* NOTE: this fixture is currently valid, but will break with optimizations:
119+
* - Scope and mutable-range based reordering may move the array creation
120+
* *after* the `mutate(aliasedObj)` call. This is invalid if mutate
121+
* reassigns inner properties.
122+
* - RecycleInto or other deeper-equality optimizations may produce invalid
123+
* output -- it may compare the array's contents / dependencies too early.
124+
* - Runtime validation for immutable values will break if `mutate` does
125+
* interior mutation of the value captured into the array.
126+
*
127+
* Before scope block creation, HIR looks like this:
128+
* //
129+
* // $1 is unscoped as obj's mutable range will be
130+
* // extended in a later pass
131+
* //
132+
* $1 = LoadLocal obj@0[0:12]
133+
* $2 = PropertyLoad $1.id
134+
* //
135+
* // $3 gets assigned a scope as Array is an allocating
136+
* // instruction, but this does *not* get extended or
137+
* // merged into the later mutation site.
138+
* // (explained in `bug-aliased-capture-aliased-mutate`)
139+
* //
140+
* $3@1 = Array[$2]
141+
* ...
142+
* $10@0 = LoadLocal shallowCopy@0[0, 12]
143+
* $11 = LoadGlobal mutate
144+
* $12 = $11($10@0[0, 12])
145+
*
146+
* When filling in scope dependencies, we find that it's incorrect to depend on
147+
* PropertyLoads from obj as it hasn't completed its mutable range. Following
148+
* the immutable / mutable-new typing system, we check the identity of obj to
149+
* detect whether it was newly created (and thus mutable) in this render pass.
150+
*
151+
* HIR with scopes looks like this.
152+
* bb0:
153+
* $1 = LoadLocal obj@0[0:12]
154+
* $2 = PropertyLoad $1.id
155+
* scopeTerminal deps=[obj@0] block=bb1 fallt=bb2
156+
* bb1:
157+
* $3@1 = Array[$2]
158+
* goto bb2
159+
* bb2:
160+
* ...
161+
*
162+
* This is surprising as deps now is entirely decoupled from temporaries used
163+
* by the block itself. scope @1's instructions now reference a value (1)
164+
* produced outside its scope range and (2) not represented in its dependencies
165+
*
166+
* The right thing to do is to ensure that all Loads from a value get assigned
167+
* the value's reactive scope. This also requires track mutating and aliasing
168+
* separately from scope range. In this example, that would correctly merge
169+
* the scopes of $3 with obj.
170+
* Runtime validation and optimizations such as ReactiveGraph-based reordering
171+
* require this as well.
172+
*
173+
* A tempting fix is to instead extend $3's ReactiveScope range up to include
174+
* $2 (the PropertyLoad). This fixes dependency deduping but not reordering
175+
* and mutability.
176+
*/
177+
function Component(t0) {
178+
const $ = _c(4);
179+
const { prop } = t0;
180+
let t1;
181+
if ($[0] !== prop) {
182+
const obj = shallowCopy(prop);
183+
const aliasedObj = identity(obj);
184+
let t2;
185+
if ($[2] !== obj) {
186+
t2 = [obj.id];
187+
$[2] = obj;
188+
$[3] = t2;
189+
} else {
190+
t2 = $[3];
191+
}
192+
const id = t2;
193+
194+
mutate(aliasedObj);
195+
setPropertyByKey(aliasedObj, "id", prop.id + 1);
196+
197+
t1 = <Stringify id={id} />;
198+
$[0] = prop;
199+
$[1] = t1;
200+
} else {
201+
t1 = $[1];
202+
}
203+
return t1;
204+
}
205+
206+
export const FIXTURE_ENTRYPOINT = {
207+
fn: Component,
208+
params: [{ prop: { id: 1 } }],
209+
sequentialRenders: [
210+
{ prop: { id: 1 } },
211+
{ prop: { id: 1 } },
212+
{ prop: { id: 2 } },
213+
],
214+
};
215+
216+
```
217+
218+
### Eval output
219+
(kind: ok) <div>{"id":[1]}</div>
220+
<div>{"id":[1]}</div>
221+
<div>{"id":[2]}</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import {
2+
Stringify,
3+
mutate,
4+
identity,
5+
shallowCopy,
6+
setPropertyByKey,
7+
} from 'shared-runtime';
8+
9+
/**
10+
* This fixture is similar to `bug-aliased-capture-aliased-mutate` and
11+
* `nonmutating-capture-in-unsplittable-memo-block`, but with a focus on
12+
* dependency extraction.
13+
*
14+
* NOTE: this fixture is currently valid, but will break with optimizations:
15+
* - Scope and mutable-range based reordering may move the array creation
16+
* *after* the `mutate(aliasedObj)` call. This is invalid if mutate
17+
* reassigns inner properties.
18+
* - RecycleInto or other deeper-equality optimizations may produce invalid
19+
* output -- it may compare the array's contents / dependencies too early.
20+
* - Runtime validation for immutable values will break if `mutate` does
21+
* interior mutation of the value captured into the array.
22+
*
23+
* Before scope block creation, HIR looks like this:
24+
* //
25+
* // $1 is unscoped as obj's mutable range will be
26+
* // extended in a later pass
27+
* //
28+
* $1 = LoadLocal obj@0[0:12]
29+
* $2 = PropertyLoad $1.id
30+
* //
31+
* // $3 gets assigned a scope as Array is an allocating
32+
* // instruction, but this does *not* get extended or
33+
* // merged into the later mutation site.
34+
* // (explained in `bug-aliased-capture-aliased-mutate`)
35+
* //
36+
* $3@1 = Array[$2]
37+
* ...
38+
* $10@0 = LoadLocal shallowCopy@0[0, 12]
39+
* $11 = LoadGlobal mutate
40+
* $12 = $11($10@0[0, 12])
41+
*
42+
* When filling in scope dependencies, we find that it's incorrect to depend on
43+
* PropertyLoads from obj as it hasn't completed its mutable range. Following
44+
* the immutable / mutable-new typing system, we check the identity of obj to
45+
* detect whether it was newly created (and thus mutable) in this render pass.
46+
*
47+
* HIR with scopes looks like this.
48+
* bb0:
49+
* $1 = LoadLocal obj@0[0:12]
50+
* $2 = PropertyLoad $1.id
51+
* scopeTerminal deps=[obj@0] block=bb1 fallt=bb2
52+
* bb1:
53+
* $3@1 = Array[$2]
54+
* goto bb2
55+
* bb2:
56+
* ...
57+
*
58+
* This is surprising as deps now is entirely decoupled from temporaries used
59+
* by the block itself. scope @1's instructions now reference a value (1)
60+
* produced outside its scope range and (2) not represented in its dependencies
61+
*
62+
* The right thing to do is to ensure that all Loads from a value get assigned
63+
* the value's reactive scope. This also requires track mutating and aliasing
64+
* separately from scope range. In this example, that would correctly merge
65+
* the scopes of $3 with obj.
66+
* Runtime validation and optimizations such as ReactiveGraph-based reordering
67+
* require this as well.
68+
*
69+
* A tempting fix is to instead extend $3's ReactiveScope range up to include
70+
* $2 (the PropertyLoad). This fixes dependency deduping but not reordering
71+
* and mutability.
72+
*/
73+
function Component({prop}) {
74+
let obj = shallowCopy(prop);
75+
const aliasedObj = identity(obj);
76+
77+
// [obj.id] currently is assigned its own reactive scope
78+
const id = [obj.id];
79+
80+
// Writing to the alias may reassign to previously captured references.
81+
// The compiler currently produces valid output, but this breaks with
82+
// reordering, recycleInto, and other potential optimizations.
83+
mutate(aliasedObj);
84+
setPropertyByKey(aliasedObj, 'id', prop.id + 1);
85+
86+
return <Stringify id={id} />;
87+
}
88+
89+
export const FIXTURE_ENTRYPOINT = {
90+
fn: Component,
91+
params: [{prop: {id: 1}}],
92+
sequentialRenders: [{prop: {id: 1}}, {prop: {id: 1}}, {prop: {id: 2}}],
93+
};

0 commit comments

Comments
 (0)