The library for web and native user interfaces.
Mirrored from https://github.com/247TaxiDriver/QOS-React.git
Synced Nov 14, 05:30 UTC
Generalize #1521 for PropertyLoad
This is a more general version of the change from #1521. That PR ensured that LoadLocal temporaries accessed outside the instruction's scope are correctly promoted. However, we have a similar pattern with PropertyLoad. This PR adds a general mechanism for handling these type of indirections: any LoadLocal/PropertyLoad temporary accessed when it's defining scope is not active will be promoted to a declaration of the defining scope. Notably, we do this in a way that ensures that the dependencies are preserved, ie that we correctly view the operand of LoadLocal/PropertyLoad as a dependency of the current scope.
Joe Savona committed
Apr 26, 2023 at 11:27 UTC
e98823f93373d3e988ed2f9323a1b803b76f8935
7 files changed
+176
-17
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+77
-17
@@ -16,6 +16,7 @@ import {
16
ReactiveFunction,
17
ReactiveInstruction,
18
ReactiveScope,
19
+ ReactiveScopeBlock,
20
ReactiveScopeDependency,
21
ReactiveValue,
22
} from "../HIR/HIR";
@@ -29,6 +30,7 @@ import {
30
ReactiveScopeDependencyTree,
31
ReactiveScopePropertyDependency,
32
} from "./DeriveMinimalDependencies";
33
+import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
34
35
/**
36
* Infers the dependencies of each scope to include variables whose values
@@ -37,7 +39,13 @@ import {
39
* their direct dependencies and those of their child scopes.
40
*/
41
export function propagateScopeDependencies(fn: ReactiveFunction): void {
40
- const context = new Context();
42
+ const promotedTemporaries: PromotedTemporaries = {
43
+ declarations: new Map(),
44
+ used: new Set(),
45
+ };
46
+ visitReactiveFunction(fn, new FindPromotedTemporaries(), promotedTemporaries);
47
+
48
+ const context = new Context(promotedTemporaries.used);
49
if (fn.id !== null) {
50
context.declare(fn.id, {
51
id: makeInstructionId(0),
@@ -53,6 +61,59 @@ export function propagateScopeDependencies(fn: ReactiveFunction): void {
61
visit(context, fn.body);
62
}
63
64
+type PromotedTemporaries = {
65
+ declarations: Map<IdentifierId, ReactiveScope>;
66
+ used: Set<IdentifierId>;
67
+};
68
+class FindPromotedTemporaries extends ReactiveFunctionVisitor<PromotedTemporaries> {
69
+ scopes: Array<ReactiveScope> = [];
70
+
71
+ override visitScope(
72
+ scope: ReactiveScopeBlock,
73
+ state: PromotedTemporaries
74
+ ): void {
75
+ this.scopes.push(scope.scope);
76
+ this.traverseScope(scope, state);
77
+ this.scopes.pop();
78
+ }
79
+
80
+ override visitInstruction(
81
+ instruction: ReactiveInstruction,
82
+ state: PromotedTemporaries
83
+ ): void {
84
+ const scope = this.scopes.at(-1);
85
+ if (instruction.lvalue === null || scope === undefined) {
86
+ return;
87
+ }
88
+ switch (instruction.value.kind) {
89
+ case "LoadLocal":
90
+ case "PropertyLoad": {
91
+ state.declarations.set(instruction.lvalue.identifier.id, scope);
92
+ break;
93
+ }
94
+ default: {
95
+ break;
96
+ }
97
+ }
98
+ this.traverseInstruction(instruction, state);
99
+ }
100
+
101
+ override visitPlace(
102
+ _id: InstructionId,
103
+ place: Place,
104
+ state: PromotedTemporaries
105
+ ): void {
106
+ const declaringScope = state.declarations.get(place.identifier.id);
107
+ if (this.scopes.length === 0 || declaringScope === undefined) {
108
+ return;
109
+ }
110
+ if (this.scopes.indexOf(declaringScope) === -1) {
111
+ // Declaring scope is not active === used outside declaring scope
112
+ state.used.add(place.identifier.id);
113
+ }
114
+ }
115
+}
116
+
117
type DeclMap = Map<IdentifierId, Decl>;
118
type Decl = {
119
id: InstructionId;
@@ -60,6 +121,7 @@ type Decl = {
121
};
122
123
class Context {
124
+ #temporariesUsedOutsideScope: Set<IdentifierId>;
125
#declarations: DeclMap = new Map();
126
#reassignments: Map<Identifier, Decl> = new Map();
127
// Reactive dependencies used in the current reactive scope.
@@ -71,8 +133,7 @@ class Context {
133
// ReactiveScope (B) that uses the produced temporary.
134
// - codegen will inline these PropertyLoads back into scope (B)
135
#properties: Map<Identifier, ReactiveScopePropertyDependency> = new Map();
74
- #temporaries: Map<Identifier, { place: Place; scope: ReactiveScope | null }> =
75
- new Map();
136
+ #temporaries: Map<Identifier, Place> = new Map();
137
#inConditionalWithinScope: boolean = false;
138
// Reactive dependencies used unconditionally in the current conditional.
139
// Composed of dependencies:
@@ -82,6 +143,10 @@ class Context {
143
new ReactiveScopeDependencyTree();
144
#scopes: Stack<ReactiveScope> = empty();
145
146
+ constructor(temporariesUsedOutsideScope: Set<IdentifierId>) {
147
+ this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope;
148
+ }
149
+
150
enter(scope: ReactiveScope, fn: () => void): Set<ReactiveScopeDependency> {
151
// Save context of previous scope
152
const prevInConditional = this.#inConditionalWithinScope;
@@ -119,6 +184,10 @@ class Context {
184
return minInnerScopeDependencies;
185
}
186
187
+ isUsedOutsideDeclaringScope(place: Place): boolean {
188
+ return this.#temporariesUsedOutsideScope.has(place.identifier.id);
189
+ }
190
+
191
/**
192
* Prints dependency tree to string for debugging.
193
* @param includeAccesses
@@ -186,21 +255,11 @@ class Context {
255
}
256
257
declareTemporary(lvalue: Place, place: Place): void {
189
- this.#temporaries.set(lvalue.identifier, {
190
- place,
191
- scope: this.currentScope.value,
192
- });
258
+ this.#temporaries.set(lvalue.identifier, place);
259
}
260
261
resolveTemporary(place: Place): Place {
196
- const temporary = this.#temporaries.get(place.identifier);
197
- if (
198
- temporary !== undefined &&
199
- (temporary.scope === null || this.#isScopeActive(temporary.scope))
200
- ) {
201
- return temporary.place;
202
- }
203
- return place;
262
+ return this.#temporaries.get(place.identifier) ?? place;
263
}
264
265
#getProperty(
@@ -539,14 +598,15 @@ function visitInstructionValue(
598
if (value.kind === "LoadLocal" && lvalue !== null) {
599
if (
600
value.place.identifier.name !== null &&
542
- lvalue.identifier.name === null
601
+ lvalue.identifier.name === null &&
602
+ !context.isUsedOutsideDeclaringScope(lvalue)
603
) {
604
context.declareTemporary(lvalue, value.place);
605
} else {
606
context.visitOperand(value.place);
607
}
608
} else if (value.kind === "PropertyLoad") {
549
- if (lvalue !== null) {
609
+ if (lvalue !== null && !context.isUsedOutsideDeclaringScope(lvalue)) {
610
context.declareProperty(
611
lvalue,
612
value.object,
compiler/forget/src/__tests__/fixtures/compiler/jsx-member-expression-tag-grouping.expect.md
new
+41
@@ -0,0 +1,41 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const maybeMutable = new MaybeMutable();
7
+ return <Foo.Bar>{maybeMutate(maybeMutable)}</Foo.Bar>;
8
+}
9
+
10
+```
11
+
12
+## Code
13
+
14
+```javascript
15
+import { unstable_useMemoCache as useMemoCache } from "react";
16
+function Component(props) {
17
+ const $ = useMemoCache(3);
18
+ let T0;
19
+ let t1;
20
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
21
+ const maybeMutable = new MaybeMutable();
22
+ T0 = Foo.Bar;
23
+ t1 = maybeMutate(maybeMutable);
24
+ $[0] = T0;
25
+ $[1] = t1;
26
+ } else {
27
+ T0 = $[0];
28
+ t1 = $[1];
29
+ }
30
+ let t2;
31
+ if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
32
+ t2 = <T0>{t1}</T0>;
33
+ $[2] = t2;
34
+ } else {
35
+ t2 = $[2];
36
+ }
37
+ return t2;
38
+}
39
+
40
+```
41
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/jsx-member-expression-tag-grouping.js
new
+4
@@ -0,0 +1,4 @@
1
+function Component(props) {
2
+ const maybeMutable = new MaybeMutable();
3
+ return <Foo.Bar>{maybeMutate(maybeMutable)}</Foo.Bar>;
4
+}
compiler/forget/src/__tests__/fixtures/compiler/temporary-accessed-outside-scope.expect.md
new
+49
@@ -0,0 +1,49 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const maybeMutable = new MaybeMutable();
7
+ let x = props;
8
+ return [x, maybeMutate(maybeMutable)];
9
+}
10
+
11
+```
12
+
13
+## Code
14
+
15
+```javascript
16
+import { unstable_useMemoCache as useMemoCache } from "react";
17
+function Component(props) {
18
+ const $ = useMemoCache(6);
19
+ const c_0 = $[0] !== props;
20
+ let t0;
21
+ let t1;
22
+ if (c_0) {
23
+ const maybeMutable = new MaybeMutable();
24
+ const x = props;
25
+ t0 = x;
26
+ t1 = maybeMutate(maybeMutable);
27
+ $[0] = props;
28
+ $[1] = t0;
29
+ $[2] = t1;
30
+ } else {
31
+ t0 = $[1];
32
+ t1 = $[2];
33
+ }
34
+ const c_3 = $[3] !== t0;
35
+ const c_4 = $[4] !== t1;
36
+ let t2;
37
+ if (c_3 || c_4) {
38
+ t2 = [t0, t1];
39
+ $[3] = t0;
40
+ $[4] = t1;
41
+ $[5] = t2;
42
+ } else {
43
+ t2 = $[5];
44
+ }
45
+ return t2;
46
+}
47
+
48
+```
49
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/temporary-accessed-outside-scope.js
new
+5
@@ -0,0 +1,5 @@
1
+function Component(props) {
2
+ const maybeMutable = new MaybeMutable();
3
+ let x = props;
4
+ return [x, maybeMutate(maybeMutable)];
5
+}