From 86ffcde8edad93ce311dff9d0d13789daf3eae9e Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 31 Jan 2023 13:39:32 -0800 Subject: [PATCH] [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. --- .../ReactiveScopes/BuildReactiveFunction.ts | 60 ++++++++++--------- .../fixtures/hir/logical-expression.expect.md | 37 +++++++++--- .../fixtures/hir/logical-expression.js | 5 +- 3 files changed, 63 insertions(+), 39 deletions(-) diff --git a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts index 90fe00e8f0..7e252fa85a 100644 --- a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts @@ -31,11 +31,6 @@ import { ReactiveValue, Terminal, } from "../HIR/HIR"; -import { - printInstructionValue, - printPlace, - printTerminal, -} from "../HIR/PrintHIR"; import { mapInstructionOperands } from "../HIR/visitors"; import { assertExhaustive } from "../Utils/utils"; @@ -477,16 +472,16 @@ class Driver { switch (terminal.kind) { case "logical": { let testBlock: BasicBlock; - let leftValue: ReactiveValue | null = null; let leftPlace: Place | null = null; + let leftValue: ReactiveValue | null = null; const defaultTestBlock = this.cx.ir.blocks.get(terminal.test)!; if (defaultTestBlock.terminal.kind === "branch") { testBlock = defaultTestBlock; } else { const leftResult = this.visitValueTerminal(defaultTestBlock.terminal); testBlock = this.cx.ir.blocks.get(leftResult.fallthrough)!; - leftValue = leftResult.value; leftPlace = leftResult.place; + leftValue = leftResult.value; } invariant( @@ -498,9 +493,32 @@ class Driver { testBlock.instructions; const leftBlock = this.cx.ir.blocks.get(testBlock.terminal.consequent)!; leftInstructions.push(...leftBlock.instructions); - // TODO: If right block ends in a value terminal, recursively process with visitValueTerminal - // similar to handling for the compound lhs case. - const rightBlock = this.cx.ir.blocks.get(testBlock.terminal.alternate)!; + if (leftPlace !== null && leftValue !== null) { + leftInstructions.forEach((instr) => + mapInstructionOperands(instr as Instruction, (place) => { + return place.identifier === leftPlace!.identifier + ? (leftValue as Place) + : place; + }) + ); + } + + let rightBlock: BasicBlock; + let rightValue: ReactiveValue | null = null; + let rightPlace: Place | null = null; + const defaultRightBlock = this.cx.ir.blocks.get( + testBlock.terminal.alternate + )!; + if (defaultRightBlock.terminal.kind === "goto") { + rightBlock = defaultRightBlock; + } else { + const rightResult = this.visitValueTerminal( + defaultRightBlock.terminal + ); + rightBlock = this.cx.ir.blocks.get(rightResult.fallthrough)!; + rightPlace = rightResult.place; + rightValue = rightResult.value; + } const rightInstructions: Array = rightBlock.instructions; const place = leftInstructions.at(-1)!.lvalue!.place; @@ -509,18 +527,11 @@ class Driver { rightInstructions.at(-1)!.lvalue!.place.identifier, "Expected both branches of a logical expression to store to the same temporary" ); - if (leftPlace !== null) { - leftInstructions.forEach((instr) => - mapInstructionOperands(instr as Instruction, (place) => { - return place.identifier === leftPlace!.identifier - ? (leftValue! as Place) - : place; - }) - ); + if (rightPlace !== null && rightValue !== null) { rightInstructions.forEach((instr) => mapInstructionOperands(instr as Instruction, (place) => { - return place.identifier === leftPlace!.identifier - ? (leftValue! as Place) + return place.identifier === rightPlace!.identifier + ? (rightValue as Place) : place; }) ); @@ -557,15 +568,6 @@ class Driver { right, loc: terminal.loc, }; - console.log( - printTerminal(terminal) + - " testBlock=" + - testBlock.id + - " " + - printPlace(place) + - "=" + - printInstructionValue(value) - ); return { place: { ...place }, value, diff --git a/compiler/forget/src/__tests__/fixtures/hir/logical-expression.expect.md b/compiler/forget/src/__tests__/fixtures/hir/logical-expression.expect.md index f2c3628cfc..f51073915b 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/logical-expression.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/logical-expression.expect.md @@ -4,8 +4,9 @@ ```javascript // @only function component(props) { - let a = (props.a && props.b && props.c) || props.d; - return a; + let a = props.a || (props.b && props.c && props.d); + let b = (props.a && props.b && props.c) || props.d; + return { a, b }; // let b = props.c || props.d; // let c = props.e ?? props.f; // return ((a && b) || c) ?? null; @@ -20,16 +21,36 @@ function component(props) { function component(props) { const $ = React.useMemoCache(); const c_0 = $[0] !== props; - let t1; + let a; if (c_0) { - t1 = (props.a && props.b && props.c) || props.d; + a = props.a || (props.b && props.c && props.d); + const c_2 = $[2] !== props; + let t3; + if (c_2) { + t3 = (props.a && props.b && props.c) || props.d; + $[2] = props; + $[3] = t3; + } else { + t3 = $[3]; + } $[0] = props; - $[1] = t1; + $[1] = a; } else { - t1 = $[1]; + a = $[1]; } - const a = t1; - return a; + const b = t3; + const c_4 = $[4] !== a; + const c_5 = $[5] !== b; + let t6; + if (c_4 || c_5) { + t6 = { a: a, b: b }; + $[4] = a; + $[5] = b; + $[6] = t6; + } else { + t6 = $[6]; + } + return t6; } ``` diff --git a/compiler/forget/src/__tests__/fixtures/hir/logical-expression.js b/compiler/forget/src/__tests__/fixtures/hir/logical-expression.js index 122434a1b0..60d684e3a1 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/logical-expression.js +++ b/compiler/forget/src/__tests__/fixtures/hir/logical-expression.js @@ -1,7 +1,8 @@ // @only function component(props) { - let a = (props.a && props.b && props.c) || props.d; - return a; + let a = props.a || (props.b && props.c && props.d); + let b = (props.a && props.b && props.c) || props.d; + return { a, b }; // let b = props.c || props.d; // let c = props.e ?? props.f; // return ((a && b) || c) ?? null;