[rhir] small: ReactiveDependency uses Identifier instead of Place
--- We never use the `Place` of a ReactiveScopeDependency, except for when we want to access its identifier. Later PRs in this stack will convert `ReactiveScopeDependency` to property access trees (and traverse over the tree). This usually involves merging multiple Dependencies into trees (where each root is a unique identifier). We then traverse over each tree to extract its dependencies (e.g. unconditional leaves). ``` {place: {loc: 1, identifier: 'props'}, path: ['a', 'b']} {place: {loc: 2, identifier: 'props'}, path: ['a']} // merges into a single tree root, which should represent a single identifier ``` The `place` of each individual `ReactiveScopeDependency` will be lost during the tree traversal, and it doesn't really make sense to recreate them using the `Place` attached to the tree root.
Mofei Zhang committed
Feb 27, 2023 at 13:38 UTC
9ca41f8a2e5aa334e3c74aeab5e60465ac0547cc
7 files changed
+30
-24
compiler/forget/src/HIR/HIR.ts
+1
-1
@@ -620,7 +620,7 @@ export type ReactiveScope = {
620
};
621
622
export type ReactiveScopeDependency = {
623
- place: Place;
623
+ identifier: Identifier;
624
path: Array<string> | null;
625
};
626
compiler/forget/src/Inference/AnalyseFunctions.ts
+4
-4
@@ -22,10 +22,10 @@ class State {
22
const objectDependency = this.properties.get(object.identifier);
23
let nextDependency: ReactiveScopeDependency;
24
if (objectDependency === undefined) {
25
- nextDependency = { place: object, path: [property] };
25
+ nextDependency = { identifier: object.identifier, path: [property] };
26
} else {
27
nextDependency = {
28
- place: objectDependency.place,
28
+ identifier: objectDependency.identifier,
29
path: [...(objectDependency.path ?? []), property],
30
};
31
}
@@ -36,7 +36,7 @@ class State {
36
const resolved: ReactiveScopeDependency = this.properties.get(
37
value.identifier
38
) ?? {
39
- place: value,
39
+ identifier: value.identifier,
40
path: null,
41
};
42
this.properties.set(lvalue.identifier, resolved);
@@ -98,7 +98,7 @@ function infer(value: FunctionExpression, state: State, context: Place[]) {
98
99
if (state.properties.has(dep.identifier)) {
100
const receiver = state.properties.get(dep.identifier)!;
101
- name = receiver.place.identifier.name;
101
+ name = receiver.identifier.name;
102
} else {
103
name = dep.identifier.name;
104
}
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+1
-1
@@ -407,7 +407,7 @@ function codegenDependency(
407
cx: Context,
408
dependency: ReactiveScopeDependency
409
): t.Expression {
410
- let object: t.Expression = convertIdentifier(dependency.place.identifier);
410
+ let object: t.Expression = convertIdentifier(dependency.identifier);
411
if (dependency.path !== null) {
412
for (const path of dependency.path) {
413
object = t.memberExpression(object, t.identifier(path));
compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts
+3
-3
@@ -56,11 +56,11 @@ export function printReactiveBlock(
56
}
57
58
function printDependency(dependency: ReactiveScopeDependency): string {
59
- const place = printPlace(dependency.place);
59
+ const identifier = printIdentifier(dependency.identifier);
60
if (dependency.path === null) {
61
- return place;
61
+ return identifier;
62
} else {
63
- return `${place}${dependency.path.map((prop) => `.${prop}`).join("")}`;
63
+ return `${identifier}${dependency.path.map((prop) => `.${prop}`).join("")}`;
64
}
65
}
66
compiler/forget/src/ReactiveScopes/PromoteUsedTemporaries.ts
+1
-1
@@ -23,7 +23,7 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
23
override visitScope(block: ReactiveScopeBlock, state: VisitorState): void {
24
this.traverseScope(block, state);
25
for (const dep of block.scope.dependencies) {
26
- const { identifier } = dep.place;
26
+ const { identifier } = dep;
27
if (identifier.name == null) {
28
identifier.name = `t${state.nextId++}`;
29
}
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+19
-13
@@ -99,10 +99,13 @@ class Context {
99
const objectDependency = this.#properties.get(resolvedObject.identifier);
100
let nextDependency: ReactiveScopeDependency;
101
if (objectDependency === undefined) {
102
- nextDependency = { place: resolvedObject, path: [property] };
102
+ nextDependency = {
103
+ identifier: resolvedObject.identifier,
104
+ path: [property],
105
+ };
106
} else {
107
nextDependency = {
105
- place: objectDependency.place,
108
+ identifier: objectDependency.identifier,
109
path: [...(objectDependency.path ?? []), property],
110
};
111
}
@@ -119,7 +122,7 @@ class Context {
122
123
visitOperand(place: Place): void {
124
const resolved = this.#temporaries.get(place.identifier) ?? place;
122
- this.visitDependency({ place: resolved, path: null });
125
+ this.visitDependency({ identifier: resolved.identifier, path: null });
126
}
127
128
visitProperty(object: Place, property: string): void {
@@ -127,10 +130,13 @@ class Context {
130
const objectDependency = this.#properties.get(resolvedObject.identifier);
131
let nextDependency: ReactiveScopeDependency;
132
if (objectDependency === undefined) {
130
- nextDependency = { place: resolvedObject, path: [property] };
133
+ nextDependency = {
134
+ identifier: resolvedObject.identifier,
135
+ path: [property],
136
+ };
137
} else {
138
nextDependency = {
133
- place: objectDependency.place,
139
+ identifier: objectDependency.identifier,
140
path: [...(objectDependency.path ?? []), property],
141
};
142
}
@@ -146,8 +152,8 @@ class Context {
152
} else {
153
// Otherwise if this operand is a temporary created for a property load, resolve it to
154
// the expanded Place. Fall back to using the operand as-is.
149
- let propDep = this.#properties.get(dependency.place.identifier);
150
- if (dependency.place.identifier.name === null && propDep !== undefined) {
155
+ let propDep = this.#properties.get(dependency.identifier);
156
+ if (dependency.identifier.name === null && propDep !== undefined) {
157
maybeDependency = propDep;
158
} else {
159
maybeDependency = dependency;
@@ -163,7 +169,7 @@ class Context {
169
// if originalDeclaration is undefined here, then this is a free var
170
// (all other decls e.g. `let x;` should be initialized in BuildHIR)
171
const originalDeclaration = this.#declarations.get(
166
- maybeDependency.place.identifier.id
172
+ maybeDependency.identifier.id
173
);
174
if (
175
originalDeclaration !== undefined &&
@@ -171,16 +177,16 @@ class Context {
177
!this.#isScopeActive(originalDeclaration.scope)
178
) {
179
originalDeclaration.scope.declarations.set(
174
- maybeDependency.place.identifier.id,
175
- maybeDependency.place.identifier
180
+ maybeDependency.identifier.id,
181
+ maybeDependency.identifier
182
);
183
}
184
185
// If this operand is used in a scope, has a dynamic value, and was defined
186
// before this scope, then its a dependency of the scope.
187
const currentDeclaration =
182
- this.#reassignments.get(maybeDependency.place.identifier) ??
183
- this.#declarations.get(maybeDependency.place.identifier.id);
188
+ this.#reassignments.get(maybeDependency.identifier) ??
189
+ this.#declarations.get(maybeDependency.identifier.id);
190
const currentScope = this.currentScope;
191
if (
192
currentScope != null &&
@@ -195,7 +201,7 @@ class Context {
201
// Check if there is an existing dependency that describes this operand
202
for (const dep of this.#dependencies) {
203
// not the same identifier
198
- if (dep.place.identifier.id !== maybeDependency.place.identifier.id) {
204
+ if (dep.identifier.id !== maybeDependency.identifier.id) {
205
continue;
206
}
207
const depPath = dep.path ?? [];
compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts
+1
-1
@@ -26,7 +26,7 @@ class Visitor extends ReactiveFunctionVisitor<State> {
26
override visitScope(scope: ReactiveScopeBlock, state: State): void {
27
this.traverseScope(scope, state);
28
for (const dep of scope.scope.dependencies) {
29
- const isReactive = state.has(dep.place.identifier.id);
29
+ const isReactive = state.has(dep.identifier.id);
30
if (!isReactive) {
31
scope.scope.dependencies.delete(dep);
32
}