@samitouri / QOS-React / commits / 5f81b7bc30

Replace NodePath with SourceLocation

Removes all the `path: NodePath` values from various IR node types, replacing them with `loc: SourceLocation`. This type is an alias for babel's source location type plus a "generated" variant. The "OtherStatement" kind also used the `path` to print back the original AST (since we don't look into these); instead, we now capture the underlying `node` and emit that as-is during codegen. While I was doing this, i also fixed up the places where we had passed a null path; the vast majority have a clear place we can pull a location from. For example, `a ?? b` syntax creates some places/instructions for the `a != null`, but those can all point back to the `a` location.

Joseph Savona committed Nov 4, 2022 at 08:55 UTC 5f81b7bc303da1b674f476f3e5fea3dbc8be35b2
6 files changed +127 -76
compiler/forget/src/HIR/BuildHIR.ts
+74 -40
@@ -11,12 +11,14 @@ import { assertExhaustive } from "../Common/utils";
11 import { invariant } from "../CompilerError";
12 import {
13 Effect,
14 + GeneratedSource,
15 HIRFunction,
16 IfTerminal,
17 InstructionKind,
18 InstructionValue,
19 Place,
20 ReturnTerminal,
21 + SourceLocation,
22 Terminal,
23 ThrowTerminal,
24 } from "./HIR";
@@ -66,7 +68,7 @@ export function lower(
68 identifier,
69 memberPath: null,
70 effect: Effect.Unknown,
69 - path: null as any,
71 + loc: param.loc ?? GeneratedSource,
72 };
73 params.push(place);
74 });
@@ -81,10 +83,12 @@ export function lower(
83 }
84
85 return {
84 - path: func,
86 id,
87 params,
88 body: builder.build(),
89 + generator: func.node.generator === true,
90 + async: func.node.async === true,
91 + loc: func.node.loc ?? GeneratedSource,
92 };
93 }
94
@@ -574,13 +578,13 @@ function lowerStatement(
578 value = {
579 kind: "Primitive",
580 value: undefined,
577 - path: init as any, // TODO
581 + loc: id.loc,
582 };
583 }
584 builder.push({
585 lvalue: { place: id, kind },
586 value,
583 - path: stmt,
587 + loc: declaration.node.loc ?? GeneratedSource,
588 });
589 }
590 return;
@@ -596,7 +600,7 @@ function lowerStatement(
600 builder.push({
601 lvalue: null,
602 value,
599 - path: stmt,
603 + loc: stmt.node.loc ?? GeneratedSource,
604 });
605 return;
606 }
@@ -632,9 +636,13 @@ function lowerStatement(
636 case "TSTypeAliasDeclaration":
637 case "WithStatement": {
638 builder.push({
635 - path: stmtPath,
639 lvalue: null,
637 - value: { kind: "OtherStatement", path: stmtPath },
640 + loc: stmtPath.node.loc ?? GeneratedSource,
641 + value: {
642 + kind: "OtherStatement",
643 + loc: stmtPath.node.loc ?? GeneratedSource,
644 + node: stmtPath.node,
645 + },
646 });
647 return;
648 }
@@ -654,6 +662,7 @@ function lowerExpression(
662 exprPath: NodePath<t.Expression>
663 ): InstructionValue {
664 const exprNode = exprPath.node;
665 + const exprLoc = exprNode.loc ?? GeneratedSource;
666 switch (exprNode.type) {
667 case "Identifier": {
668 const expr = exprPath as NodePath<t.Identifier>;
@@ -663,7 +672,7 @@ function lowerExpression(
672 return {
673 kind: "Primitive",
674 value: null,
666 - path: exprPath,
675 + loc: exprLoc,
676 };
677 }
678 case "BooleanLiteral":
@@ -676,7 +685,7 @@ function lowerExpression(
685 return {
686 kind: "Primitive",
687 value,
679 - path: exprPath,
688 + loc: exprLoc,
689 };
690 }
691 case "ObjectExpression": {
@@ -701,7 +710,7 @@ function lowerExpression(
710 return {
711 kind: "ObjectExpression",
712 properties,
704 - path: exprPath,
713 + loc: exprLoc,
714 };
715 }
716 case "ArrayExpression": {
@@ -716,7 +725,7 @@ function lowerExpression(
725 return {
726 kind: "ArrayExpression",
727 elements,
719 - path: exprPath,
728 + loc: exprLoc,
729 };
730 }
731 case "NewExpression": {
@@ -739,7 +748,7 @@ function lowerExpression(
748 kind: "NewExpression",
749 callee,
750 args,
742 - path: exprPath,
751 + loc: exprLoc,
752 };
753 }
754 case "CallExpression": {
@@ -762,7 +771,7 @@ function lowerExpression(
771 kind: "CallExpression",
772 callee,
773 args,
765 - path: exprPath,
774 + loc: exprLoc,
775 };
776 }
777 case "BinaryExpression": {
@@ -780,7 +789,7 @@ function lowerExpression(
789 operator,
790 left,
791 right,
783 - path: exprPath,
792 + loc: exprLoc,
793 };
794 }
795 case "LogicalExpression": {
@@ -797,6 +806,7 @@ function lowerExpression(
806 return lowerConditional(
807 builder,
808 left,
809 + exprLoc,
810 () => left,
811 () => lowerExpression(builder, expr.get("right"))
812 );
@@ -806,6 +816,7 @@ function lowerExpression(
816 return lowerConditional(
817 builder,
818 left,
819 + exprLoc,
820 () => lowerExpression(builder, expr.get("right")),
821 () => left
822 );
@@ -821,15 +832,15 @@ function lowerExpression(
832 identifier: builder.makeTemporary(),
833 memberPath: null,
834 effect: Effect.Unknown,
824 - path: null as any,
835 + loc: left.loc,
836 };
837 builder.push({
838 value: {
839 kind: "Primitive",
840 value: null,
830 - path: null as any,
841 + loc: GeneratedSource,
842 },
832 - path: exprPath,
843 + loc: left.loc,
844 lvalue: { place: { ...nullPlace }, kind: InstructionKind.Const },
845 });
846
@@ -838,7 +849,7 @@ function lowerExpression(
849 identifier: builder.makeTemporary(),
850 memberPath: null,
851 effect: Effect.Unknown,
841 - path: null as any,
852 + loc: left.loc,
853 };
854 builder.push({
855 lvalue: {
@@ -850,13 +861,14 @@ function lowerExpression(
861 operator: "!=",
862 left,
863 right: nullPlace,
853 - path: null as any,
864 + loc: left.loc,
865 },
855 - path: null as any,
866 + loc: left.loc,
867 });
868 return lowerConditional(
869 builder,
870 condPlace,
871 + exprLoc,
872 () => left,
873 () => lowerExpression(builder, expr.get("right"))
874 );
@@ -877,8 +889,8 @@ function lowerExpression(
889 todoInvariant(operator === "=", "todo: support non-simple assignment");
890 builder.push({
891 lvalue: { place: left, kind: InstructionKind.Reassign },
880 - path: exprPath,
892 value: right,
893 + loc: exprLoc,
894 });
895 return left;
896 }
@@ -896,7 +908,7 @@ function lowerExpression(
908 identifier: object.identifier,
909 memberPath: [...(object.memberPath ?? []), property.node.name],
910 effect: Effect.Unknown,
899 - path: exprPath,
911 + loc: exprLoc,
912 };
913 return place;
914 }
@@ -933,10 +945,10 @@ function lowerExpression(
945 });
946 return {
947 kind: "JsxExpression",
936 - path: exprPath,
948 tag,
949 props,
950 children,
951 + loc: exprLoc,
952 };
953 }
954 default: {
@@ -952,6 +964,7 @@ function lowerExpression(
964 function lowerConditional(
965 builder: HIRBuilder,
966 test: Place,
967 + loc: SourceLocation,
968 consequent: () => InstructionValue,
969 alternate: () => InstructionValue
970 ): Place {
@@ -960,7 +973,7 @@ function lowerConditional(
973 identifier: builder.makeTemporary(),
974 memberPath: null,
975 effect: Effect.Read,
963 - path: null as any, // TODO
976 + loc,
977 };
978 // Block for code following the if
979 const continuationBlock = builder.reserve();
@@ -970,7 +983,7 @@ function lowerConditional(
983 builder.push({
984 value,
985 lvalue: { place: { ...place }, kind: InstructionKind.Const },
973 - path: value.path,
986 + loc: value.loc,
987 });
988 return {
989 kind: "goto",
@@ -983,7 +996,7 @@ function lowerConditional(
996 builder.push({
997 value,
998 lvalue: { place: { ...place }, kind: InstructionKind.Const },
986 - path: value.path,
999 + loc: value.loc,
1000 });
1001 return {
1002 kind: "goto",
@@ -1008,6 +1021,7 @@ function lowerJsxElementName(
1021 >
1022 ): Place {
1023 todoInvariant(exprPath.isJSXIdentifier(), "handle non-identifier tags");
1024 + const exprLoc = exprPath.node.loc ?? GeneratedSource;
1025 const tag: string = exprPath.node.name;
1026 if (tag.match(/^[A-Z]/)) {
1027 const binding =
@@ -1023,7 +1037,7 @@ function lowerJsxElementName(
1037 identifier: identifier,
1038 memberPath: null,
1039 effect: Effect.Unknown,
1026 - path: exprPath,
1040 + loc: exprLoc,
1041 };
1042 return place;
1043 } else {
@@ -1032,11 +1046,15 @@ function lowerJsxElementName(
1046 identifier: builder.makeTemporary(),
1047 memberPath: null,
1048 effect: Effect.Unknown,
1035 - path: exprPath,
1049 + loc: exprLoc,
1050 };
1051 builder.push({
1038 - value: { kind: "Primitive", value: tag, path: exprPath },
1039 - path: exprPath,
1052 + value: {
1053 + kind: "Primitive",
1054 + value: tag,
1055 + loc: exprLoc,
1056 + },
1057 + loc: exprLoc,
1058 lvalue: { place, kind: InstructionKind.Const },
1059 });
1060 return { ...place };
@@ -1053,6 +1071,8 @@ function lowerJsxElement(
1071 | t.JSXFragment
1072 >
1073 ): Place {
1074 + const exprNode = exprPath.node;
1075 + const exprLoc = exprNode.loc ?? GeneratedSource;
1076 if (exprPath.isJSXElement()) {
1077 return lowerExpressionToPlace(builder, exprPath);
1078 } else if (exprPath.isJSXExpressionContainer()) {
@@ -1065,25 +1085,37 @@ function lowerJsxElement(
1085 identifier: builder.makeTemporary(),
1086 memberPath: null,
1087 effect: Effect.Unknown,
1068 - path: exprPath,
1088 + loc: exprLoc,
1089 };
1090 builder.push({
1071 - value: { kind: "JSXText", value: exprPath.node.value, path: exprPath },
1072 - path: exprPath,
1091 + value: {
1092 + kind: "JSXText",
1093 + value: exprPath.node.value,
1094 + loc: exprLoc,
1095 + },
1096 + loc: exprLoc,
1097 lvalue: { place: { ...place }, kind: InstructionKind.Const },
1098 });
1099 return place;
1100 } else {
1101 + invariant(
1102 + t.isJSXFragment(exprNode) || t.isJSXSpreadChild(exprNode),
1103 + "Expected refinement to work"
1104 + );
1105 const place: Place = {
1106 kind: "Identifier",
1107 identifier: builder.makeTemporary(),
1108 memberPath: null,
1109 effect: Effect.Unknown,
1082 - path: exprPath,
1110 + loc: exprLoc,
1111 };
1112 builder.push({
1085 - value: { kind: "OtherStatement", path: exprPath },
1086 - path: exprPath,
1113 + value: {
1114 + kind: "OtherStatement",
1115 + node: exprNode,
1116 + loc: exprLoc,
1117 + },
1118 + loc: exprLoc,
1119 lvalue: { place: { ...place }, kind: InstructionKind.Const },
1120 });
1121 return place;
@@ -1098,16 +1130,17 @@ function lowerExpressionToPlace(
1130 if (instr.kind === "Identifier") {
1131 return instr;
1132 }
1133 + const exprLoc = exprPath.node.loc ?? GeneratedSource;
1134 const place: Place = {
1135 kind: "Identifier",
1136 identifier: builder.makeTemporary(),
1137 memberPath: null,
1138 effect: Effect.Unknown,
1106 - path: exprPath,
1139 + loc: exprLoc,
1140 };
1141 builder.push({
1142 value: instr,
1110 - path: exprPath,
1143 + loc: exprLoc,
1144 lvalue: { place: { ...place }, kind: InstructionKind.Const },
1145 });
1146 return place;
@@ -1115,6 +1148,7 @@ function lowerExpressionToPlace(
1148
1149 function lowerLVal(builder: HIRBuilder, exprPath: NodePath<t.LVal>): Place {
1150 const exprNode = exprPath.node;
1151 + const exprLoc = exprNode.loc ?? GeneratedSource;
1152 switch (exprNode.type) {
1153 case "Identifier": {
1154 // const expr = exprPath as NodePath<t.Identifier>;
@@ -1133,7 +1167,7 @@ function lowerLVal(builder: HIRBuilder, exprPath: NodePath<t.LVal>): Place {
1167 identifier: identifier,
1168 memberPath: null,
1169 effect: Effect.Unknown,
1136 - path: exprPath,
1170 + loc: exprLoc,
1171 };
1172 return place;
1173 }
@@ -1152,7 +1186,7 @@ function lowerLVal(builder: HIRBuilder, exprPath: NodePath<t.LVal>): Place {
1186 identifier: object.identifier,
1187 memberPath: [...(object.memberPath ?? []), propertyPath.node.name],
1188 effect: Effect.Unknown,
1155 - path: exprPath,
1189 + loc: exprLoc,
1190 };
1191 return place;
1192 }
compiler/forget/src/HIR/Codegen.ts
+8 -15
@@ -45,18 +45,13 @@ export default function codegen(fn: HIRFunction): t.Function {
45 const entry = fn.body.blocks.get(fn.body.entry)!;
46 const cx: Context = { ir: fn.body, temp: new Map() };
47 const body = codegenBlock(cx, entry);
48 - const node = fn.path.node;
49 - todoInvariant(
50 - t.isFunctionDeclaration(node),
51 - "todo: handle other than function declaration"
52 - );
48 const params = fn.params.map((param) => convertIdentifier(param.identifier));
49 return t.functionDeclaration(
50 fn.id !== null ? convertIdentifier(fn.id) : null,
51 params,
52 body,
58 - node.generator,
59 - node.async
53 + fn.generator,
54 + fn.async
55 );
56 }
57
@@ -158,7 +153,7 @@ function writeBlock(cx: Context, block: BasicBlock, body: Array<t.Statement>) {
153 }
154
155 function writeInstr(cx: Context, instr: Instruction, body: Array<t.Statement>) {
161 - let value;
156 + let value: t.Expression;
157 const instrValue = instr.value;
158 switch (instrValue.kind) {
159 case "ArrayExpression": {
@@ -253,15 +248,13 @@ function writeInstr(cx: Context, instr: Instruction, body: Array<t.Statement>) {
248 break;
249 }
250 case "OtherStatement": {
256 - const node = instrValue.path.node;
257 - if (node != null) {
258 - invariant(
259 - t.isStatement(node),
260 - "Expected node to be a statement if present"
261 - );
251 + const node = instrValue.node;
252 + if (t.isStatement(node)) {
253 body.push(node);
254 + return;
255 }
264 - return;
256 + value = node as any; // TODO(josephsavona) complete handling of JSX fragment/spreadchild elements
257 + break;
258 }
259 case "Identifier": {
260 value = codegenPlace(cx, instrValue);
compiler/forget/src/HIR/HIR.ts
+24 -8
@@ -5,7 +5,6 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import { NodePath } from "@babel/core";
8 import * as t from "@babel/types";
9 import { invariant } from "../CompilerError";
10
@@ -22,6 +21,16 @@ import { invariant } from "../CompilerError";
21
22 // AST -> (lowering) -> HIR -> (dep analysis) -> Reactive Scopes -> (scheduling?) -> HIR -> (codegen) -> AST
23
24 +/**
25 + * A location in a source file, intended to be used for providing diagnostic information and
26 + * transforming code while preserving source information (ie to emit source maps).
27 + *
28 + * `GeneratedSource` indicates that there is no single source location from which the code derives.
29 + *
30 + */
31 +export const GeneratedSource = Symbol();
32 +export type SourceLocation = t.SourceLocation | typeof GeneratedSource;
33 +
34 /**
35 * A React function defines a computation that takes some set of reactive
36 * inputs (eg props, hook arguments) and returns a result (JSX, hook return
@@ -37,7 +46,7 @@ import { invariant } from "../CompilerError";
46 * may depend upon (transitively).
47 */
48 export type ReactFunction = {
40 - path: NodePath<t.Function>;
49 + loc: SourceLocation;
50 id: Identifier | null;
51 params: Array<Place>;
52 returnScope: ScopeId;
@@ -59,10 +68,12 @@ export type ReactiveScope = {
68 * A function declaration including its path
69 */
70 export type HIRFunction = {
62 - path: NodePath<t.Function>;
71 + loc: SourceLocation;
72 id: Identifier | null;
73 params: Array<Place>;
74 body: HIR;
75 + generator: boolean;
76 + async: boolean;
77 };
78
79 /**
@@ -140,7 +151,7 @@ export type SwitchTerminal = {
151 export type Instruction = {
152 lvalue: LValue | null;
153 value: InstructionValue;
143 - path: NodePath;
154 + loc: SourceLocation;
155 };
156
157 export type LValue = {
@@ -162,7 +173,9 @@ export enum InstructionKind {
173 *
174 * Values are therefore only a Place or a primitive value.
175 */
165 -export type InstructionValue = (InstructionData & { path: NodePath }) | Place;
176 +export type InstructionValue =
177 + | (InstructionData & { loc: SourceLocation })
178 + | Place;
179
180 export type Phi = {
181 kind: "Phi";
@@ -199,7 +212,10 @@ export type InstructionData =
212 * which are not directly represented, but included for completeness and to allow
213 * passing through in codegen.
214 */
202 - | { kind: "OtherStatement" };
215 + | {
216 + kind: "OtherStatement";
217 + node: t.Statement | t.JSXSpreadChild | t.JSXFragment;
218 + };
219
220 /**
221 * A place where data may be read from / written to:
@@ -211,7 +227,7 @@ export type Place = {
227 identifier: Identifier;
228 memberPath: Array<string> | null;
229 effect: Effect;
214 - path: NodePath;
230 + loc: SourceLocation;
231 };
232
233 /**
@@ -220,7 +236,7 @@ export type Place = {
236 export type Primitive = {
237 kind: "Primitive";
238 value: number | boolean | string | null | undefined;
223 - path: NodePath<t.Node | null | undefined>;
239 + loc: SourceLocation;
240 };
241
242 /**
compiler/forget/src/HIR/InferReferenceEffects.ts
+8 -8
@@ -20,7 +20,7 @@ import {
20 ValueKind,
21 } from "./HIR";
22 import { mapTerminalSuccessors } from "./HIRBuilder";
23 -import { printMixedHIR, printPlace } from "./PrintHIR";
23 +import { printMixedHIR, printPlace, printSourceLocation } from "./PrintHIR";
24
25 /**
26 * For every usage of a value in the given function, infers the effect or action
@@ -72,12 +72,12 @@ export default function inferReferenceEffects(fn: HIRFunction) {
72 kind: "Identifier",
73 memberPath: null,
74 identifier: fn.id as any,
75 - path: null as any, // TODO
75 + loc: fn.loc,
76 effect: Effect.Freeze,
77 };
78 const value: InstructionValue = {
79 kind: "Primitive",
80 - path: null as any, // TODO
80 + loc: fn.loc,
81 value: undefined,
82 };
83 initialEnvironment.initialize(value, ValueKind.Frozen);
@@ -86,7 +86,7 @@ export default function inferReferenceEffects(fn: HIRFunction) {
86 for (const param of fn.params) {
87 const value: InstructionValue = {
88 kind: "Primitive",
89 - path: null as any, // TODO
89 + loc: param.loc,
90 value: undefined,
91 };
92 initialEnvironment.initialize(value, ValueKind.Frozen);
@@ -189,7 +189,9 @@ class Environment {
189 const values = this.#variables.get(place.identifier.id);
190 invariant(
191 values != null,
192 - `Expected value kind to be initialized at '${String(place.path)}'`
192 + `Expected value kind to be initialized at '${printSourceLocation(
193 + place.loc
194 + )}'`
195 );
196 let mergedKind: ValueKind | null = null;
197 for (const value of values) {
@@ -226,9 +228,7 @@ class Environment {
228 );
229 invariant(
230 this.#values.has(value),
229 - `Expected value to be initialized at '${String(value.path)}' in '${String(
230 - value.path?.parentPath
231 - )}'`
231 + `Expected value to be initialized at '${printSourceLocation(value.loc)}'`
232 );
233 this.#variables.set(place.identifier.id, new Set([value]));
234 }
compiler/forget/src/HIR/PrintHIR.ts
+11 -3
@@ -5,6 +5,7 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 +import generate from "@babel/generator";
9 import { assertExhaustive } from "../Common/utils";
10 import {
11 HIR,
@@ -15,6 +16,7 @@ import {
16 LValue,
17 Phi,
18 Place,
19 + SourceLocation,
20 Terminal,
21 } from "./HIR";
22
@@ -217,9 +219,7 @@ function printInstructionValue(instrValue: InstructionValue): string {
219 break;
220 }
221 case "OtherStatement": {
220 - value = `Other(${instrValue.path?.node?.type}): \`${String(
221 - instrValue.path
222 - )}\``;
222 + value = `OtherStatement(${generate(instrValue.node).code})`;
223 break;
224 }
225 case "Identifier": {
@@ -270,3 +270,11 @@ export function printPlace(place: Place): string {
270 export function printIdentifier(id: Identifier): string {
271 return `${id.name ?? ""}\$${id.id}`;
272 }
273 +
274 +export function printSourceLocation(loc: SourceLocation): string {
275 + if (typeof loc === "symbol") {
276 + return "generated";
277 + } else {
278 + return `${loc.start.line}:${loc.start.column}:${loc.end.line}:${loc.end.column}`;
279 + }
280 +}
compiler/forget/src/HIR/ScopeAnalysis.ts
+2 -2
@@ -65,7 +65,7 @@ export default function analyzeScopes(fn: HIRFunction): ReactFunction {
65 instructions: fn.body,
66 });
67 return {
68 - path: fn.path,
68 + loc: fn.loc,
69 id: fn.id,
70 params: fn.params,
71 returnScope: returnScopeId,
@@ -177,7 +177,7 @@ function analyze(fn: HIRFunction): ReactFunction {
177 const scopes: Map<ScopeId, ReactiveScope> = new Map();
178
179 return {
180 - path: fn.path,
180 + loc: fn.loc,
181 id: fn.id,
182 params: fn.params,
183 returnScope: returnScopeId,