Use distinct type for scope dependencies
This is a pre-req to deleting the `Place.memberPath` field. We no longer need memberPath in the HIR now that we have PropertyStore and PropertyLoad. However, scope dependencies use memberPath to track the precise fields that a computation depends upon. This PR changes scopes dependencies to use a new `ReactiveScopeDependency` type (Place + optional path), which allows the next PR to remove `Place.memberPath`.
Joe Savona committed
Jan 3, 2023 at 15:34 UTC
c69cba357f5b24ec110ecac40b30de48fa83ae3a
4 files changed
+62
-27
compiler/forget/src/HIR/HIR.ts
+6
-1
@@ -388,10 +388,15 @@ export enum Effect {
388
export type ReactiveScope = {
389
id: ScopeId;
390
range: MutableRange;
391
- dependencies: Set<Place>;
391
+ dependencies: Set<ReactiveScopeDependency>;
392
outputs: Set<Identifier>;
393
};
394
395
+export type ReactiveScopeDependency = {
396
+ place: Place;
397
+ path: Array<string> | null;
398
+};
399
+
400
/**
401
* Simulated opaque type for BlockIds to prevent using normal numbers as block ids
402
* accidentally.
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+15
-1
@@ -23,6 +23,7 @@ import {
23
ReactiveBlock,
24
ReactiveFunction,
25
ReactiveScope,
26
+ ReactiveScopeDependency,
27
ReactiveTerminal,
28
ReactiveValueBlock,
29
} from "../HIR/HIR";
@@ -137,7 +138,7 @@ function codegenReactiveScope(
138
for (const dep of scope.dependencies) {
139
const index = cx.nextCacheIndex;
140
const changeIdentifier = t.identifier(`c_${index}`);
140
- const depValue = codegenPlace(cx.temp, dep);
141
+ const depValue = codegenDependency(cx, dep);
142
143
changeIdentifiers.push(changeIdentifier);
144
statements.push(
@@ -380,3 +381,16 @@ function codegenValueBlock(
381
return t.sequenceExpression(expressions);
382
}
383
}
384
+
385
+function codegenDependency(
386
+ cx: Context,
387
+ dependency: ReactiveScopeDependency
388
+): t.Expression {
389
+ let object: t.Expression = convertIdentifier(dependency.place.identifier);
390
+ if (dependency.path !== null) {
391
+ for (const path of dependency.path) {
392
+ object = t.memberExpression(object, t.identifier(path));
393
+ }
394
+ }
395
+ return object;
396
+}
compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts
+11
-1
@@ -9,6 +9,7 @@ import invariant from "invariant";
9
import {
10
ReactiveFunction,
11
ReactiveScopeBlock,
12
+ ReactiveScopeDependency,
13
ReactiveStatement,
14
ReactiveTerminal,
15
ReactiveValueBlock,
@@ -43,7 +44,7 @@ export function printReactiveBlock(
44
`scope @${block.scope.id} [${block.scope.range.start}:${
45
block.scope.range.end
46
}] deps=[${Array.from(block.scope.dependencies)
46
- .map((dep) => printPlace(dep))
47
+ .map((dep) => printDependency(dep))
48
.join(", ")}] out=[${Array.from(block.scope.outputs)
49
.map((out) => printIdentifier(out))
50
.join(", ")}] {`
@@ -52,6 +53,15 @@ export function printReactiveBlock(
53
writer.writeLine("}");
54
}
55
56
+function printDependency(dependency: ReactiveScopeDependency): string {
57
+ const place = printPlace(dependency.place);
58
+ if (dependency.path === null) {
59
+ return place;
60
+ } else {
61
+ return `${place}${dependency.path.map((prop) => `.${prop}`).join("")}`;
62
+ }
63
+}
64
+
65
export function printReactiveInstructions(
66
writer: Writer,
67
instructions: Array<ReactiveStatement>
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+30
-24
@@ -17,6 +17,7 @@ import {
17
ReactiveBlock,
18
ReactiveFunction,
19
ReactiveScope,
20
+ ReactiveScopeDependency,
21
ReactiveValueBlock,
22
} from "../HIR/HIR";
23
import { eachInstructionValueOperand } from "../HIR/visitors";
@@ -55,13 +56,13 @@ type Scopes = Array<ReactiveScope>;
56
57
class Context {
58
#declarations: DeclMap = new Map();
58
- #dependencies: Set<Place> = new Set();
59
- #properties: Map<Identifier, Place> = new Map();
59
+ #dependencies: Set<ReactiveScopeDependency> = new Set();
60
+ #properties: Map<Identifier, ReactiveScopeDependency> = new Map();
61
#scopes: Scopes = [];
62
62
- enter(scope: ReactiveScope, fn: () => void): Set<Place> {
63
+ enter(scope: ReactiveScope, fn: () => void): Set<ReactiveScopeDependency> {
64
const previousDependencies = this.#dependencies;
64
- const scopedDependencies = new Set<Place>();
65
+ const scopedDependencies = new Set<ReactiveScopeDependency>();
66
this.#dependencies = scopedDependencies;
67
this.#scopes.push(scope);
68
fn();
@@ -83,17 +84,17 @@ class Context {
84
object.memberPath === null,
85
"Expected operands to have null memberPath"
86
);
86
- const objectPlace = this.#properties.get(object.identifier);
87
- let place: Place;
88
- if (objectPlace === undefined) {
89
- place = { ...object, memberPath: [property] };
87
+ const objectDependency = this.#properties.get(object.identifier);
88
+ let nextDependency: ReactiveScopeDependency;
89
+ if (objectDependency === undefined) {
90
+ nextDependency = { place: object, path: [property] };
91
} else {
91
- place = {
92
- ...objectPlace,
93
- memberPath: [...(objectPlace.memberPath ?? []), property],
92
+ nextDependency = {
93
+ place: objectDependency.place,
94
+ path: [...(objectDependency.path ?? []), property],
95
};
96
}
96
- this.#properties.set(lvalue.identifier, place);
97
+ this.#properties.set(lvalue.identifier, nextDependency);
98
}
99
100
#isScopeActive(scope: ReactiveScope): boolean {
@@ -104,27 +105,32 @@ class Context {
105
return this.#scopes.at(-1)!;
106
}
107
107
- visitOperand(operand: Place): void {
108
- let maybeDependency: Place;
109
- if (operand.memberPath !== null) {
108
+ visitOperand(place: Place): void {
109
+ this.visitDependency({ place, path: null });
110
+ }
111
+
112
+ visitDependency(dependency: ReactiveScopeDependency): void {
113
+ let maybeDependency: ReactiveScopeDependency;
114
+ if (dependency.path !== null) {
115
// Operands may have memberPaths when propagating depenencies of an inner scope upward
116
// In this case we use the dependency as-is
112
- maybeDependency = operand;
117
+ maybeDependency = dependency;
118
} else {
119
// Otherwise if this operand is a temporary created for a property load, resolve it to
120
// the expanded Place. Fall back to using the operand as-is.
116
- maybeDependency = this.#properties.get(operand.identifier) ?? operand;
121
+ maybeDependency =
122
+ this.#properties.get(dependency.place.identifier) ?? dependency;
123
}
124
119
- const decl = this.#declarations.get(maybeDependency.identifier);
125
+ const decl = this.#declarations.get(maybeDependency.place.identifier);
126
127
// Any value used after its defining scope has concluded must be added as an
128
// output of its defining scope. Regardless of whether its a const or not,
129
// some later code needs access to the value.
130
if (decl !== undefined) {
125
- const operandScope = maybeDependency.identifier.scope;
131
+ const operandScope = maybeDependency.place.identifier.scope;
132
if (operandScope !== null && !this.#isScopeActive(operandScope)) {
127
- operandScope.outputs.add(maybeDependency.identifier);
133
+ operandScope.outputs.add(maybeDependency.place.identifier);
134
}
135
}
136
@@ -140,15 +146,15 @@ class Context {
146
// Check if there is an existing dependency that describes this operand
147
for (const dep of this.#dependencies) {
148
// not the same identifier
143
- if (dep.identifier !== maybeDependency.identifier) {
149
+ if (dep.place.identifier !== maybeDependency.place.identifier) {
150
continue;
151
}
146
- const depPath = dep.memberPath;
152
+ const depPath = dep.path;
153
// existing dep covers all paths
154
if (depPath === null) {
155
return;
156
}
151
- const operandPath = maybeDependency.memberPath;
157
+ const operandPath = maybeDependency.path;
158
// existing dep is for a path, this operand covers all paths so swap them
159
if (operandPath === null) {
160
this.#dependencies.delete(dep);
@@ -187,7 +193,7 @@ function visit(context: Context, block: ReactiveBlock): void {
193
// normal dependency collection. child scopes may have dependencies
194
// on values created within the outer scope, which necessarily cannot
195
// be dependencies of the outer scope
190
- context.visitOperand(dep);
196
+ context.visitDependency(dep);
197
}
198
break;
199
}