From d243e748d00773c1eb9dbdc4eceed019323fdc9d Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 5 Apr 2023 16:26:40 -0700 Subject: [PATCH] Build -> Codegen for LabelTerminal NOTE: See background in #1476. Updates BuildHIR to use the new LabelTerminal for LabeledStatements, and adds support for HIR->ReactiveFunction transformation and codegen. Note that we sometimes produce an extraneous block wrapper if it turns out the label wasn't necessary, that seems...fine? --- compiler/forget/src/HIR/BuildHIR.ts | 19 ++++-- compiler/forget/src/HIR/HIR.ts | 8 ++- compiler/forget/src/HIR/HIRBuilder.ts | 3 +- .../ReactiveScopes/BuildReactiveFunction.ts | 31 +++++++++- .../ReactiveScopes/CodegenReactiveFunction.ts | 3 + .../ReactiveScopes/FlattenReactiveLoops.ts | 1 + .../ReactiveScopes/PrintReactiveFunction.ts | 6 ++ .../PropagateScopeDependencies.ts | 4 ++ .../forget/src/ReactiveScopes/visitors.ts | 8 +++ compiler/forget/src/SSA/LeaveSSA.ts | 3 +- .../fixtures/compiler/complex-while.expect.md | 10 ++-- .../conditional-break-labeled.expect.md | 60 +++++++++++++++++++ ...abeled.js => conditional-break-labeled.js} | 0 .../error.conditional-break-labeled.expect.md | 30 ---------- ...e.expect.md => inverted-if-else.expect.md} | 18 ++++-- ...nverted-if-else.js => inverted-if-else.js} | 0 .../fixtures/compiler/inverted-if.expect.md | 14 +++-- .../unconditional-break-label.expect.md | 4 +- 18 files changed, 167 insertions(+), 55 deletions(-) create mode 100644 compiler/forget/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md rename compiler/forget/src/__tests__/fixtures/compiler/{error.conditional-break-labeled.js => conditional-break-labeled.js} (100%) delete mode 100644 compiler/forget/src/__tests__/fixtures/compiler/error.conditional-break-labeled.expect.md rename compiler/forget/src/__tests__/fixtures/compiler/{error.inverted-if-else.expect.md => inverted-if-else.expect.md} (52%) rename compiler/forget/src/__tests__/fixtures/compiler/{error.inverted-if-else.js => inverted-if-else.js} (100%) diff --git a/compiler/forget/src/HIR/BuildHIR.ts b/compiler/forget/src/HIR/BuildHIR.ts index 7c5c7abecc..36f338f37f 100644 --- a/compiler/forget/src/HIR/BuildHIR.ts +++ b/compiler/forget/src/HIR/BuildHIR.ts @@ -450,15 +450,24 @@ function lowerStatement( // All other statements create a continuation block to allow `break`, // explicitly *don't* pass the label down const continuationBlock = builder.reserve("block"); - builder.label(label, continuationBlock.id, () => { - lowerStatement(builder, stmt.get("body")); - }); - builder.terminateWithContinuation( - { + const block = builder.enter("block", () => { + builder.label(label, continuationBlock.id, () => { + lowerStatement(builder, stmt.get("body")); + }); + return { kind: "goto", block: continuationBlock.id, variant: GotoVariant.Break, id: makeInstructionId(0), + }; + }); + builder.terminateWithContinuation( + { + kind: "label", + block, + fallthrough: continuationBlock.id, + id: makeInstructionId(0), + loc: stmt.node.loc ?? GeneratedSource, }, continuationBlock ); diff --git a/compiler/forget/src/HIR/HIR.ts b/compiler/forget/src/HIR/HIR.ts index b3f5d8199e..3caad7f65d 100644 --- a/compiler/forget/src/HIR/HIR.ts +++ b/compiler/forget/src/HIR/HIR.ts @@ -135,7 +135,8 @@ export type ReactiveTerminal = | ReactiveWhileTerminal | ReactiveForTerminal | ReactiveForOfTerminal - | ReactiveIfTerminal; + | ReactiveIfTerminal + | ReactiveLabelTerminal; export type ReactiveBreakTerminal = { kind: "break"; @@ -201,6 +202,11 @@ export type ReactiveIfTerminal = { alternate: ReactiveBlock | null; id: InstructionId; }; +export type ReactiveLabelTerminal = { + kind: "label"; + block: ReactiveBlock; + id: InstructionId; +}; /** * A function lowered to HIR form, ie where its body is lowered to an HIR control-flow graph diff --git a/compiler/forget/src/HIR/HIRBuilder.ts b/compiler/forget/src/HIR/HIRBuilder.ts index 7f77329872..b4b2c147f3 100644 --- a/compiler/forget/src/HIR/HIRBuilder.ts +++ b/compiler/forget/src/HIR/HIRBuilder.ts @@ -541,7 +541,8 @@ export function removeUnreachableFallthroughs(func: HIR): void { if ( block.terminal.kind === "if" || block.terminal.kind === "switch" || - block.terminal.kind === "while" + block.terminal.kind === "while" || + block.terminal.kind === "label" ) { if ( block.terminal.fallthrough !== null && diff --git a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts index 597fb086e1..bb373124da 100644 --- a/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/BuildReactiveFunction.ts @@ -29,7 +29,6 @@ import { ReactiveValue, Terminal, } from "../HIR/HIR"; -import todo from "../Utils/todo"; import { assertExhaustive } from "../Utils/utils"; /** @@ -531,7 +530,35 @@ class Driver { break; } case "label": { - todo("Support label terminals"); + const fallthroughId = + terminal.fallthrough !== null && + !this.cx.isScheduled(terminal.fallthrough) + ? terminal.fallthrough + : null; + if (fallthroughId !== null) { + const scheduleId = this.cx.schedule(fallthroughId, "if"); + scheduleIds.push(scheduleId); + } + + const block = this.traverseBlock( + this.cx.ir.blocks.get(terminal.block)! + ); + + this.cx.unscheduleAll(scheduleIds); + blockValue.push({ + kind: "terminal", + terminal: { + kind: "label", + block, + id: terminal.id, + }, + label: fallthroughId, + }); + if (fallthroughId !== null) { + this.visitBlock(this.cx.ir.blocks.get(fallthroughId)!, blockValue); + } + + break; } case "optional-call": case "ternary": diff --git a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts index ba53166f23..23330cea76 100644 --- a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -409,6 +409,9 @@ function codegenTerminal( const test = codegenInstructionValue(cx, terminal.test); return t.whileStatement(test, codegenBlock(cx, terminal.loop)); } + case "label": { + return codegenBlock(cx, terminal.block); + } default: { assertExhaustive( terminal, diff --git a/compiler/forget/src/ReactiveScopes/FlattenReactiveLoops.ts b/compiler/forget/src/ReactiveScopes/FlattenReactiveLoops.ts index 469bfc46b7..461fc8e23f 100644 --- a/compiler/forget/src/ReactiveScopes/FlattenReactiveLoops.ts +++ b/compiler/forget/src/ReactiveScopes/FlattenReactiveLoops.ts @@ -54,6 +54,7 @@ class Transform extends ReactiveFunctionTransform { break; } // Non-loop terminals passthrough is contextual, inherits the parent isWithinScope + case "label": case "break": case "continue": case "if": diff --git a/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts index b0c073d195..3f4a5d3457 100644 --- a/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts @@ -258,6 +258,12 @@ function printTerminal(writer: Writer, terminal: ReactiveTerminal): void { } break; } + case "label": { + writer.writeLine("{"); + printReactiveInstructions(writer, terminal.block); + writer.writeLine("}"); + break; + } default: assertExhaustive(terminal, `Unhandled terminal ${terminal}`); } diff --git a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts index dd21946e1a..3447bad41e 100644 --- a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts +++ b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts @@ -443,6 +443,10 @@ function visit(context: Context, block: ReactiveBlock): void { } break; } + case "label": { + visit(context, terminal.block); + break; + } default: { assertExhaustive( terminal, diff --git a/compiler/forget/src/ReactiveScopes/visitors.ts b/compiler/forget/src/ReactiveScopes/visitors.ts index 64fa92508a..3ca2141420 100644 --- a/compiler/forget/src/ReactiveScopes/visitors.ts +++ b/compiler/forget/src/ReactiveScopes/visitors.ts @@ -149,6 +149,10 @@ export class ReactiveFunctionVisitor { } break; } + case "label": { + this.visitBlock(terminal.block, state); + break; + } default: { assertExhaustive( terminal, @@ -355,6 +359,10 @@ export function mapTerminalBlocks( } break; } + case "label": { + terminal.block = fn(terminal.block); + break; + } default: { assertExhaustive( terminal, diff --git a/compiler/forget/src/SSA/LeaveSSA.ts b/compiler/forget/src/SSA/LeaveSSA.ts index 434f9df6db..b6230a2b2c 100644 --- a/compiler/forget/src/SSA/LeaveSSA.ts +++ b/compiler/forget/src/SSA/LeaveSSA.ts @@ -296,7 +296,8 @@ export function leaveSSA(fn: HIRFunction): void { terminal.kind === "while" || terminal.kind === "do-while" || terminal.kind === "for" || - terminal.kind === "for-of") && + terminal.kind === "for-of" || + terminal.kind === "label") && terminal.fallthrough !== null ) { const fallthrough = fn.body.blocks.get(terminal.fallthrough)!; diff --git a/compiler/forget/src/__tests__/fixtures/compiler/complex-while.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/complex-while.expect.md index 2b52261931..d98e5a68f5 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/complex-while.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/complex-while.expect.md @@ -19,10 +19,12 @@ function foo(a, b, c) { ```javascript function foo(a, b, c) { - if (a) { - while (b) { - if (c) { - break; + { + if (a) { + while (b) { + if (c) { + break; + } } } } diff --git a/compiler/forget/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md new file mode 100644 index 0000000000..d1122366eb --- /dev/null +++ b/compiler/forget/src/__tests__/fixtures/compiler/conditional-break-labeled.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +/** + * props.b *does* influence `a` + */ +function Component(props) { + const a = []; + a.push(props.a); + label: { + if (props.b) { + break label; + } + a.push(props.c); + } + a.push(props.d); + return a; +} + +``` + +## Code + +```javascript +/** + * props.b *does* influence `a` + */ +function Component(props) { + const $ = React.unstable_useMemoCache(5); + const c_0 = $[0] !== props.a; + const c_1 = $[1] !== props.b; + const c_2 = $[2] !== props.c; + const c_3 = $[3] !== props.d; + let a; + if (c_0 || c_1 || c_2 || c_3) { + a = []; + a.push(props.a); + bb1: { + if (props.b) { + break bb1; + } + + a.push(props.c); + } + + a.push(props.d); + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; + } else { + a = $[4]; + } + return a; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.conditional-break-labeled.js b/compiler/forget/src/__tests__/fixtures/compiler/conditional-break-labeled.js similarity index 100% rename from compiler/forget/src/__tests__/fixtures/compiler/error.conditional-break-labeled.js rename to compiler/forget/src/__tests__/fixtures/compiler/conditional-break-labeled.js diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.conditional-break-labeled.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/error.conditional-break-labeled.expect.md deleted file mode 100644 index 4bf3feaec6..0000000000 --- a/compiler/forget/src/__tests__/fixtures/compiler/error.conditional-break-labeled.expect.md +++ /dev/null @@ -1,30 +0,0 @@ - -## Input - -```javascript -/** - * props.b *does* influence `a` - */ -function Component(props) { - const a = []; - a.push(props.a); - label: { - if (props.b) { - break label; - } - a.push(props.c); - } - a.push(props.d); - return a; -} - -``` - - -## Error - -``` -Expected a break target -``` - - \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.inverted-if-else.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/inverted-if-else.expect.md similarity index 52% rename from compiler/forget/src/__tests__/fixtures/compiler/error.inverted-if-else.expect.md rename to compiler/forget/src/__tests__/fixtures/compiler/inverted-if-else.expect.md index fea88eedb2..a2b2957d87 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/error.inverted-if-else.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/inverted-if-else.expect.md @@ -16,11 +16,21 @@ function foo(a, b, c) { ``` +## Code -## Error +```javascript +function foo(a, b, c) { + let x = undefined; + bb1: { + if (a) { + x = b; + break bb1; + } + + x = c; + } + return x; +} ``` -Expected a break target -``` - \ No newline at end of file diff --git a/compiler/forget/src/__tests__/fixtures/compiler/error.inverted-if-else.js b/compiler/forget/src/__tests__/fixtures/compiler/inverted-if-else.js similarity index 100% rename from compiler/forget/src/__tests__/fixtures/compiler/error.inverted-if-else.js rename to compiler/forget/src/__tests__/fixtures/compiler/inverted-if-else.js diff --git a/compiler/forget/src/__tests__/fixtures/compiler/inverted-if.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/inverted-if.expect.md index 014e485b24..3d9a33f887 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/inverted-if.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/inverted-if.expect.md @@ -28,13 +28,15 @@ function foo(a, b, c, d) { let y; if (c_0 || c_1 || c_2 || c_3) { y = []; - bb1: if (a) { - if (b) { - y.push(c); - break bb1; - } + bb1: { + if (a) { + if (b) { + y.push(c); + break bb1; + } - y.push(d); + y.push(d); + } } $[0] = a; $[1] = b; diff --git a/compiler/forget/src/__tests__/fixtures/compiler/unconditional-break-label.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/unconditional-break-label.expect.md index 417f1362ea..4e344babdd 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/unconditional-break-label.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/unconditional-break-label.expect.md @@ -17,7 +17,9 @@ function foo(a) { ```javascript function foo(a) { - return a + 1; + { + return a + 1; + } } ```