@samitouri / QOS-React-2 / commits / c2b2df0d17

Remove unused Place.memberPath

After the previous PR to change the scope dependency representation, `Place.memberPath` is now completely unused, this PR deletes that field and all references. Rejoice!

Joe Savona committed Jan 3, 2023 at 15:34 UTC c2b2df0d170ad27dc72c0d03c7567faf6b4f5fa6
13 files changed +30 -175
compiler/forget/src/HIR/BuildHIR.ts
+8 -41
@@ -79,7 +79,6 @@ export function lower(
79 const place: Place = {
80 kind: "Identifier",
81 identifier,
82 - memberPath: null,
82 effect: Effect.Unknown,
83 loc: param.loc ?? GeneratedSource,
84 };
@@ -624,6 +623,10 @@ function lowerStatement(
623 nodeKind === "let" ? InstructionKind.Let : InstructionKind.Const;
624 for (const declaration of stmt.get("declarations")) {
625 const id = declaration.get("id");
626 + invariant(
627 + id.isIdentifier(),
628 + "Support non-identifier variable declarations"
629 + );
630 const init = declaration.get("init");
631 let value: InstructionValue;
632 if (init.hasNode()) {
@@ -942,11 +945,12 @@ function lowerExpression(
945 const leftNode = left.node;
946 switch (leftNode.type) {
947 case "Identifier": {
948 + const lvalue = left as NodePath<t.Identifier>;
949 return lowerAssignment(
950 builder,
951 leftNode.loc ?? GeneratedSource,
952 InstructionKind.Reassign,
949 - left,
953 + lvalue,
954 lowerExpression(builder, expr.get("right"))
955 );
956 }
@@ -1247,7 +1251,6 @@ function lowerJsxElementName(
1251 const place: Place = {
1252 kind: "Identifier",
1253 identifier: identifier,
1250 - memberPath: null,
1254 effect: Effect.Unknown,
1255 loc: exprLoc,
1256 };
@@ -1373,47 +1376,12 @@ function lowerIdentifier(
1376 const place: Place = {
1377 kind: "Identifier",
1378 identifier: identifier,
1376 - memberPath: null,
1379 effect: Effect.Unknown,
1380 loc: exprLoc,
1381 };
1382 return place;
1383 }
1384
1383 -function lowerLVal(builder: HIRBuilder, exprPath: NodePath<t.LVal>): Place {
1384 - const exprNode = exprPath.node;
1385 - const exprLoc = exprNode.loc ?? GeneratedSource;
1386 - switch (exprNode.type) {
1387 - case "Identifier": {
1388 - const expr = exprPath as NodePath<t.Identifier>;
1389 - return lowerIdentifier(builder, expr);
1390 - }
1391 - case "MemberExpression": {
1392 - const expr = exprPath as NodePath<t.MemberExpression>;
1393 - const objectPath = expr.get("object");
1394 - todoInvariant(objectPath.isLVal(), "Support complex object assignment");
1395 - const object = lowerLVal(builder, objectPath);
1396 - const propertyPath = expr.get("property");
1397 - todoInvariant(
1398 - propertyPath.isIdentifier(),
1399 - "Support non-identifier properties"
1400 - );
1401 - const place: Place = {
1402 - kind: "Identifier",
1403 - identifier: object.identifier,
1404 - memberPath: [...(object.memberPath ?? []), propertyPath.node.name],
1405 - effect: Effect.Unknown,
1406 - loc: exprLoc,
1407 - };
1408 - return place;
1409 - }
1410 - default: {
1411 - todo(`lowerLVal(${exprNode.type})`);
1412 - // assertExhaustive(exprNode, "Unexpected lval kind");
1413 - }
1414 - }
1415 -}
1416 -
1385 /**
1386 * Creates a temporary Identifier and Place referencing that identifier.
1387 */
@@ -1421,7 +1389,6 @@ function buildTemporaryPlace(builder: HIRBuilder, loc: SourceLocation): Place {
1389 const place: Place = {
1390 kind: "Identifier",
1391 identifier: builder.makeTemporary(),
1424 - memberPath: null,
1392 effect: Effect.Unknown,
1393 loc,
1394 };
@@ -1432,10 +1399,10 @@ function lowerAssignment(
1399 builder: HIRBuilder,
1400 loc: SourceLocation,
1401 kind: InstructionKind,
1435 - lvalue: NodePath<t.LVal>,
1402 + lvalue: NodePath<t.Identifier>,
1403 value: InstructionValue
1404 ): InstructionValue {
1438 - const id = lowerLVal(builder, lvalue);
1405 + const id = lowerIdentifier(builder, lvalue);
1406 builder.push({
1407 id: makeInstructionId(0),
1408 lvalue: { place: id, kind },
compiler/forget/src/HIR/Codegen.ts
+6 -25
@@ -302,10 +302,7 @@ export function codegenInstruction(
302 if (instr.lvalue === null) {
303 return t.expressionStatement(value);
304 }
305 - if (
306 - instr.lvalue.place.memberPath === null &&
307 - instr.lvalue.place.identifier.name === null
308 - ) {
305 + if (instr.lvalue.place.identifier.name === null) {
306 // temporary
307 temp.set(instr.lvalue.place.identifier.id, value);
308 return t.emptyStatement();
@@ -511,15 +508,7 @@ function codegenJsxElement(
508 }
509
510 export function codegenLVal(lval: LValue): t.LVal {
514 - const expr = convertIdentifier(lval.place.identifier);
515 - const memberPath = lval.place.memberPath;
516 - return memberPath == null
517 - ? expr
518 - : memberPath.reduceRight(
519 - (path: t.Identifier | t.MemberExpression, member) =>
520 - t.memberExpression(path, t.identifier(member)),
521 - expr
522 - );
511 + return convertIdentifier(lval.place.identifier);
512 }
513
514 function codegenValue(
@@ -543,19 +532,11 @@ function codegenValue(
532
533 export function codegenPlace(temp: Temporaries, place: Place): t.Expression {
534 todoInvariant(place.kind === "Identifier", "support scope values");
546 - if (place.memberPath === null) {
547 - let tmp = temp.get(place.identifier.id);
548 - if (tmp != null) {
549 - return tmp;
550 - }
551 - return convertIdentifier(place.identifier);
552 - } else {
553 - let object: t.Expression = convertIdentifier(place.identifier);
554 - for (const path of place.memberPath) {
555 - object = t.memberExpression(object, t.identifier(path));
556 - }
557 - return object;
535 + let tmp = temp.get(place.identifier.id);
536 + if (tmp != null) {
537 + return tmp;
538 }
539 + return convertIdentifier(place.identifier);
540 }
541
542 export function convertIdentifier(identifier: Identifier): t.Identifier {
compiler/forget/src/HIR/HIR.ts
-1
@@ -316,7 +316,6 @@ export type InstructionData =
316 export type Place = {
317 kind: "Identifier";
318 identifier: Identifier;
319 - memberPath: Array<string> | null;
319 effect: Effect;
320 loc: SourceLocation;
321 };
compiler/forget/src/HIR/InferAlias.ts
-5
@@ -7,11 +7,6 @@ class AliasAnalyser {
7 aliases = new DisjointSet<Identifier>();
8
9 alias(lvalue: LValue, alias: Place) {
10 - // This is handled by InferAliasForStores.
11 - if (lvalue.place.memberPath !== null) {
12 - return;
13 - }
14 -
10 this.aliases.union([lvalue.place.identifier, alias.identifier]);
11 }
12 }
compiler/forget/src/HIR/InferMutableLifetimes.ts
+7 -11
@@ -122,19 +122,15 @@ export function inferMutableLifetimes(
122 }
123
124 if (instr.lvalue !== null) {
125 - if (instr.lvalue.place.memberPath === null) {
126 - const lvalueId = instr.lvalue.place.identifier;
125 + const lvalueId = instr.lvalue.place.identifier;
126
128 - // lvalue start being mutable when they're initially assigned a
129 - // value.
130 - lvalueId.mutableRange.start = instr.id;
127 + // lvalue start being mutable when they're initially assigned a
128 + // value.
129 + lvalueId.mutableRange.start = instr.id;
130
132 - // Let's be optimistic and assume this lvalue is not mutable by
133 - // default.
134 - lvalueId.mutableRange.end = makeInstructionId(instr.id + 1);
135 - } else {
136 - inferPlace(instr.lvalue.place, instr, inferMutableRangeForStores);
137 - }
131 + // Let's be optimistic and assume this lvalue is not mutable by
132 + // default.
133 + lvalueId.mutableRange.end = makeInstructionId(instr.id + 1);
134 }
135 }
136 }
compiler/forget/src/HIR/InferReferenceEffects.ts
+1 -30
@@ -74,7 +74,6 @@ export default function inferReferenceEffects(fn: HIRFunction) {
74 const initialEnvironment = Environment.empty();
75 const id: Place = {
76 kind: "Identifier",
77 - memberPath: null,
77 identifier: fn.id as any,
78 loc: fn.loc,
79 effect: Effect.Freeze,
@@ -172,7 +171,7 @@ class Environment {
171 */
172 initialize(value: InstructionValue, kind: ValueKind) {
173 invariant(
175 - value.kind !== "Identifier" || value.memberPath !== null,
174 + value.kind !== "Identifier",
175 "Expected all top-level identifiers to be defined as variables, not values"
176 );
177 this.#values.set(value, kind);
@@ -218,10 +217,6 @@ class Environment {
217 * Defines (initializing or updating) a variable with a specific kind of value.
218 */
219 define(place: Place, value: InstructionValue) {
221 - invariant(
222 - place.memberPath === null,
223 - "Expected a top-level identifier, not a member path"
224 - );
220 invariant(
221 this.#values.has(value),
222 `Expected value to be initialized at '${printSourceLocation(value.loc)}'`
@@ -587,10 +582,6 @@ function inferBlock(env: Environment, block: BasicBlock) {
582
583 const lvalue = instr.lvalue;
584 if (lvalue !== null) {
590 - invariant(
591 - lvalue.place.memberPath === null,
592 - "PropertyLoad must always be saved to a temporary"
593 - );
585 env.alias(lvalue.place, instrValue.value);
586 lvalue.place.effect = Effect.Store;
587 }
@@ -611,27 +602,15 @@ function inferBlock(env: Environment, block: BasicBlock) {
602 env.reference(instrValue.object, Effect.Read);
603 const lvalue = instr.lvalue;
604 if (lvalue !== null) {
614 - invariant(
615 - lvalue.place.memberPath === null,
616 - "PropertyLoad must always be saved to a temporary"
617 - );
605 env.initialize(instrValue, env.kind(instrValue.object));
606 env.define(lvalue.place, instrValue);
607 }
608 continue;
609 }
610 case "Identifier": {
624 - invariant(
625 - instrValue.memberPath === null,
626 - "Expected RHS memberPath to be lowered to PropertyLoad"
627 - );
611 env.reference(instrValue, Effect.Read);
612 const lvalue = instr.lvalue;
613 if (lvalue !== null) {
631 - invariant(
632 - lvalue.place.memberPath === null,
633 - "Expected lvalue member path to be null"
634 - );
614 lvalue.place.effect = Effect.Mutate;
615 // direct aliasing: `a = b`;
616 env.alias(lvalue.place, instrValue);
@@ -654,10 +633,6 @@ function inferBlock(env: Environment, block: BasicBlock) {
633
634 env.initialize(instrValue, valueKind);
635 if (instr.lvalue !== null) {
657 - invariant(
658 - instr.lvalue.place.memberPath === null,
659 - "Expected lvalue member path to be null"
660 - );
636 env.define(instr.lvalue.place, instrValue);
637 instr.lvalue.place.effect = lvalueEffect;
638 }
@@ -695,10 +670,6 @@ type HookKind = { kind: "State" } | { kind: "Ref" } | { kind: "Custom" };
670 type Hook = HookKind & { effectKind: Effect; valueKind: ValueKind };
671
672 function parseHookCall(place: Place): Hook | null {
698 - if (place.memberPath !== null) {
699 - // Hook calls must be statically resolved
700 - return null;
701 - }
673 const name = place.identifier.name;
674 if (name === null || !name.match(/^_?use/)) {
675 return null;
compiler/forget/src/HIR/InferTypes.ts
+1 -1
@@ -132,7 +132,7 @@ function generateTypeEquation(
132 function assignType(place: Place | undefined): Type | null {
133 // We type only top level identifiers. Typing objects is not very useful
134 // when we have to be so conservative.
135 - if (place?.memberPath !== null) {
135 + if (place === undefined) {
136 return null;
137 }
138
compiler/forget/src/HIR/PrintHIR.ts
+1 -8
@@ -321,14 +321,7 @@ export function printLValue(lval: LValue): string {
321
322 export function printPlace(place: Place): string {
323 const items = [place.effect, " ", printIdentifier(place.identifier)];
324 - if (place.memberPath !== null) {
325 - for (const path of place.memberPath) {
326 - items.push(".");
327 - items.push(path);
328 - }
329 - } else {
330 - items.push(printType(place.identifier.type));
331 - }
324 + items.push(printType(place.identifier.type));
325 return items.filter((x) => x != null).join("");
326 }
327
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+1 -5
@@ -305,11 +305,7 @@ export function codegenInstructionNullable(
305 value: t.Expression
306 ): t.Statement | null {
307 let statement;
308 - if (
309 - instr.lvalue !== null &&
310 - instr.lvalue.place.memberPath === null &&
311 - cx.declared(instr.lvalue.place.identifier)
312 - ) {
308 + if (instr.lvalue !== null && cx.declared(instr.lvalue.place.identifier)) {
309 statement = codegenInstruction(
310 cx.temp,
311 {
compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts
+1 -5
@@ -80,11 +80,7 @@ export function inferReactiveScopeVariables(fn: HIRFunction) {
80 const operands: Array<Identifier> = [];
81 if (instr.lvalue !== null) {
82 const range = instr.lvalue.place.identifier.mutableRange;
83 - if (
84 - instr.lvalue.place.memberPath !== null ||
85 - range.end > range.start + 1 ||
86 - mayAllocate(instr.value)
87 - ) {
83 + if (range.end > range.start + 1 || mayAllocate(instr.value)) {
84 operands.push(instr.lvalue!.place.identifier);
85 }
86 }
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+2 -25
@@ -21,7 +21,6 @@ import {
21 ReactiveValueBlock,
22 } from "../HIR/HIR";
23 import { eachInstructionValueOperand } from "../HIR/visitors";
24 -import { invariant } from "../Utils/CompilerError";
24 import { assertExhaustive } from "../Utils/utils";
25
26 /**
@@ -76,14 +75,6 @@ class Context {
75 }
76
77 declareProperty(lvalue: Place, object: Place, property: string): void {
79 - invariant(
80 - lvalue.memberPath === null,
81 - "Expected property loads to be stored to a temporary (no member path)"
82 - );
83 - invariant(
84 - object.memberPath === null,
85 - "Expected operands to have null memberPath"
86 - );
78 const objectDependency = this.#properties.get(object.identifier);
79 let nextDependency: ReactiveScopeDependency;
80 if (objectDependency === undefined) {
@@ -281,17 +272,7 @@ function visitInstructionValue(
272 ): void {
273 for (const operand of eachInstructionValueOperand(value)) {
274 // check for method invocation, we want to depend on the callee, not the method
284 - if (
285 - value.kind === "CallExpression" &&
286 - operand === value.callee &&
287 - operand.memberPath !== null
288 - ) {
289 - const callee = {
290 - ...operand,
291 - memberPath: operand.memberPath.slice(0, -1),
292 - };
293 - context.visitOperand(callee);
294 - } else if (value.kind === "PropertyLoad" && lvalue !== null) {
275 + if (value.kind === "PropertyLoad" && lvalue !== null) {
276 context.declareProperty(lvalue.place, value.object, value.property);
277 } else {
278 context.visitOperand(operand);
@@ -302,11 +283,7 @@ function visitInstructionValue(
283 function visitInstruction(context: Context, instr: Instruction): void {
284 const { lvalue } = instr;
285 visitInstructionValue(context, instr.value, lvalue);
305 - if (
306 - lvalue !== null &&
307 - lvalue.kind !== InstructionKind.Reassign &&
308 - lvalue.place.memberPath === null
309 - ) {
286 + if (lvalue !== null && lvalue.kind !== InstructionKind.Reassign) {
287 const range = lvalue.place.identifier.mutableRange;
288 // TODO: only assign Const if the value is never reassigned
289 const kind =
compiler/forget/src/SSA/EnterSSA.ts
+2 -6
@@ -199,12 +199,8 @@ export default function enterSSA(func: HIRFunction, env: Environment) {
199 if (instr.lvalue != null) {
200 const oldPlace = instr.lvalue.place;
201 let newPlace: Place;
202 - if (oldPlace.memberPath !== null) {
203 - newPlace = builder.getPlace(oldPlace);
204 - } else {
205 - newPlace = builder.definePlace(oldPlace);
206 - instr.lvalue.kind = InstructionKind.Const;
207 - }
202 + newPlace = builder.definePlace(oldPlace);
203 + instr.lvalue.kind = InstructionKind.Const;
204 instr.lvalue.place = newPlace;
205 }
206 }
compiler/forget/src/SSA/LeaveSSA.ts
-12
@@ -136,13 +136,6 @@ export function leaveSSA(fn: HIRFunction) {
136 const update = fn.body.blocks.get(terminal.update)!;
137 rewritePhis.push(...update.phis);
138 update.phis.clear();
139 -
140 - // find declarations in the for init
141 - for (const instr of init.instructions) {
142 - if (instr.lvalue !== null && instr.lvalue.place.memberPath === null) {
143 - // hasDeclaration.add(instr.lvalue.place.identifier);
144 - }
145 - }
139 }
140
141 for (const phi of reassignmentPhis) {
@@ -197,7 +190,6 @@ export function leaveSSA(fn: HIRFunction) {
190 lvalue: {
191 place: {
192 kind: "Identifier",
200 - memberPath: null,
193 identifier: canonicalId,
194 effect: Effect.Mutate,
195 loc: GeneratedSource,
@@ -208,7 +200,6 @@ export function leaveSSA(fn: HIRFunction) {
200 initOperand !== null
201 ? {
202 kind: "Identifier",
211 - memberPath: null,
203 identifier: initOperand,
204 effect: Effect.Read,
205 loc: GeneratedSource,
@@ -234,7 +225,6 @@ export function leaveSSA(fn: HIRFunction) {
225 lvalue: {
226 place: {
227 kind: "Identifier",
237 - memberPath: null,
228 identifier: canonicalId,
229 effect: Effect.Mutate,
230 loc: GeneratedSource,
@@ -243,7 +233,6 @@ export function leaveSSA(fn: HIRFunction) {
233 },
234 value: {
235 kind: "Identifier",
246 - memberPath: null,
236 identifier: operand,
237 effect: Effect.Read,
238 loc: GeneratedSource,
@@ -295,7 +284,6 @@ export function leaveSSA(fn: HIRFunction) {
284 if (lvalue !== null) {
285 if (
286 lvalue.kind === InstructionKind.Const &&
298 - lvalue.place.memberPath === null &&
287 rewrites.has(lvalue.place.identifier)
288 ) {
289 // For rewrites, the declaration of the canonical identifier has to be `let`,