Use IdentifierIds to when comparing Identifier
With the upcoming changes to SSA renaming in #1194, we rewrite phi operand identifiers to have the same IdentifierId as the declaration the identifier originated from: so downstream checks need to compare ids instead of the identifier instance.
Lauren Tan committed
Feb 13, 2023 at 16:52 UTC
a76627c9725baf996ce1ea5e83a9042f841ef6f1
6 files changed
+18
-14
compiler/forget/src/HIR/HIR.ts
+1
-1
@@ -556,7 +556,7 @@ export type ReactiveScope = {
556
id: ScopeId;
557
range: MutableRange;
558
dependencies: Set<ReactiveScopeDependency>;
559
- declarations: Set<Identifier>;
559
+ declarations: Map<IdentifierId, Identifier>;
560
reassignments: Set<Identifier>;
561
};
562
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+4
-4
@@ -76,7 +76,7 @@ export function codegenReactiveFunction(
76
77
class Context {
78
#nextCacheIndex: number = 0;
79
- #declarations: Set<Identifier> = new Set();
79
+ #declarations: Set<IdentifierId> = new Set();
80
temp: Temporaries = new Map();
81
errors: CompilerError = new CompilerError();
82
@@ -85,11 +85,11 @@ class Context {
85
}
86
87
declare(identifier: Identifier): void {
88
- this.#declarations.add(identifier);
88
+ this.#declarations.add(identifier.id);
89
}
90
91
hasDeclared(identifier: Identifier): boolean {
92
- return this.#declarations.has(identifier);
92
+ return this.#declarations.has(identifier.id);
93
}
94
}
95
@@ -179,7 +179,7 @@ function codegenReactiveScope(
179
);
180
}
181
let firstOutputIndex: number | null = null;
182
- for (const declaration of scope.declarations) {
182
+ for (const [, declaration] of scope.declarations) {
183
const index = cx.nextCacheIndex;
184
if (firstOutputIndex === null) {
185
firstOutputIndex = index;
compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts
+1
-1
@@ -127,7 +127,7 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void {
127
id: makeScopeId(scopes.size),
128
range: identifier.mutableRange,
129
dependencies: new Set(),
130
- declarations: new Set(),
130
+ declarations: new Map(),
131
reassignments: new Set(),
132
};
133
scopes.set(groupIdentifier, scope);
compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts
+1
-1
@@ -46,7 +46,7 @@ export function printReactiveBlock(
46
}] dependencies=[${Array.from(block.scope.dependencies)
47
.map((dep) => printDependency(dep))
48
.join(", ")}] declarations=[${Array.from(block.scope.declarations)
49
- .map((decl) => printIdentifier(decl))
49
+ .map(([, decl]) => printIdentifier(decl))
50
.join(", ")}] reassignments=[${Array.from(block.scope.reassignments).map(
51
(reassign) => printIdentifier(reassign)
52
)}] {`
compiler/forget/src/ReactiveScopes/PromoteUsedTemporaries.ts
+1
-1
@@ -33,7 +33,7 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
33
// value.
34
// Many of our current test fixtures do not return a value, so
35
// it is better for now to promote (and memoize) every output.
36
- for (const identifier of block.scope.declarations) {
36
+ for (const [, identifier] of block.scope.declarations) {
37
if (identifier.name == null) {
38
identifier.name = `t${state.nextId++}`;
39
}
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+10
-6
@@ -7,6 +7,7 @@
7
8
import {
9
Identifier,
10
+ IdentifierId,
11
InstructionId,
12
InstructionKind,
13
LValue,
@@ -54,7 +55,7 @@ enum DeclKind {
55
Dynamic = "Dynamic",
56
}
57
57
-type DeclMap = Map<Identifier, Decl>;
58
+type DeclMap = Map<IdentifierId, Decl>;
59
type Decl = {
60
kind: DeclKind;
61
id: InstructionId;
@@ -94,7 +95,7 @@ class Context {
95
* on itself.
96
*/
97
declare(identifier: Identifier, decl: Decl): void {
97
- this.#declarations.set(identifier, decl);
98
+ this.#declarations.set(identifier.id, decl);
99
}
100
101
declareProperty(lvalue: Place, object: Place, property: string): void {
@@ -158,7 +159,7 @@ class Context {
159
}
160
}
161
161
- const decl = this.#declarations.get(maybeDependency.place.identifier);
162
+ const decl = this.#declarations.get(maybeDependency.place.identifier.id);
163
// if decl is undefined here, then this is a free var
164
// (all other decls e.g. `let x;` should be initialized in BuildHIR)
165
@@ -172,7 +173,10 @@ class Context {
173
decl.scope !== null &&
174
!this.#isScopeActive(decl.scope)
175
) {
175
- decl.scope.declarations.add(maybeDependency.place.identifier);
176
+ decl.scope.declarations.set(
177
+ maybeDependency.place.identifier.id,
178
+ maybeDependency.place.identifier
179
+ );
180
}
181
182
// If this operand is used in a scope, has a dynamic value, and was defined
@@ -188,7 +192,7 @@ class Context {
192
// Check if there is an existing dependency that describes this operand
193
for (const dep of this.#dependencies) {
194
// not the same identifier
191
- if (dep.place.identifier !== maybeDependency.place.identifier) {
195
+ if (dep.place.identifier.id !== maybeDependency.place.identifier.id) {
196
continue;
197
}
198
const depPath = dep.path;
@@ -229,7 +233,7 @@ class Context {
233
if (lvalue.kind !== InstructionKind.Reassign) {
234
return;
235
}
232
- const declaration = this.#declarations.get(lvalue.place.identifier);
236
+ const declaration = this.#declarations.get(lvalue.place.identifier.id);
237
if (
238
this.currentScope != null &&
239
lvalue.place.identifier.scope != null &&