Construct LoadGlobal; consume hook info from types
Updates BuildHIR to produce LoadGlobal instructions for references to globals. Note that this breaks our previous strategy of finding hook calls: that relied on looking at the callee of a CallExpression and checking its name, which relied on the callee not being lowered to a temporary. By lowering the name (eg `useState`) to a temporary first, we now no longer see the name at the callsite. Thankfully @gsathya solved this for us already by teaching type inference about hooks, and more generally implementing type inference. I updated this so that we infer the type of a LoadGlobal if the name is a hook: the type inference picks this up and propagates the type forward correctly. So now, all places that needed to check for a hook can just look at the type and everything works. This is much more robust than before - you can now reassign a hook to a local variable and we'll still detect that when you call it, you're calling a hook.
Joe Savona committed
Feb 17, 2023 at 09:47 UTC
6b654f306ceb204ec0b2b7fd5556f92a512aca57
11 files changed
+107
-63
compiler/forget/src/HIR/BuildHIR.ts
+33
-9
@@ -79,6 +79,14 @@ export function lower(
79
func.get("params").forEach((param) => {
80
if (param.isIdentifier()) {
81
const identifier = builder.resolveIdentifier(param);
82
+ if (identifier === null) {
83
+ builder.errors.push({
84
+ reason: `(BuildHIR::lower) Could not find binding for param '${param.node.name}'`,
85
+ severity: ErrorSeverity.Invariant,
86
+ nodePath: param,
87
+ });
88
+ return;
89
+ }
90
const place: Place = {
91
kind: "Identifier",
92
identifier,
@@ -1591,14 +1599,7 @@ function lowerJsxElementName(
1599
}
1600
const tag: string = exprPath.node.name;
1601
if (tag.match(/^[A-Z]/)) {
1594
- const identifier = builder.resolveIdentifier(exprPath);
1595
- const place: Place = {
1596
- kind: "Identifier",
1597
- identifier: identifier,
1598
- effect: Effect.Unknown,
1599
- loc: exprLoc,
1600
- };
1601
- return place;
1602
+ return lowerIdentifier(builder, exprPath);
1603
} else {
1604
const place: Place = buildTemporaryPlace(builder, exprLoc);
1605
builder.push({
@@ -1728,11 +1729,34 @@ function lowerExpressionToVoid(
1729
1730
function lowerIdentifier(
1731
builder: HIRBuilder,
1731
- exprPath: NodePath<t.Identifier>
1732
+ exprPath: NodePath<t.Identifier | t.JSXIdentifier>
1733
): Place {
1734
const exprNode = exprPath.node;
1735
const exprLoc = exprNode.loc ?? GeneratedSource;
1736
const identifier = builder.resolveIdentifier(exprPath);
1737
+ if (identifier === null) {
1738
+ const place = buildTemporaryPlace(
1739
+ builder,
1740
+ exprPath.node.loc ?? GeneratedSource
1741
+ );
1742
+ const global = builder.resolveGlobal(exprPath);
1743
+ let value: InstructionValue;
1744
+ if (global !== null) {
1745
+ value = { kind: "LoadGlobal", name: global.name, loc: place.loc };
1746
+ } else {
1747
+ value = { kind: "UnsupportedNode", node: exprPath.node, loc: place.loc };
1748
+ }
1749
+ builder.push({
1750
+ id: makeInstructionId(0),
1751
+ value,
1752
+ loc: place.loc,
1753
+ lvalue: {
1754
+ place: { ...place },
1755
+ kind: InstructionKind.Const,
1756
+ },
1757
+ });
1758
+ return place;
1759
+ }
1760
const place: Place = {
1761
kind: "Identifier",
1762
identifier: identifier,
compiler/forget/src/HIR/Globals.ts
+7
-4
@@ -13,14 +13,17 @@ const GLOBALS: Map<string, t.Identifier> = new Map([
13
["Math", t.identifier("Math")],
14
]);
15
16
+export type Global = {
17
+ name: string;
18
+};
19
+
20
// TODO: This will work as a stopgap but it isn't really correct. We need proper handling of globals
21
// and module-scoped variables, which means understanding module constants and imports.
18
-export function getOrAddGlobal(identifierName: string): t.Identifier {
22
+export function getGlobalDeclaration(identifierName: string): Global | null {
23
const ident = GLOBALS.get(identifierName);
24
if (ident != null) {
25
return ident;
26
}
23
- const newIdent = t.identifier(identifierName);
24
- GLOBALS.set(identifierName, newIdent);
25
- return newIdent;
27
+ // TODO: return null if not explicitly configured by the user
28
+ return { name: identifierName };
29
}
compiler/forget/src/HIR/HIR.ts
+5
-2
@@ -8,6 +8,7 @@
8
import * as t from "@babel/types";
9
import invariant from "invariant";
10
import { Environment } from "./Environment";
11
+import { Hook } from "./Hooks";
12
13
// *******************************************************************************************
14
// *******************************************************************************************
@@ -641,7 +642,7 @@ export type FunctionType = {
642
};
643
export type HookType = {
644
kind: "Hook";
644
- name: string;
645
+ definition: Hook;
646
};
647
export type ObjectType = { kind: "Object" };
648
export type TypeVar = {
@@ -720,7 +721,9 @@ function funcTypeEquals(tA: Type, tB: Type): boolean {
721
}
722
723
function hookTypeEquals(tA: Type, tB: Type): boolean {
723
- return tA.kind === "Hook" && tB.kind === "Hook" && tA.name === tB.name;
724
+ return (
725
+ tA.kind === "Hook" && tB.kind === "Hook" && tA.definition === tB.definition
726
+ );
727
}
728
729
function phiTypeEquals(tA: Type, tB: Type): boolean {
compiler/forget/src/HIR/HIRBuilder.ts
+11
-6
@@ -12,7 +12,7 @@ import { CompilerError } from "../CompilerError";
12
import { logHIR } from "../Utils/logger";
13
import { assertExhaustive } from "../Utils/utils";
14
import { Environment } from "./Environment";
15
-import { getOrAddGlobal } from "./Globals";
15
+import { getGlobalDeclaration, Global } from "./Globals";
16
import {
17
BasicBlock,
18
BlockId,
@@ -136,6 +136,10 @@ export default class HIRBuilder {
136
};
137
}
138
139
+ resolveGlobal(path: NodePath<t.Identifier | t.JSXIdentifier>): Global | null {
140
+ return getGlobalDeclaration(path.node.name);
141
+ }
142
+
143
/**
144
* Maps an Identifier (or JSX identifier) Babel node to an internal `Identifier`
145
* which represents the variable being referenced, according to the JS scoping rules.
@@ -168,15 +172,16 @@ export default class HIRBuilder {
172
*/
173
resolveIdentifier(
174
path: NodePath<t.Identifier | t.JSXIdentifier>
171
- ): Identifier {
175
+ ): Identifier | null {
176
const originalName = path.node.name;
173
- const node =
174
- path.scope.getBindingIdentifier(originalName) ??
175
- getOrAddGlobal(originalName);
177
+ const node = path.scope.getBindingIdentifier(originalName);
178
+ if (node == null) {
179
+ return null;
180
+ }
181
return this.resolveBinding(node);
182
}
183
179
- resolveBinding(node: t.Identifier) {
184
+ resolveBinding(node: t.Identifier): Identifier {
185
const originalName = node.name;
186
let name = originalName;
187
let index = 0;
compiler/forget/src/Inference/DropMemoCalls.ts
+3
-9
@@ -1,11 +1,4 @@
1
-import invariant from "invariant";
2
-import {
3
- Effect,
4
- HIRFunction,
5
- HookType,
6
- InstructionValue,
7
- isHookType,
8
-} from "../HIR";
1
+import { Effect, HIRFunction, HookType, isHookType } from "../HIR";
2
3
export default function (func: HIRFunction) {
4
for (const [_, block] of func.body.blocks) {
@@ -13,7 +6,8 @@ export default function (func: HIRFunction) {
6
switch (instr.value.kind) {
7
case "CallExpression": {
8
if (isHookType(instr.value.callee.identifier)) {
16
- const name = (instr.value.callee.identifier.type as HookType).name;
9
+ const name = (instr.value.callee.identifier.type as HookType)
10
+ .definition.name;
11
if (name === "useMemo") {
12
const [fn] = instr.value.args;
13
compiler/forget/src/Inference/InferReferenceEffects.ts
+2
-2
@@ -589,8 +589,8 @@ function inferBlock(
589
valueKind = ValueKind.Mutable;
590
effectKind = Effect.Mutate;
591
const hook =
592
- instrValue.callee.identifier.name !== null
593
- ? env.getHookDeclaration(instrValue.callee.identifier.name)
592
+ instrValue.callee.identifier.type.kind === "Hook"
593
+ ? instrValue.callee.identifier.type.definition
594
: null;
595
if (hook !== null) {
596
effectKind = hook.effectKind;
compiler/forget/src/ReactiveScopes/FlattenScopesWithHooks.ts
+4
-14
@@ -6,8 +6,8 @@
6
*/
7
8
import {
9
- Environment,
9
InstructionId,
10
+ isHookType,
11
ReactiveFunction,
12
ReactiveScopeBlock,
13
ReactiveStatement,
@@ -31,19 +31,12 @@ import {
31
* to ensure the hook call does not inadvertently become conditional.
32
*/
33
export function flattenScopesWithHooks(fn: ReactiveFunction): void {
34
- visitReactiveFunction(fn, new Transform(fn.env), { hasHook: false });
34
+ visitReactiveFunction(fn, new Transform(), { hasHook: false });
35
}
36
37
type State = { hasHook: boolean };
38
39
class Transform extends ReactiveFunctionTransform<State> {
40
- env: Environment;
41
-
42
- constructor(env: Environment) {
43
- super();
44
- this.env = env;
45
- }
46
-
40
override transformScope(
41
scope: ReactiveScopeBlock,
42
outerState: State
@@ -65,12 +58,9 @@ class Transform extends ReactiveFunctionTransform<State> {
58
): void {
59
if (
60
value.kind === "CallExpression" &&
68
- value.callee.identifier.name !== null
61
+ isHookType(value.callee.identifier)
62
) {
70
- const hook = this.env.getHookDeclaration(value.callee.identifier.name);
71
- if (hook !== null) {
72
- state.hasHook = true;
73
- }
63
+ state.hasHook = true;
64
}
65
}
66
}
compiler/forget/src/ReactiveScopes/InferReactiveIdentifiers.ts
+4
-16
@@ -6,10 +6,10 @@
6
*/
7
8
import { CompilerError } from "../CompilerError";
9
-import { Environment } from "../HIR";
9
import {
10
Effect,
11
IdentifierId,
12
+ isHookType,
13
ReactiveFunction,
14
ReactiveInstruction,
15
} from "../HIR/HIR";
@@ -22,13 +22,6 @@ import {
22
23
type IdentifierReactivity = Map<IdentifierId, boolean>;
24
class Visitor extends ReactiveFunctionVisitor<IdentifierReactivity> {
25
- env: Environment;
26
-
27
- constructor(env: Environment) {
28
- super();
29
- this.env = env;
30
- }
31
-
25
override visitInstruction(
26
instr: ReactiveInstruction,
27
reactivityMap: IdentifierReactivity
@@ -52,19 +45,14 @@ class Visitor extends ReactiveFunctionVisitor<IdentifierReactivity> {
45
if (
46
!hasReactiveInput &&
47
instr.value.kind === "CallExpression" &&
55
- instr.value.callee.identifier.name !== null
48
+ isHookType(instr.value.callee.identifier)
49
) {
50
// Hooks cannot be memoized. Even if they do not accept any reactive inputs,
51
// they are not guaranteed to memoize their return value, and their result
52
// must be assumed to be reactive.
53
// TODO: use types or an opt-in registry of custom hook information to
54
// allow treating safe hooks as non-reactive.
62
- const hook = this.env.getHookDeclaration(
63
- instr.value.callee.identifier.name
64
- );
65
- if (hook !== null) {
66
- hasReactiveInput = true;
67
- }
55
+ hasReactiveInput = true;
56
}
57
reactivityMap.set(lval.place.identifier.id, hasReactiveInput);
58
@@ -143,7 +131,7 @@ class Visitor extends ReactiveFunctionVisitor<IdentifierReactivity> {
131
export function inferReactiveIdentifiers(
132
fn: ReactiveFunction
133
): Set<IdentifierId> {
146
- const visitor = new Visitor(fn.env);
134
+ const visitor = new Visitor();
135
const reactivityMap: IdentifierReactivity = new Map();
136
for (const param of fn.params) {
137
reactivityMap.set(param.identifier.id, true);
compiler/forget/src/TypeInference/InferTypes.ts
+10
-1
@@ -121,6 +121,15 @@ function* generateInstructionTypes(
121
break;
122
}
123
124
+ case "LoadGlobal": {
125
+ const hook = env.getHookDeclaration(value.name);
126
+ if (hook !== null) {
127
+ const type: Type = { kind: "Hook", definition: hook };
128
+ yield equation(left, type);
129
+ }
130
+ break;
131
+ }
132
+
133
case "CallExpression": {
134
const hook =
135
value.callee.identifier.name !== null
@@ -128,7 +137,7 @@ function* generateInstructionTypes(
137
: null;
138
let type: Type;
139
if (hook !== null) {
131
- type = { kind: "Hook", name: hook.name };
140
+ type = { kind: "Hook", definition: hook };
141
} else {
142
type = { kind: "Function" };
143
}
compiler/forget/src/__tests__/fixtures/hir/useRef-rename-mutable.expect.md
new
+23
@@ -0,0 +1,23 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ const x = useRef;
7
+ const ref = x(null);
8
+ return ref.current;
9
+}
10
+
11
+```
12
+
13
+## Code
14
+
15
+```javascript
16
+function Component(props) {
17
+ const x = useRef;
18
+ const ref = x(null);
19
+ return ref.current;
20
+}
21
+
22
+```
23
+
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/hir/useRef-rename-mutable.js
new
+5
@@ -0,0 +1,5 @@
1
+function Component(props) {
2
+ const x = useRef;
3
+ const ref = x(null);
4
+ return ref.current;
5
+}