[valueblocks] Update some passes to use the new visitor
Updates some passes from ReactiveScopes/ to use the visitor added in the previous PR. The +124/-354 line count on this diff tells the story — the new visitor avoids a lot of boilerplate and helps focus on the logic not the traversal. Note that there are a few passes which transform the function such as adding/removing scopes. A follow-up will extend the visitor to support that and convert the remaining passes.
Joe Savona committed
Jan 31, 2023 at 13:39 UTC
50f300bc92a7edb684b0735e0cb46c1c866a4916
5 files changed
+124
-354
compiler/forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts
+30
-77
@@ -5,18 +5,18 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
-import invariant from "invariant";
8
import {
9
InstructionId,
11
- makeInstructionId,
10
+ LValue,
11
+ Place,
12
ReactiveBlock,
13
ReactiveFunction,
14
ReactiveScope,
15
ReactiveValueBlock,
16
ScopeId,
17
} from "../HIR/HIR";
18
-import { getInstructionScope, getPlaceScope } from "./BuildReactiveBlocks";
19
-import { eachTerminalBlock, eachTerminalOperand } from "./visitors";
18
+import { getPlaceScope } from "./BuildReactiveBlocks";
19
+import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
20
21
/**
22
* Note: this is the 2nd of 4 passes that determine how to break a function into discrete
@@ -62,86 +62,39 @@ import { eachTerminalBlock, eachTerminalOperand } from "./visitors";
62
63
export function alignReactiveScopesToBlockScopes(fn: ReactiveFunction): void {
64
const context = new Context();
65
- context.enter(() => {
66
- visitBlock(context, fn.body);
67
- });
65
+ visitReactiveFunction(fn, new Visitor(), context);
66
}
67
70
-function visitBlock(context: Context, block: ReactiveBlock): void {
71
- for (const stmt of block) {
72
- switch (stmt.kind) {
73
- case "instruction": {
74
- context.visitId(stmt.instruction.id);
75
- const scope = getInstructionScope(stmt.instruction);
76
- if (scope !== null) {
77
- context.visitScope(scope);
78
- }
79
- break;
80
- }
81
- case "terminal": {
82
- const id = stmt.terminal.id;
83
- if (id !== null) {
84
- context.visitId(id);
85
- }
86
- eachTerminalOperand(stmt.terminal, (operand) => {
87
- const scope = getPlaceScope(id!, operand);
88
- if (scope !== null) {
89
- context.visitScope(scope);
90
- }
91
- });
92
- eachTerminalBlock(
93
- stmt.terminal,
94
- (block) => {
95
- context.enter(() => visitBlock(context, block));
96
- },
97
- (valueBlock) => {
98
- context.enter(
99
- () => visitValueBlock(context, valueBlock, id!),
100
- "value"
101
- );
102
- }
103
- );
104
- break;
105
- }
106
- case "scope": {
107
- invariant(false, "Expected scopes to be constructed later");
108
- }
109
- }
68
+class Visitor extends ReactiveFunctionVisitor<Context> {
69
+ override visitID(id: InstructionId, state: Context): void {
70
+ state.visitId(id);
71
}
111
-}
112
-
113
-function visitValueBlock(
114
- context: Context,
115
- block: ReactiveValueBlock,
116
- start: InstructionId
117
-): void {
118
- for (const stmt of block.instructions) {
119
- switch (stmt.kind) {
120
- case "instruction": {
121
- context.visitId(stmt.instruction.id);
122
- const scope = getInstructionScope(stmt.instruction);
123
- if (scope !== null) {
124
- scope.range.start = makeInstructionId(
125
- Math.min(start, scope.range.start)
126
- );
127
- context.visitScope(scope);
128
- }
129
- break;
130
- }
131
- default: {
132
- invariant(false, "Unexpected terminal or scope in value block");
133
- }
72
+ override visitPlace(id: InstructionId, place: Place, state: Context): void {
73
+ const scope = getPlaceScope(id, place);
74
+ if (scope !== null) {
75
+ state.visitScope(scope);
76
}
77
}
136
- if (block.last !== null) {
137
- context.visitId(block.last.id);
138
- if (block.last.value.kind === "Identifier") {
139
- const scope = getPlaceScope(block.last.id, block.last.value);
140
- if (scope !== null) {
141
- context.visitScope(scope);
142
- }
78
+ override visitLValue(
79
+ id: InstructionId,
80
+ lvalue: LValue,
81
+ state: Context
82
+ ): void {
83
+ const scope = getPlaceScope(id, lvalue.place);
84
+ if (scope !== null) {
85
+ state.visitScope(scope);
86
}
87
}
88
+ override visitBlock(block: ReactiveBlock, state: Context): void {
89
+ state.enter(() => {
90
+ this.traverseBlock(block, state);
91
+ }, "block");
92
+ }
93
+ override visitValueBlock(block: ReactiveValueBlock, state: Context): void {
94
+ state.enter(() => {
95
+ super.visitValueBlock(block, state);
96
+ }, "value");
97
+ }
98
}
99
100
type PendingReactiveScope = { active: boolean; scope: ReactiveScope };
compiler/forget/src/ReactiveScopes/MergeOverlappingReactiveScopes.ts
+21
-89
@@ -5,27 +5,20 @@
5
* LICENSE file in the root directory of this source tree.
6
*/
7
8
-import invariant from "invariant";
8
import {
9
InstructionId,
10
+ LValue,
11
makeInstructionId,
12
Place,
13
ReactiveBlock,
14
ReactiveFunction,
15
- ReactiveInstruction,
15
ReactiveScope,
17
- ReactiveValue,
18
- ReactiveValueBlock,
16
ScopeId,
17
} from "../HIR";
18
import DisjointSet from "../Utils/DisjointSet";
19
import { retainWhere } from "../Utils/utils";
20
import { getPlaceScope } from "./BuildReactiveBlocks";
24
-import {
25
- eachReactiveValueOperand,
26
- eachTerminalBlock,
27
- eachTerminalOperand,
28
-} from "./visitors";
21
+import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
22
23
/**
24
* Note: this is the 3rd of 4 passes that determine how to break a function into discrete
@@ -106,92 +99,31 @@ import {
99
*/
100
export function mergeOverlappingReactiveScopes(fn: ReactiveFunction): void {
101
const context = new Context();
109
- context.enter(() => {
110
- visitBlock(context, fn.body);
111
- });
102
+ // context.enter(() => {
103
+ // visitBlock(context, fn.body);
104
+ // });
105
+ visitReactiveFunction(fn, new Visitor(), context);
106
context.complete();
107
}
108
115
-function visitBlock(context: Context, block: ReactiveBlock): void {
116
- for (const stmt of block) {
117
- switch (stmt.kind) {
118
- case "instruction": {
119
- visitValue(context, stmt.instruction.id, stmt.instruction.value);
120
- visitInstruction(context, stmt.instruction);
121
- break;
122
- }
123
- case "terminal": {
124
- const id = stmt.terminal.id;
125
- if (id !== null) {
126
- context.visitId(id);
127
- eachTerminalOperand(stmt.terminal, (operand) => {
128
- visitValue(context, id, operand);
129
- });
130
- }
131
- eachTerminalBlock(
132
- stmt.terminal,
133
- (block) => {
134
- context.enter(() => {
135
- visitBlock(context, block);
136
- });
137
- },
138
- (valueBlock) => {
139
- context.enter(() => {
140
- visitValueBlock(context, valueBlock);
141
- });
142
- }
143
- );
144
- break;
145
- }
146
- case "scope": {
147
- invariant(false, "Expected scopes to be constructed later");
148
- }
149
- }
109
+class Visitor extends ReactiveFunctionVisitor<Context> {
110
+ override visitID(id: InstructionId, state: Context): void {
111
+ state.visitId(id);
112
}
151
-}
152
-
153
-function visitValueBlock(context: Context, block: ReactiveValueBlock): void {
154
- for (const stmt of block.instructions) {
155
- switch (stmt.kind) {
156
- case "instruction": {
157
- visitValue(context, stmt.instruction.id, stmt.instruction.value);
158
- visitInstruction(context, stmt.instruction);
159
- break;
160
- }
161
- default: {
162
- invariant(false, "Unexpected terminal or scope in value block");
163
- }
164
- }
113
+ override visitPlace(id: InstructionId, place: Place, state: Context): void {
114
+ state.visitPlace(id, place);
115
}
166
- if (block.last !== null) {
167
- context.visitId(block.last.id);
168
- if (block.last.value.kind === "Identifier") {
169
- context.visitPlace(block.last.id, block.last.value);
170
- }
116
+ override visitLValue(
117
+ id: InstructionId,
118
+ lvalue: LValue,
119
+ state: Context
120
+ ): void {
121
+ state.visitPlace(id, lvalue.place);
122
}
172
-}
173
-
174
-function visitInstruction(
175
- context: Context,
176
- instruction: ReactiveInstruction
177
-): void {
178
- context.visitId(instruction.id);
179
- invariant(
180
- instruction.lvalue !== null,
181
- "Expected lvalues to not be null when assigning scopes. " +
182
- "Pruning lvalues too early can result in missing scope information."
183
- );
184
- context.visitPlace(instruction.id, instruction.lvalue.place);
185
-}
186
-
187
-function visitValue(
188
- context: Context,
189
- id: InstructionId,
190
- value: ReactiveValue
191
-): void {
192
- context.visitId(id);
193
- for (const operand of eachReactiveValueOperand(value)) {
194
- context.visitPlace(id, operand);
123
+ override visitBlock(block: ReactiveBlock, state: Context): void {
124
+ state.enter(() => {
125
+ this.traverseBlock(block, state);
126
+ });
127
}
128
}
129
compiler/forget/src/ReactiveScopes/PruneTemporaryLValues.ts
+25
-17
@@ -7,11 +7,13 @@
7
8
import {
9
Identifier,
10
+ InstructionId,
11
InstructionKind,
12
+ Place,
13
ReactiveFunction,
14
ReactiveInstruction,
15
} from "../HIR/HIR";
14
-import { visitFunction } from "./visitors";
16
+import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
17
18
/**
19
* Nulls out lvalues for temporary variables that are never accessed later. This only
@@ -19,23 +21,29 @@ import { visitFunction } from "./visitors";
21
*/
22
export function pruneTemporaryLValues(fn: ReactiveFunction): void {
23
const lvalues = new Map<Identifier, ReactiveInstruction>();
22
- visitFunction(fn, {
23
- visitInstruction: (instr) => {
24
- if (
25
- instr.lvalue !== null &&
26
- instr.lvalue.kind === InstructionKind.Const &&
27
- instr.lvalue.place.identifier.name === null
28
- ) {
29
- lvalues.set(instr.lvalue.place.identifier, instr);
30
- }
31
- },
32
- visitValue: (value) => {
33
- if (value.kind === "Identifier") {
34
- lvalues.delete(value.identifier);
35
- }
36
- },
37
- });
24
+ visitReactiveFunction(fn, new Visitor(), lvalues);
25
for (const [, instr] of lvalues) {
26
instr.lvalue = null;
27
}
28
}
29
+
30
+type LValues = Map<Identifier, ReactiveInstruction>;
31
+
32
+class Visitor extends ReactiveFunctionVisitor<LValues> {
33
+ override visitPlace(id: InstructionId, place: Place, state: LValues): void {
34
+ state.delete(place.identifier);
35
+ }
36
+ override visitInstruction(
37
+ instruction: ReactiveInstruction,
38
+ state: LValues
39
+ ): void {
40
+ this.traverseInstruction(instruction, state);
41
+ if (
42
+ instruction.lvalue !== null &&
43
+ instruction.lvalue.kind === InstructionKind.Const &&
44
+ instruction.lvalue.place.identifier.name === null
45
+ ) {
46
+ state.set(instruction.lvalue.place.identifier, instruction);
47
+ }
48
+ }
49
+}
compiler/forget/src/ReactiveScopes/PruneUnusedLabels.ts
+18
-58
@@ -7,77 +7,37 @@
7
8
import {
9
BlockId,
10
- ReactiveBlock,
10
ReactiveFunction,
11
ReactiveTerminal,
12
+ ReactiveTerminalStatement,
13
} from "../HIR/HIR";
14
-import { assertExhaustive } from "../Utils/utils";
14
+import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
15
16
/**
17
* Prunes terminal labels that are never explicitly jumped to.
18
*/
19
export function pruneUnusedLabels(fn: ReactiveFunction): void {
20
const labels: Labels = new Set();
21
- visitBlock(labels, fn.body);
21
+ visitReactiveFunction(fn, new Visitor(), labels);
22
}
23
24
type Labels = Set<BlockId>;
25
26
-function visitBlock(labels: Labels, block: ReactiveBlock): void {
27
- for (const item of block) {
28
- if (item.kind === "terminal") {
29
- // first visit the terminal's contents, which is the only place that can
30
- // reference the terminal's label
31
- visitTerminal(labels, item.terminal);
32
- // if the label wasn't referenced by a break/continue, we can prune it
33
- if (item.label !== null && !labels.has(item.label)) {
34
- item.label = null;
35
- }
36
- } else if (item.kind === "scope") {
37
- visitBlock(labels, item.instructions);
38
- }
39
- }
40
-}
41
-
42
-function visitTerminal(labels: Labels, terminal: ReactiveTerminal): void {
43
- switch (terminal.kind) {
44
- case "break":
45
- case "continue": {
46
- if (terminal.label !== null) {
47
- labels.add(terminal.label);
48
- }
49
- break;
50
- }
51
- case "for": {
52
- visitBlock(labels, terminal.loop);
53
- break;
54
- }
55
- case "if": {
56
- visitBlock(labels, terminal.consequent);
57
- if (terminal.alternate !== null) {
58
- visitBlock(labels, terminal.alternate);
59
- }
60
- break;
61
- }
62
- case "return":
63
- case "throw": {
64
- break;
65
- }
66
- case "switch": {
67
- for (const case_ of terminal.cases) {
68
- visitBlock(labels, case_.block!);
69
- }
70
- break;
71
- }
72
- case "while": {
73
- visitBlock(labels, terminal.loop);
74
- break;
75
- }
76
- default: {
77
- assertExhaustive(
78
- terminal,
79
- `Unexpected terminal kind '${(terminal as any).kind}'`
80
- );
26
+class Visitor extends ReactiveFunctionVisitor<Labels> {
27
+ override visitTerminal(
28
+ stmt: ReactiveTerminalStatement<ReactiveTerminal>,
29
+ state: Labels
30
+ ): void {
31
+ this.traverseTerminal(stmt, state);
32
+ const { terminal } = stmt;
33
+ if (
34
+ (terminal.kind === "break" || terminal.kind === "continue") &&
35
+ terminal.label !== null
36
+ ) {
37
+ state.add(terminal.label);
38
+ }
39
+ if (stmt.label !== null && !state.has(stmt.label)) {
40
+ stmt.label = null;
41
}
42
}
43
}
compiler/forget/src/ReactiveScopes/RenameVariables.ts
+30
-113
@@ -9,14 +9,18 @@ import invariant from "invariant";
9
import {
10
Identifier,
11
IdentifierId,
12
+ InstructionId,
13
+ Place,
14
ReactiveBlock,
15
ReactiveFunction,
14
- ReactiveTerminal,
15
- ReactiveValueBlock,
16
+ ReactiveInstruction,
17
+ ReactiveScopeBlock,
18
} from "../HIR/HIR";
17
-import { eachInstructionValueOperand } from "../HIR/visitors";
18
-import { assertExhaustive } from "../Utils/utils";
19
-import { eachReactiveValueOperand } from "./visitors";
19
+import {
20
+ eachReactiveValueOperand,
21
+ ReactiveFunctionVisitor,
22
+ visitReactiveFunction,
23
+} from "./visitors";
24
25
/**
26
* Ensures that each named variable in the given function has a unique name
@@ -35,122 +39,35 @@ export function renameVariables(fn: ReactiveFunction): void {
39
for (const param of fn.params) {
40
scopes.visit(param.identifier);
41
}
38
- visitBlock(scopes, fn.body);
42
+ visitReactiveFunction(fn, new Visitor(), scopes);
43
});
44
}
45
42
-function visitBlock(scopes: Scopes, block: ReactiveBlock): void {
43
- scopes.enter(() => visitBlockInner(scopes, block));
44
-}
45
-
46
-function visitBlockInner(scopes: Scopes, block: ReactiveBlock): void {
47
- for (const stmt of block) {
48
- switch (stmt.kind) {
49
- case "instruction": {
50
- for (const operand of eachReactiveValueOperand(
51
- stmt.instruction.value
52
- )) {
53
- scopes.visit(operand.identifier);
54
- }
55
- if (stmt.instruction.lvalue !== null) {
56
- scopes.visit(stmt.instruction.lvalue.place.identifier);
57
- }
58
- break;
59
- }
60
- case "scope": {
61
- // NOTE: we intentionally don't enter new block scope here,
62
- // since the outputs of the scope will be in the outer block
63
- visitBlockInner(scopes, stmt.instructions);
64
- break;
65
- }
66
- case "terminal": {
67
- visitTerminal(scopes, stmt.terminal);
68
- break;
69
- }
70
- }
46
+class Visitor extends ReactiveFunctionVisitor<Scopes> {
47
+ override visitPlace(id: InstructionId, place: Place, state: Scopes): void {
48
+ state.visit(place.identifier);
49
}
72
-}
73
-
74
-function visitValueBlock(scopes: Scopes, block: ReactiveValueBlock): void {
75
- for (const stmt of block.instructions) {
76
- invariant(
77
- stmt.kind === "instruction",
78
- "Value blocks may only contain instructions"
79
- );
80
- for (const operand of eachReactiveValueOperand(stmt.instruction.value)) {
81
- scopes.visit(operand.identifier);
82
- }
83
- if (stmt.instruction.lvalue !== null) {
84
- scopes.visit(stmt.instruction.lvalue.place.identifier);
85
- }
50
+ override visitBlock(block: ReactiveBlock, state: Scopes): void {
51
+ state.enter(() => {
52
+ this.traverseBlock(block, state);
53
+ });
54
}
87
- if (block.last !== null) {
88
- for (const operand of eachInstructionValueOperand(block.last.value)) {
89
- scopes.visit(operand.identifier);
55
+ override visitInstruction(
56
+ instruction: ReactiveInstruction,
57
+ state: Scopes
58
+ ): void {
59
+ for (const operand of eachReactiveValueOperand(instruction.value)) {
60
+ state.visit(operand.identifier);
61
}
91
- }
92
-}
93
-
94
-export function visitTerminal(
95
- scopes: Scopes,
96
- terminal: ReactiveTerminal
97
-): void {
98
- switch (terminal.kind) {
99
- case "return": {
100
- if (terminal.value !== null) {
101
- scopes.visit(terminal.value.identifier);
102
- }
103
- break;
104
- }
105
- case "throw": {
106
- scopes.visit(terminal.value.identifier);
107
- break;
108
- }
109
- case "break":
110
- case "continue": {
111
- break;
112
- }
113
- case "for": {
114
- scopes.enter(() => {
115
- visitValueBlock(scopes, terminal.init);
116
- visitValueBlock(scopes, terminal.test);
117
- visitValueBlock(scopes, terminal.update);
118
- visitBlock(scopes, terminal.loop);
119
- });
120
- break;
121
- }
122
- case "while": {
123
- visitValueBlock(scopes, terminal.test);
124
- visitBlock(scopes, terminal.loop);
125
- break;
126
- }
127
- case "if": {
128
- scopes.visit(terminal.test.identifier);
129
- visitBlock(scopes, terminal.consequent);
130
- if (terminal.alternate !== null) {
131
- visitBlock(scopes, terminal.alternate);
132
- }
133
- break;
134
- }
135
- case "switch": {
136
- scopes.visit(terminal.test.identifier);
137
- for (const case_ of terminal.cases) {
138
- if (case_.test !== null) {
139
- scopes.visit(case_.test.identifier);
140
- }
141
- if (case_.block !== undefined) {
142
- visitBlock(scopes, case_.block);
143
- }
144
- }
145
- break;
146
- }
147
- default: {
148
- assertExhaustive(
149
- terminal,
150
- `Unexpected terminal kind '${(terminal as any).kind}'`
151
- );
62
+ if (instruction.lvalue !== null) {
63
+ state.visit(instruction.lvalue.place.identifier);
64
}
65
}
66
+ override visitScope(scope: ReactiveScopeBlock, state: Scopes): void {
67
+ // Intentionally bypass visitBlock() since scopes do not introduce a new
68
+ // block scope
69
+ this.traverseBlock(scope.instructions, state);
70
+ }
71
}
72
73
class Scopes {