@samitouri / QOS-React / commits / 86ffcde8ed

[valueblocks] Handle compound RHS for logicals

The previous PR handled the case where the LHS of a logical was compound, but didn't handle compound RHS values. This is fixed now.

Joe Savona committed Jan 31, 2023 at 13:39 UTC 86ffcde8edad93ce311dff9d0d13789daf3eae9e
3 files changed +63 -39
compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts
+31 -29
@@ -31,11 +31,6 @@ import {
31 ReactiveValue,
32 Terminal,
33 } from "../HIR/HIR";
34 -import {
35 - printInstructionValue,
36 - printPlace,
37 - printTerminal,
38 -} from "../HIR/PrintHIR";
34 import { mapInstructionOperands } from "../HIR/visitors";
35 import { assertExhaustive } from "../Utils/utils";
36
@@ -477,16 +472,16 @@ class Driver {
472 switch (terminal.kind) {
473 case "logical": {
474 let testBlock: BasicBlock;
480 - let leftValue: ReactiveValue | null = null;
475 let leftPlace: Place | null = null;
476 + let leftValue: ReactiveValue | null = null;
477 const defaultTestBlock = this.cx.ir.blocks.get(terminal.test)!;
478 if (defaultTestBlock.terminal.kind === "branch") {
479 testBlock = defaultTestBlock;
480 } else {
481 const leftResult = this.visitValueTerminal(defaultTestBlock.terminal);
482 testBlock = this.cx.ir.blocks.get(leftResult.fallthrough)!;
488 - leftValue = leftResult.value;
483 leftPlace = leftResult.place;
484 + leftValue = leftResult.value;
485 }
486
487 invariant(
@@ -498,9 +493,32 @@ class Driver {
493 testBlock.instructions;
494 const leftBlock = this.cx.ir.blocks.get(testBlock.terminal.consequent)!;
495 leftInstructions.push(...leftBlock.instructions);
501 - // TODO: If right block ends in a value terminal, recursively process with visitValueTerminal
502 - // similar to handling for the compound lhs case.
503 - const rightBlock = this.cx.ir.blocks.get(testBlock.terminal.alternate)!;
496 + if (leftPlace !== null && leftValue !== null) {
497 + leftInstructions.forEach((instr) =>
498 + mapInstructionOperands(instr as Instruction, (place) => {
499 + return place.identifier === leftPlace!.identifier
500 + ? (leftValue as Place)
501 + : place;
502 + })
503 + );
504 + }
505 +
506 + let rightBlock: BasicBlock;
507 + let rightValue: ReactiveValue | null = null;
508 + let rightPlace: Place | null = null;
509 + const defaultRightBlock = this.cx.ir.blocks.get(
510 + testBlock.terminal.alternate
511 + )!;
512 + if (defaultRightBlock.terminal.kind === "goto") {
513 + rightBlock = defaultRightBlock;
514 + } else {
515 + const rightResult = this.visitValueTerminal(
516 + defaultRightBlock.terminal
517 + );
518 + rightBlock = this.cx.ir.blocks.get(rightResult.fallthrough)!;
519 + rightPlace = rightResult.place;
520 + rightValue = rightResult.value;
521 + }
522 const rightInstructions: Array<ReactiveInstruction> =
523 rightBlock.instructions;
524 const place = leftInstructions.at(-1)!.lvalue!.place;
@@ -509,18 +527,11 @@ class Driver {
527 rightInstructions.at(-1)!.lvalue!.place.identifier,
528 "Expected both branches of a logical expression to store to the same temporary"
529 );
512 - if (leftPlace !== null) {
513 - leftInstructions.forEach((instr) =>
514 - mapInstructionOperands(instr as Instruction, (place) => {
515 - return place.identifier === leftPlace!.identifier
516 - ? (leftValue! as Place)
517 - : place;
518 - })
519 - );
530 + if (rightPlace !== null && rightValue !== null) {
531 rightInstructions.forEach((instr) =>
532 mapInstructionOperands(instr as Instruction, (place) => {
522 - return place.identifier === leftPlace!.identifier
523 - ? (leftValue! as Place)
533 + return place.identifier === rightPlace!.identifier
534 + ? (rightValue as Place)
535 : place;
536 })
537 );
@@ -557,15 +568,6 @@ class Driver {
568 right,
569 loc: terminal.loc,
570 };
560 - console.log(
561 - printTerminal(terminal) +
562 - " testBlock=" +
563 - testBlock.id +
564 - " " +
565 - printPlace(place) +
566 - "=" +
567 - printInstructionValue(value)
568 - );
571 return {
572 place: { ...place },
573 value,
compiler/forget/src/__tests__/fixtures/hir/logical-expression.expect.md
+29 -8
@@ -4,8 +4,9 @@
4 ```javascript
5 // @only
6 function component(props) {
7 - let a = (props.a && props.b && props.c) || props.d;
8 - return a;
7 + let a = props.a || (props.b && props.c && props.d);
8 + let b = (props.a && props.b && props.c) || props.d;
9 + return { a, b };
10 // let b = props.c || props.d;
11 // let c = props.e ?? props.f;
12 // return ((a && b) || c) ?? null;
@@ -20,16 +21,36 @@ function component(props) {
21 function component(props) {
22 const $ = React.useMemoCache();
23 const c_0 = $[0] !== props;
23 - let t1;
24 + let a;
25 if (c_0) {
25 - t1 = (props.a && props.b && props.c) || props.d;
26 + a = props.a || (props.b && props.c && props.d);
27 + const c_2 = $[2] !== props;
28 + let t3;
29 + if (c_2) {
30 + t3 = (props.a && props.b && props.c) || props.d;
31 + $[2] = props;
32 + $[3] = t3;
33 + } else {
34 + t3 = $[3];
35 + }
36 $[0] = props;
27 - $[1] = t1;
37 + $[1] = a;
38 } else {
29 - t1 = $[1];
39 + a = $[1];
40 }
31 - const a = t1;
32 - return a;
41 + const b = t3;
42 + const c_4 = $[4] !== a;
43 + const c_5 = $[5] !== b;
44 + let t6;
45 + if (c_4 || c_5) {
46 + t6 = { a: a, b: b };
47 + $[4] = a;
48 + $[5] = b;
49 + $[6] = t6;
50 + } else {
51 + t6 = $[6];
52 + }
53 + return t6;
54 }
55
56 ```
compiler/forget/src/__tests__/fixtures/hir/logical-expression.js
+3 -2
@@ -1,7 +1,8 @@
1 // @only
2 function component(props) {
3 - let a = (props.a && props.b && props.c) || props.d;
4 - return a;
3 + let a = props.a || (props.b && props.c && props.d);
4 + let b = (props.a && props.b && props.c) || props.d;
5 + return { a, b };
6 // let b = props.c || props.d;
7 // let c = props.e ?? props.f;
8 // return ((a && b) || c) ?? null;