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

DeclareLocal instruction

Adds a `DeclareLocal` instruction which represents declaring a named variable without initializing it. Currently declarations without an initializer (`let x`) are transformed into a declaration to undefined (`let x = undefined`) which changes the semantics due to hoisting and TDZ (temporary dead zone). The correct thing is to represent declaration without initialization.

Joe Savona committed Mar 15, 2023 at 13:12 UTC f54f653277ce73169e9221e744979191140257e9
15 files changed +130 -35
compiler/forget/src/HIR/BuildHIR.ts
+41 -19
@@ -589,32 +589,54 @@ function lowerStatement(
589 for (const declaration of stmt.get("declarations")) {
590 const id = declaration.get("id");
591 const init = declaration.get("init");
592 - let value: Place;
592 if (init.node != null) {
594 - value = lowerExpressionToTemporary(
593 + const value = lowerExpressionToTemporary(
594 builder,
595 init as NodePath<t.Expression>
596 );
597 + lowerAssignment(
598 + builder,
599 + stmt.node.loc ?? GeneratedSource,
600 + kind,
601 + id,
602 + value
603 + );
604 + } else if (id.isIdentifier()) {
605 + const loc = stmt.node.loc ?? GeneratedSource;
606 + const identifier = builder.resolveIdentifier(id);
607 + if (identifier == null) {
608 + builder.errors.push({
609 + reason: `(BuildHIR::lowerAssignment) Could not find binding for declaration.`,
610 + severity: ErrorSeverity.Invariant,
611 + nodePath: id,
612 + });
613 + } else {
614 + builder.push({
615 + id: makeInstructionId(0),
616 + lvalue: buildTemporaryPlace(builder, loc),
617 + value: {
618 + kind: "DeclareLocal",
619 + lvalue: {
620 + kind,
621 + place: {
622 + effect: Effect.Unknown,
623 + identifier,
624 + kind: "Identifier",
625 + loc: id.node.loc ?? GeneratedSource,
626 + },
627 + },
628 + loc: id.node.loc ?? GeneratedSource,
629 + },
630 + loc,
631 + });
632 + }
633 } else {
599 - value = buildTemporaryPlace(builder, id.node.loc ?? GeneratedSource);
600 - builder.push({
601 - id: makeInstructionId(0),
602 - lvalue: { ...value },
603 - value: {
604 - kind: "Primitive",
605 - value: undefined,
606 - loc: id.node.loc ?? GeneratedSource,
607 - },
608 - loc: value.loc,
634 + builder.errors.push({
635 + reason: `(BuildHIR::lowerStatement) Expected variable declaration to be an identifier if no initializer was provided.`,
636 + severity: ErrorSeverity.InvalidInput,
637 + nodePath: stmt,
638 });
639 }
611 - lowerAssignment(
612 - builder,
613 - stmt.node.loc ?? GeneratedSource,
614 - kind,
615 - id,
616 - value
617 - );
640 }
641 return;
642 }
compiler/forget/src/HIR/HIR.ts
+5
@@ -446,6 +446,11 @@ export type InstructionValue =
446 place: Place;
447 loc: SourceLocation;
448 }
449 + | {
450 + kind: "DeclareLocal";
451 + lvalue: LValue;
452 + loc: SourceLocation;
453 + }
454 | {
455 kind: "StoreLocal";
456 lvalue: LValue;
compiler/forget/src/HIR/PrintHIR.ts
+6
@@ -327,6 +327,12 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
327 value = `LoadLocal ${printPlace(instrValue.place)}`;
328 break;
329 }
330 + case "DeclareLocal": {
331 + value = `DeclareLocal ${instrValue.lvalue.kind} ${printPlace(
332 + instrValue.lvalue.place
333 + )}`;
334 + break;
335 + }
336 case "StoreLocal": {
337 value = `StoreLocal ${instrValue.lvalue.kind} ${printPlace(
338 instrValue.lvalue.place
compiler/forget/src/HIR/visitors.ts
+8
@@ -25,6 +25,7 @@ export function* eachInstructionLValue(
25 yield instr.lvalue;
26 }
27 switch (instr.value.kind) {
28 + case "DeclareLocal":
29 case "StoreLocal": {
30 yield instr.value.lvalue.place;
31 break;
@@ -65,6 +66,9 @@ export function* eachInstructionValueOperand(
66 yield* eachCallArgument(instrValue.args);
67 break;
68 }
69 + case "DeclareLocal": {
70 + break;
71 + }
72 case "LoadLocal": {
73 yield instrValue.place;
74 break;
@@ -274,6 +278,7 @@ export function mapInstructionLValues(
278 fn: (place: Place) => Place
279 ): void {
280 switch (instr.value.kind) {
281 + case "DeclareLocal":
282 case "StoreLocal": {
283 const lvalue = instr.value.lvalue;
284 lvalue.place = fn(lvalue.place);
@@ -329,6 +334,9 @@ export function mapInstructionOperands(
334 instrValue.value = fn(instrValue.value);
335 break;
336 }
337 + case "DeclareLocal": {
338 + break;
339 + }
340 case "LoadLocal": {
341 instrValue.place = fn(instrValue.place);
342 break;
compiler/forget/src/Inference/InferReferenceEffects.ts
+12
@@ -815,6 +815,18 @@ function inferBlock(
815 state.alias(lvalue, instrValue.place);
816 continue;
817 }
818 + case "DeclareLocal": {
819 + const value: InstructionValue = {
820 + kind: "Primitive",
821 + loc: instrValue.loc,
822 + value: undefined,
823 + };
824 + state.initialize(value, ValueKind.Immutable);
825 + state.define(instrValue.lvalue.place, value);
826 + state.alias(instr.lvalue, instrValue.lvalue.place);
827 + instr.lvalue.effect = Effect.Mutate;
828 + continue;
829 + }
830 case "StoreLocal": {
831 const effect =
832 state.isDefined(instrValue.lvalue.place) &&
compiler/forget/src/Optimization/DeadCodeElimination.ts
+3
@@ -160,6 +160,9 @@ function pruneableValue(
160 used: Set<Identifier>
161 ): boolean {
162 switch (value.kind) {
163 + case "DeclareLocal": {
164 + return !used.has(value.lvalue.place.identifier);
165 + }
166 case "StoreLocal": {
167 // Stores are pruneable only if the identifier being stored to is never read later
168 return !used.has(value.lvalue.place.identifier);
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+18 -3
@@ -360,14 +360,27 @@ function codegenInstructionNullable(
360 instr: ReactiveInstruction
361 ): t.Statement | null {
362 let statement;
363 - if (instr.value.kind === "StoreLocal" || instr.value.kind === "Destructure") {
363 + if (
364 + instr.value.kind === "StoreLocal" ||
365 + instr.value.kind === "Destructure" ||
366 + instr.value.kind === "DeclareLocal"
367 + ) {
368 let kind: InstructionKind = instr.value.lvalue.kind;
369 let lvalue;
370 + let value: t.Expression | null;
371 if (instr.value.kind === "StoreLocal") {
372 kind = cx.hasDeclared(instr.value.lvalue.place.identifier)
373 ? InstructionKind.Reassign
374 : kind;
375 lvalue = instr.value.lvalue.place;
376 + value = codegenPlace(cx, instr.value.value);
377 + } else if (instr.value.kind === "DeclareLocal") {
378 + if (cx.hasDeclared(instr.value.lvalue.place.identifier)) {
379 + return null;
380 + }
381 + kind = instr.value.lvalue.kind;
382 + lvalue = instr.value.lvalue.place;
383 + value = null;
384 } else {
385 lvalue = instr.value.lvalue.pattern;
386 for (const place of eachPatternOperand(lvalue)) {
@@ -376,8 +389,8 @@ function codegenInstructionNullable(
389 break;
390 }
391 }
392 + value = codegenPlace(cx, instr.value.value);
393 }
380 - const value = codegenPlace(cx, instr.value.value);
394 switch (kind) {
395 case InstructionKind.Const: {
396 return createVariableDeclaration(instr.loc, "const", [
@@ -390,6 +403,7 @@ function codegenInstructionNullable(
403 ]);
404 }
405 case InstructionKind.Reassign: {
406 + invariant(value !== null, "Expected a value for reassignment");
407 return createExpressionStatement(
408 instr.loc,
409 t.assignmentExpression("=", codegenLValue(lvalue), value)
@@ -844,10 +858,11 @@ function codegenInstructionValue(
858 value = t.identifier(instrValue.name);
859 break;
860 }
861 + case "DeclareLocal":
862 case "Destructure":
863 case "StoreLocal": {
864 CompilerError.invariant(
850 - `Unexpected StoreLocal in codegenInstructionValue`,
865 + `Unexpected ${instrValue.kind} in codegenInstructionValue`,
866 instrValue.loc
867 );
868 }
compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts
+1
@@ -205,6 +205,7 @@ function mayAllocate(value: InstructionValue): boolean {
205 case "Destructure": {
206 return doesPatternContainSpreadElement(value.lvalue.pattern);
207 }
208 + case "DeclareLocal":
209 case "StoreLocal":
210 case "LoadGlobal":
211 case "TypeCastExpression":
compiler/forget/src/ReactiveScopes/PruneNonEscapingScopes.ts
+12
@@ -444,6 +444,18 @@ function computeMemoizationInputs(
444 rvalues: [value.place],
445 };
446 }
447 + case "DeclareLocal": {
448 + const lvalues = [
449 + { place: value.lvalue.place, level: MemoizationLevel.Unmemoized },
450 + ];
451 + if (lvalue !== null) {
452 + lvalues.push({ place: lvalue, level: MemoizationLevel.Unmemoized });
453 + }
454 + return {
455 + lvalues,
456 + rvalues: [],
457 + };
458 + }
459 case "StoreLocal": {
460 const lvalues = [
461 { place: value.lvalue.place, level: MemoizationLevel.Conditional },
compiler/forget/src/SSA/LeaveSSA.ts
+15 -1
@@ -128,7 +128,21 @@ export function leaveSSA(fn: HIRFunction): void {
128 // Iterate the instructions and perform any rewrites as well as promoting SSA variables to
129 // `let` or `reassign` where possible.
130 const { lvalue, value } = instr;
131 - if (value.kind === "StoreLocal") {
131 + if (value.kind === "DeclareLocal") {
132 + const name = value.lvalue.place.identifier.name;
133 + if (name !== null) {
134 + if (declarations.has(name)) {
135 + CompilerError.invariant(
136 + `Unexpected duplicate declaration of '${name}'`,
137 + value.lvalue.place.loc
138 + );
139 + }
140 + declarations.set(name, {
141 + lvalue: value.lvalue,
142 + place: value.lvalue.place,
143 + });
144 + }
145 + } else if (value.kind === "StoreLocal") {
146 if (value.lvalue.place.identifier.name != null) {
147 const originalLVal = declarations.get(
148 value.lvalue.place.identifier.name
compiler/forget/src/__tests__/fixtures/compiler/assignment-in-nested-if.expect.md
+1 -1
@@ -21,7 +21,7 @@ function useBar(props) {
21 ```javascript
22 function useBar(props) {
23 const $ = React.unstable_useMemoCache(1);
24 - let z = undefined;
24 + let z;
25 if (props.a) {
26 if (props.b) {
27 let t0;
compiler/forget/src/__tests__/fixtures/compiler/destructuring-assignment.expect.md
+8 -8
@@ -29,16 +29,16 @@ function foo(a, b, c) {
29 function foo(a, b, c) {
30 const $ = React.unstable_useMemoCache(5);
31
32 - const [d, t54] = a;
32 + const [d, t46] = a;
33
34 - const [t56] = t54;
35 - const { e: t58 } = t56;
36 - const { f: g } = t58;
34 + const [t48] = t46;
35 + const { e: t50 } = t48;
36 + const { f: g } = t50;
37
38 - const { l: t63, o } = b;
39 - const { m: t66 } = t63;
40 - const [t68] = t66;
41 - const [n] = t68;
38 + const { l: t55, o } = b;
39 + const { m: t58 } = t55;
40 + const [t60] = t58;
41 + const [n] = t60;
42 const c_0 = $[0] !== d;
43 const c_1 = $[1] !== g;
44 const c_2 = $[2] !== n;
compiler/forget/src/__tests__/fixtures/compiler/ssa-leave-case.expect.md
-1
@@ -29,7 +29,6 @@ function Component(props) {
29 let y;
30 if (c_0) {
31 x = [];
32 - y = undefined;
32 if (props.p0) {
33 x.push(props.p1);
34 y = x;
compiler/forget/src/__tests__/fixtures/compiler/switch-non-final-default.expect.md
-1
@@ -38,7 +38,6 @@ function Component(props) {
38 let y;
39 if (c_0) {
40 x = [];
41 - y = undefined;
41 bb1: switch (props.p0) {
42 case 1: {
43 break bb1;
compiler/forget/src/__tests__/fixtures/compiler/switch.expect.md
-1
@@ -33,7 +33,6 @@ function Component(props) {
33 let y;
34 if (c_0) {
35 x = [];
36 - y = undefined;
36 switch (props.p0) {
37 case true: {
38 x.push(props.p2);