@samitouri / QOS-React / commits / fe95b5df90

[hir] add DeclareContext (2/n)

--- In `lower`, we now ensure that all context variables are declared by a `DeclareContext` instruction. `DeclareContext` always produces a `let` declaration, and `StoreContext` is always a reassign. There are a few reasons we need `DeclareContext`: - DeclareLocal assumes it is storing to a SSA-fied identifier (which always stores an immutable primitive). This does not fit context variables. - Without DeclareContext, we need custom logic in some passes to initialize identifier / context state (e.g. `MutableRange`, ValueKind, etc) for the `StoreContext` that declares the context. This PR stack models context variables as concrete identifiers (with references to context variables modeled by `Place` referencing the context variable identifier). @josephsavona pointed out that this is abusing the notion of Identifier/Place, as context variables are essentially interior properties of a ContextEnvironment. Since we are not modeling `ContextEnvironment` implicitly or explicitly, all inference for context variables is essentially pointer analysis.

Mofei Zhang committed May 12, 2023 at 12:48 UTC fe95b5df903903da6b372dcc2fd43f842ab3226d
18 files changed +245 -62
compiler/forget/src/HIR/BuildHIR.ts
+66 -18
@@ -647,19 +647,38 @@ function lowerStatement(
647 nodePath: id,
648 });
649 } else {
650 - lowerValueToTemporary(builder, {
651 - kind: "DeclareLocal",
652 - lvalue: {
653 - kind,
654 - place: {
655 - effect: Effect.Unknown,
656 - identifier,
657 - kind: "Identifier",
658 - loc: id.node.loc ?? GeneratedSource,
659 - },
660 - },
650 + const place: Place = {
651 + effect: Effect.Unknown,
652 + identifier,
653 + kind: "Identifier",
654 loc: id.node.loc ?? GeneratedSource,
662 - });
655 + };
656 + if (builder.isContextIdentifier(id)) {
657 + if (kind === InstructionKind.Const) {
658 + builder.errors.push({
659 + reason: `(BuildHIR::lowerAssignment) Invalid declaration kind (const) for variable later reassigned.`,
660 + severity: ErrorSeverity.InvalidInput,
661 + nodePath: id,
662 + });
663 + }
664 + lowerValueToTemporary(builder, {
665 + kind: "DeclareContext",
666 + lvalue: {
667 + kind: InstructionKind.Let,
668 + place,
669 + },
670 + loc: id.node.loc ?? GeneratedSource,
671 + });
672 + } else {
673 + lowerValueToTemporary(builder, {
674 + kind: "DeclareLocal",
675 + lvalue: {
676 + kind,
677 + place,
678 + },
679 + loc: id.node.loc ?? GeneratedSource,
680 + });
681 + }
682 }
683 } else {
684 builder.errors.push({
@@ -2461,12 +2480,41 @@ function lowerAssignment(
2480 effect: Effect.Unknown,
2481 loc: lvalue.node.loc ?? GeneratedSource,
2482 };
2464 - const temporary = lowerValueToTemporary(builder, {
2465 - kind: getStoreKind(builder, lvalue),
2466 - lvalue: { place: { ...place }, kind },
2467 - value,
2468 - loc,
2469 - });
2483 +
2484 + let temporary;
2485 + if (builder.isContextIdentifier(lvalue)) {
2486 + if (kind !== InstructionKind.Reassign) {
2487 + if (kind === InstructionKind.Const) {
2488 + builder.errors.push({
2489 + reason: `(BuildHIR::lowerAssignment) Invalid declaration kind (const) for variable later reassigned.`,
2490 + severity: ErrorSeverity.InvalidInput,
2491 + nodePath: lvalue,
2492 + });
2493 + }
2494 + lowerValueToTemporary(builder, {
2495 + kind: "DeclareContext",
2496 + lvalue: {
2497 + kind: InstructionKind.Let,
2498 + place: { ...place },
2499 + },
2500 + loc: place.loc,
2501 + });
2502 + }
2503 +
2504 + temporary = lowerValueToTemporary(builder, {
2505 + kind: "StoreContext",
2506 + lvalue: { place: { ...place }, kind: InstructionKind.Reassign },
2507 + value,
2508 + loc,
2509 + });
2510 + } else {
2511 + temporary = lowerValueToTemporary(builder, {
2512 + kind: "StoreLocal",
2513 + lvalue: { place: { ...place }, kind },
2514 + value,
2515 + loc,
2516 + });
2517 + }
2518 return { kind: "LoadLocal", place: temporary, loc: temporary.loc };
2519 }
2520 case "MemberExpression": {
compiler/forget/src/HIR/HIR.ts
+12 -1
@@ -562,6 +562,14 @@ export type InstructionValue =
562 lvalue: LValue;
563 loc: SourceLocation;
564 }
565 + | {
566 + kind: "DeclareContext";
567 + lvalue: {
568 + kind: InstructionKind.Let;
569 + place: Place;
570 + };
571 + loc: SourceLocation;
572 + }
573 | {
574 kind: "StoreLocal";
575 lvalue: LValue;
@@ -570,7 +578,10 @@ export type InstructionValue =
578 }
579 | {
580 kind: "StoreContext";
573 - lvalue: LValue;
581 + lvalue: {
582 + kind: InstructionKind.Reassign;
583 + place: Place;
584 + };
585 value: Place;
586 loc: SourceLocation;
587 }
compiler/forget/src/HIR/PrintHIR.ts
+6
@@ -354,6 +354,12 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
354 )}`;
355 break;
356 }
357 + case "DeclareContext": {
358 + value = `DeclareContext ${instrValue.lvalue.kind} ${printPlace(
359 + instrValue.lvalue.place
360 + )}`;
361 + break;
362 + }
363 case "StoreLocal": {
364 value = `StoreLocal ${instrValue.lvalue.kind} ${printPlace(
365 instrValue.lvalue.place
compiler/forget/src/HIR/visitors.ts
+3
@@ -26,6 +26,7 @@ export function* eachInstructionLValue(
26 }
27 switch (instr.value.kind) {
28 case "DeclareLocal":
29 + case "DeclareContext":
30 case "StoreLocal": {
31 yield instr.value.lvalue.place;
32 break;
@@ -61,6 +62,7 @@ export function* eachInstructionValueOperand(
62 yield* eachCallArgument(instrValue.args);
63 break;
64 }
65 + case "DeclareContext":
66 case "DeclareLocal": {
67 break;
68 }
@@ -351,6 +353,7 @@ export function mapInstructionOperands(
353 instrValue.value = fn(instrValue.value);
354 break;
355 }
356 + case "DeclareContext":
357 case "DeclareLocal": {
358 break;
359 }
compiler/forget/src/Inference/InferMutableLifetimes.ts
-10
@@ -118,16 +118,6 @@ export function inferMutableLifetimes(
118 }
119
120 for (const instr of block.instructions) {
121 - if (instr.value.kind === "StoreContext") {
122 - const id = instr.value.lvalue.place.identifier;
123 - // Context variables do not participate in SSA and are not generally considered
124 - // lvalues (). This hack tries to initialize a mutable range the first time we
125 - // visit an context variable assignment.
126 - if (id.mutableRange.start === 0 && id.mutableRange.end === 0) {
127 - id.mutableRange.start = instr.id;
128 - id.mutableRange.end = makeInstructionId(instr.id + 1);
129 - }
130 - }
121 for (const operand of eachInstructionLValue(instr)) {
122 const lvalueId = operand.identifier;
123
compiler/forget/src/Inference/InferReferenceEffects.ts
+5 -17
@@ -876,8 +876,11 @@ function inferBlock(
876 };
877 state.initialize(value, ValueKind.Immutable);
878 state.define(instrValue.lvalue.place, value);
879 - state.alias(instr.lvalue, instrValue.lvalue.place);
880 - instr.lvalue.effect = Effect.Mutate;
879 + continue;
880 + }
881 + case "DeclareContext": {
882 + state.initialize(instrValue, ValueKind.Mutable);
883 + state.define(instrValue.lvalue.place, instrValue);
884 continue;
885 }
886 case "StoreLocal": {
@@ -901,21 +904,6 @@ function inferBlock(
904
905 const lvalue = instr.lvalue;
906 state.alias(lvalue, instrValue.value);
904 - // this logic is really awkward
905 - // Essentially, we want to say that
906 - // 1. instr.lvalue (the value produced by the instruction itself) has a
907 - // ValueKind of the rhs.
908 - // - this is for chained assignment
909 - // 2. instr.value.lvalue (the store location) has a ValueKind of Mutable
910 -
911 - // As an alternative, we could insert a CreateContextVariable instruction
912 - // before the initial StoreContext
913 - const storeLValue = instrValue.lvalue.place;
914 - if (!state.isDefined(storeLValue)) {
915 - const instrCopy = { ...instrValue };
916 - state.initialize(instrCopy, ValueKind.Mutable);
917 - state.define(storeLValue, instrCopy);
918 - }
907 lvalue.effect = Effect.Store;
908 continue;
909 }
compiler/forget/src/Optimization/DeadCodeElimination.ts
+1
@@ -223,6 +223,7 @@ function pruneableValue(value: InstructionValue, state: State): boolean {
223 return false;
224 }
225 case "LoadContext":
226 + case "DeclareContext":
227 case "StoreContext": {
228 return false;
229 }
compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts
+22 -8
@@ -443,21 +443,25 @@ function codegenInstructionNullable(
443 instr.value.kind === "StoreLocal" ||
444 instr.value.kind === "StoreContext" ||
445 instr.value.kind === "Destructure" ||
446 - instr.value.kind === "DeclareLocal"
446 + instr.value.kind === "DeclareLocal" ||
447 + instr.value.kind === "DeclareContext"
448 ) {
449 let kind: InstructionKind = instr.value.lvalue.kind;
450 let lvalue;
451 let value: t.Expression | null;
451 - if (
452 - instr.value.kind === "StoreLocal" ||
453 - instr.value.kind === "StoreContext"
454 - ) {
452 + if (instr.value.kind === "StoreLocal") {
453 kind = cx.hasDeclared(instr.value.lvalue.place.identifier)
454 ? InstructionKind.Reassign
455 : kind;
456 lvalue = instr.value.lvalue.place;
457 value = codegenPlace(cx, instr.value.value);
460 - } else if (instr.value.kind === "DeclareLocal") {
458 + } else if (instr.value.kind === "StoreContext") {
459 + lvalue = instr.value.lvalue.place;
460 + value = codegenPlace(cx, instr.value.value);
461 + } else if (
462 + instr.value.kind === "DeclareLocal" ||
463 + instr.value.kind === "DeclareContext"
464 + ) {
465 if (cx.hasDeclared(instr.value.lvalue.place.identifier)) {
466 return null;
467 }
@@ -507,8 +511,17 @@ function codegenInstructionNullable(
511 invariant(value !== null, "Expected a value for reassignment");
512 const expr = t.assignmentExpression("=", codegenLValue(lvalue), value);
513 if (instr.lvalue !== null) {
510 - cx.temp.set(instr.lvalue.identifier.id, expr);
511 - return null;
514 + if (instr.value.kind !== "StoreContext") {
515 + cx.temp.set(instr.lvalue.identifier.id, expr);
516 + return null;
517 + } else {
518 + // Handle chained reassignments for context variables
519 + const statement = codegenInstruction(cx, instr, expr);
520 + if (statement.type === "EmptyStatement") {
521 + return null;
522 + }
523 + return statement;
524 + }
525 } else {
526 return createExpressionStatement(instr.loc, expr);
527 }
@@ -1017,6 +1030,7 @@ function codegenInstructionValue(
1030 }
1031 case "Debugger":
1032 case "DeclareLocal":
1033 + case "DeclareContext":
1034 case "Destructure":
1035 case "StoreLocal":
1036 case "StoreContext": {
compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts
+1
@@ -224,6 +224,7 @@ function mayAllocate(value: InstructionValue): boolean {
224 }
225 case "Await":
226 case "DeclareLocal":
227 + case "DeclareContext":
228 case "StoreLocal":
229 case "LoadGlobal":
230 case "TypeCastExpression":
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+7 -6
@@ -513,10 +513,7 @@ class PropagationVisitor extends ReactiveFunctionVisitor<Context> {
513 value: ReactiveValue,
514 lvalue: Place | null
515 ): void {
516 - if (
517 - (value.kind === "LoadLocal" || value.kind === "LoadContext") &&
518 - lvalue !== null
519 - ) {
516 + if (value.kind === "LoadLocal" && lvalue !== null) {
517 if (
518 value.place.identifier.name !== null &&
519 lvalue.identifier.name === null &&
@@ -532,7 +529,7 @@ class PropagationVisitor extends ReactiveFunctionVisitor<Context> {
529 } else {
530 context.visitProperty(value.object, value.property);
531 }
535 - } else if (value.kind === "StoreLocal" || value.kind === "StoreContext") {
532 + } else if (value.kind === "StoreLocal") {
533 context.visitOperand(value.value);
534 if (value.lvalue.kind === InstructionKind.Reassign) {
535 context.visitReassignment(value.lvalue.place);
@@ -541,11 +538,15 @@ class PropagationVisitor extends ReactiveFunctionVisitor<Context> {
538 id,
539 scope: context.currentScope,
540 });
544 - } else if (value.kind === "DeclareLocal") {
541 + } else if (value.kind === "DeclareLocal" || value.kind === "DeclareContext") {
542 // Some variables may be declared and never initialized. We need
543 // to retain (and hoist) these declarations if they are included
544 // in a reactive scope. One approach is to simply add all `DeclareLocal`s
545 // as scope declarations.
546 +
547 + // We add context variable declarations here, not at `StoreContext`, since
548 + // context Store / Loads are modeled as reads and mutates to the underlying
549 + // variable reference (instead of through intermediate / inlined temporaries)
550 context.declare(value.lvalue.place.identifier, {
551 id,
552 scope: context.currentScope,
compiler/forget/src/ReactiveScopes/PruneNonEscapingScopes.ts
+13
@@ -471,6 +471,19 @@ function computeMemoizationInputs(
471 rvalues: [value.place],
472 };
473 }
474 + case "DeclareContext": {
475 + const lvalues = [
476 + { place: value.lvalue.place, level: MemoizationLevel.Memoized },
477 + ];
478 + if (lvalue !== null) {
479 + lvalues.push({ place: lvalue, level: MemoizationLevel.Unmemoized });
480 + }
481 + return {
482 + lvalues,
483 + rvalues: [],
484 + };
485 + }
486 +
487 case "DeclareLocal": {
488 const lvalues = [
489 { place: value.lvalue.place, level: MemoizationLevel.Unmemoized },
compiler/forget/src/TypeInference/InferTypes.ts
+1
@@ -202,6 +202,7 @@ function* generateInstructionTypes(
202 }
203
204 case "DeclareLocal":
205 + case "DeclareContext":
206 case "Destructure":
207 case "NewExpression":
208 case "TypeCastExpression":
compiler/forget/src/__tests__/fixtures/compiler/_bug.lambda-reassign-shadowed-primitive.expect.md
+2 -1
@@ -31,7 +31,8 @@ function Component() {
31 }
32 const x = t0;
33
34 - let x_0 = 56;
34 + let x_0;
35 + x_0 = 56;
36 const fn = function () {
37 x_0 = 42;
38 };
compiler/forget/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-3.expect.md
+2 -1
@@ -28,7 +28,8 @@ function bar(a, b) {
28 if (c_0 || c_1) {
29 const x = [a, b];
30 y = {};
31 - let t = {};
31 + let t;
32 + t = {};
33 (function () {
34 y = x[0][1];
35 t = x[1][0];
compiler/forget/src/__tests__/fixtures/compiler/chained-assignment-context-variable.expect.md new
+49
@@ -0,0 +1,49 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component() {
6 + let x,
7 + y = (x = {});
8 + const foo = () => {
9 + x = getObject();
10 + };
11 + foo();
12 + return [y, x];
13 +}
14 +
15 +```
16 +
17 +## Code
18 +
19 +```javascript
20 +import { unstable_useMemoCache as useMemoCache } from "react";
21 +function Component() {
22 + const $ = useMemoCache(3);
23 + let x;
24 + let y;
25 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
26 + y = x = {};
27 +
28 + const foo = () => {
29 + x = getObject();
30 + };
31 + foo();
32 + $[0] = x;
33 + $[1] = y;
34 + } else {
35 + x = $[0];
36 + y = $[1];
37 + }
38 + let t0;
39 + if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
40 + t0 = [y, x];
41 + $[2] = t0;
42 + } else {
43 + t0 = $[2];
44 + }
45 + return t0;
46 +}
47 +
48 +```
49 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/chained-assignment-context-variable.js new
+9
@@ -0,0 +1,9 @@
1 +function Component() {
2 + let x,
3 + y = (x = {});
4 + const foo = () => {
5 + x = getObject();
6 + };
7 + foo();
8 + return [y, x];
9 +}
compiler/forget/src/__tests__/fixtures/compiler/declare-reassign-variable-in-closure.expect.md new
+37
@@ -0,0 +1,37 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(p) {
6 + let x;
7 + const foo = () => {
8 + x = {};
9 + };
10 + foo();
11 +
12 + return x;
13 +}
14 +
15 +```
16 +
17 +## Code
18 +
19 +```javascript
20 +import { unstable_useMemoCache as useMemoCache } from "react";
21 +function Component(p) {
22 + const $ = useMemoCache(1);
23 + let x;
24 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
25 + const foo = () => {
26 + x = {};
27 + };
28 + foo();
29 + $[0] = x;
30 + } else {
31 + x = $[0];
32 + }
33 + return x;
34 +}
35 +
36 +```
37 +
\ No newline at end of file
compiler/forget/src/__tests__/fixtures/compiler/declare-reassign-variable-in-closure.js new
+9
@@ -0,0 +1,9 @@
1 +function Component(p) {
2 + let x;
3 + const foo = () => {
4 + x = {};
5 + };
6 + foo();
7 +
8 + return x;
9 +}