Rename variables during BuildHIR
Per design discussion, this PR changes BuildHIR to maintain the invariant that, for each distinct variable in the input, that all references to that variable in the HIR will have the same unique `name` _and_ same unique `id`. Phrased differently: Identifiers with the same id will have the same name and vice-versa. This isn't an invariant we maintain throughout compilation — SSA form changes the `id`s — but crucially, ensuring that the `name` is also unique allows us to understand later which identifiers referred to the same original variable and which were different. Follow-up PRs will ensure that we maintain variable identifiers in the output as well, in all cases except shadowing (and for shadowing, we'll rewrite identifiers inside lambdas).
Joe Savona committed
Jan 17, 2023 at 09:40 UTC
2e501b0f01df960b3ed7cc63df3d7a208220c25d
5 files changed
+106
-68
compiler/forget/src/HIR/BuildHIR.ts
+11
-46
@@ -34,24 +34,6 @@ import HIRBuilder, { Environment } from "./HIRBuilder";
34
// *******************************************************************************************
35
// *******************************************************************************************
36
37
-const GLOBALS: Map<string, t.Identifier> = new Map([
38
- ["Map", t.identifier("Map")],
39
- ["Set", t.identifier("Set")],
40
- ["Math", t.identifier("Math")],
41
-]);
42
-
43
-// TODO: This will work as a stopgap but it isn't really correct. We need proper handling of globals
44
-// and module-scoped variables, which means understanding module constants and imports.
45
-function getOrAddGlobal(identifierName: string): t.Identifier {
46
- const ident = GLOBALS.get(identifierName);
47
- if (ident != null) {
48
- return ident;
49
- }
50
- const newIdent = t.identifier(identifierName);
51
- GLOBALS.set(identifierName, newIdent);
52
- return newIdent;
53
-}
54
-
37
/**
38
* Lower a function declaration into a control flow graph that models aspects of
39
* control flow that are necessary for memoization. Notably, only control flow
@@ -70,14 +52,14 @@ export function lower(
52
const builder = new HIRBuilder(env);
53
54
const id =
73
- func.isFunctionDeclaration() && func.node.id != null
74
- ? builder.resolveIdentifier(func.node.id)
55
+ func.isFunctionDeclaration() && func.get("id").hasNode()
56
+ ? builder.resolveIdentifier(func.get("id") as NodePath<t.Identifier>)
57
: null;
58
59
const params: Array<Place> = [];
78
- for (const param of func.get("params")) {
60
+ func.get("params").forEach((param) => {
61
if (param.isIdentifier()) {
80
- const identifier = builder.resolveIdentifier(param.node);
62
+ const identifier = builder.resolveIdentifier(param);
63
const place: Place = {
64
kind: "Identifier",
65
identifier,
@@ -97,9 +79,8 @@ export function lower(
79
source: func.toString(),
80
loc: param.node.loc ?? null,
81
});
100
- continue;
82
}
102
- }
83
+ });
84
85
const body = func.get("body");
86
if (body.isExpression()) {
@@ -1360,14 +1341,7 @@ function lowerJsxElementName(
1341
const exprLoc = exprPath.node.loc ?? GeneratedSource;
1342
const tag: string = exprPath.node.name;
1343
if (tag.match(/^[A-Z]/)) {
1363
- const binding =
1364
- exprPath.scope.getBindingIdentifier(tag) ?? getOrAddGlobal(tag);
1365
- invariant(
1366
- binding != null,
1367
- `Expected to find a binding for variable '%s'`,
1368
- tag
1369
- );
1370
- const identifier = builder.resolveIdentifier(binding);
1344
+ const identifier = builder.resolveIdentifier(exprPath);
1345
const place: Place = {
1346
kind: "Identifier",
1347
identifier: identifier,
@@ -1487,15 +1461,7 @@ function lowerIdentifier(
1461
): Place {
1462
const exprNode = exprPath.node;
1463
const exprLoc = exprNode.loc ?? GeneratedSource;
1490
- const binding =
1491
- exprPath.scope.getBindingIdentifier(exprNode.name) ??
1492
- getOrAddGlobal(exprNode.name);
1493
- invariant(
1494
- binding != null,
1495
- `Expected to find a binding for variable '%s'`,
1496
- exprNode.name
1497
- );
1498
- const identifier = builder.resolveIdentifier(binding);
1464
+ const identifier = builder.resolveIdentifier(exprPath);
1465
const place: Place = {
1466
kind: "Identifier",
1467
identifier: identifier,
@@ -1694,21 +1660,20 @@ function gatherCapturedDeps(
1660
fn.get("body").traverse({
1661
Expression(path) {
1662
// TODO(gsn): Handle member expressions
1697
- if (!path.isIdentifier) {
1663
+ if (!path.isIdentifier()) {
1664
return;
1665
}
1666
1701
- const id = path as NodePath<t.Identifier>;
1702
- const binding = id.scope.getBinding(id.node.name);
1667
+ const binding = path.scope.getBinding(path.node.name);
1668
if (binding === undefined || !pureScopes.has(binding.scope)) {
1669
return;
1670
}
1671
1672
captured.add({
1673
kind: "Identifier",
1709
- identifier: builder.resolveIdentifier(binding.identifier),
1674
+ identifier: builder.resolveIdentifier(path),
1675
effect: Effect.Unknown,
1711
- loc: id.node.loc!,
1676
+ loc: path.node.loc ?? GeneratedSource,
1677
});
1678
},
1679
});
compiler/forget/src/HIR/Globals.ts
new
+26
@@ -0,0 +1,26 @@
1
+/**
2
+ * Copyright (c) Facebook, Inc. and its affiliates.
3
+ *
4
+ * This source code is licensed under the MIT license found in the
5
+ * LICENSE file in the root directory of this source tree.
6
+ */
7
+
8
+import * as t from "@babel/types";
9
+
10
+const GLOBALS: Map<string, t.Identifier> = new Map([
11
+ ["Map", t.identifier("Map")],
12
+ ["Set", t.identifier("Set")],
13
+ ["Math", t.identifier("Math")],
14
+]);
15
+
16
+// TODO: This will work as a stopgap but it isn't really correct. We need proper handling of globals
17
+// and module-scoped variables, which means understanding module constants and imports.
18
+export function getOrAddGlobal(identifierName: string): t.Identifier {
19
+ const ident = GLOBALS.get(identifierName);
20
+ if (ident != null) {
21
+ return ident;
22
+ }
23
+ const newIdent = t.identifier(identifierName);
24
+ GLOBALS.set(identifierName, newIdent);
25
+ return newIdent;
26
+}
compiler/forget/src/HIR/HIRBuilder.ts
+64
-17
@@ -5,11 +5,13 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
+import { NodePath } from "@babel/traverse";
9
import * as t from "@babel/types";
10
import invariant from "invariant";
11
import { CompilerError, CompilerErrorOptions } from "../CompilerError";
12
import { logHIR } from "../Utils/logger";
13
import { assertExhaustive } from "../Utils/utils";
14
+import { getOrAddGlobal } from "./Globals";
15
import {
16
BasicBlock,
17
BlockId,
@@ -82,7 +84,8 @@ export default class HIRBuilder {
84
#current: WipBlock = newBlock(makeBlockId(0));
85
#entry: BlockId = makeBlockId(0);
86
#scopes: Array<Scope> = [];
85
- #bindings: Map<t.Identifier, Identifier> = new Map();
87
+ #bindings: Map<string, { node: t.Identifier; identifier: Identifier }> =
88
+ new Map();
89
#env: Environment;
90
errors: CompilerError[] = [];
91
@@ -124,23 +127,67 @@ export default class HIRBuilder {
127
};
128
}
129
127
- resolveIdentifier(node: t.Identifier): Identifier {
128
- let identifier = this.#bindings.get(node);
129
- if (identifier == null) {
130
- const id = this.nextIdentifierId;
131
- identifier = {
132
- id,
133
- name: node.name,
134
- mutableRange: {
135
- start: makeInstructionId(0),
136
- end: makeInstructionId(0),
137
- },
138
- scope: null,
139
- type: makeType(),
140
- };
141
- this.#bindings.set(node, identifier);
130
+ /**
131
+ * Maps an Identifier (or JSX identifier) Babel node to an internal `Identifier`
132
+ * which represents the variable being referenced, according to the JS scoping rules.
133
+ *
134
+ * Because Forget does not preserve _all_ block scopes in the input (only those that
135
+ * happen to occur from control flow), this resolution ensures that different variables
136
+ * with the same name are mapped to a unique name. Concretely, this function maintains
137
+ * the invariant that all references to a given variable will return an `Identifier`
138
+ * with the same (unique for the function) `name` and `id`.
139
+ *
140
+ * Example:
141
+ *
142
+ * ```javascript
143
+ * function foo() {
144
+ * const x = 0;
145
+ * {
146
+ * const x = 1;
147
+ * }
148
+ * return x;
149
+ * }
150
+ * ```
151
+ *
152
+ * The above converts as follows:
153
+ *
154
+ * ```
155
+ * Const Identifier { name: 'x', id: 0 } = Primitive { value: 0 };
156
+ * Const Identifier { name: 'x_0', id: 1 } = Primitive { value: 1 };
157
+ * Return Identifier { name: 'x', id: 0};
158
+ * ```
159
+ */
160
+ resolveIdentifier(
161
+ path: NodePath<t.Identifier | t.JSXIdentifier>
162
+ ): Identifier {
163
+ const originalName = path.node.name;
164
+ const node =
165
+ path.scope.getBindingIdentifier(originalName) ??
166
+ getOrAddGlobal(originalName);
167
+ let name = originalName;
168
+ let index = 0;
169
+ while (true) {
170
+ const mapping = this.#bindings.get(name);
171
+ if (mapping === undefined) {
172
+ const id = this.nextIdentifierId;
173
+ const identifier: Identifier = {
174
+ id,
175
+ name,
176
+ mutableRange: {
177
+ start: makeInstructionId(0),
178
+ end: makeInstructionId(0),
179
+ },
180
+ scope: null,
181
+ type: makeType(),
182
+ };
183
+ this.#bindings.set(name, { node, identifier });
184
+ return identifier;
185
+ } else if (mapping.node === node) {
186
+ return mapping.identifier;
187
+ } else {
188
+ name = `${originalName}_${index++}`;
189
+ }
190
}
143
- return identifier;
191
}
192
193
/**
compiler/forget/src/__tests__/fixtures/hir/ssa-shadowing.expect.md
+4
-4
@@ -35,11 +35,11 @@ function Foo(cond) {
35
str$0 = str;
36
37
if (cond) {
38
- const str$1 = "other test";
39
- log(str$1);
38
+ const str_0 = "other test";
39
+ log(str_0);
40
} else {
41
- const str$2 = "fallthrough test";
42
- str$0 = str$2;
41
+ const str$1 = "fallthrough test";
42
+ str$0 = str$1;
43
}
44
45
$[0] = cond;
compiler/forget/src/__tests__/fixtures/hir/type-test-return-type-inference.expect.md
+1
-1
@@ -40,7 +40,7 @@ function component() {
40
const z = {};
41
}
42
43
- const z = foo();
43
+ const z_0 = foo();
44
}
45
46
```