refactor(compiler): Move conditional branch logic into an expression. (#51931)

This is a pure refactor: we previously crammed a lot of data into a complicated array on the conditional op. Now, we use a new conditional branch expression to store that information.

PR Close #51931
This commit is contained in:
Dylan Hunn 2023-09-26 22:25:18 -07:00 committed by Alex Rickabaugh
parent 56d25add17
commit aa0ac8a74c
5 changed files with 64 additions and 16 deletions

View file

@ -304,6 +304,11 @@ export enum ExpressionKind {
* An expression that will cause a literal slot index to be emitted.
*/
SlotLiteralExpr,
/**
* A test expression for a conditional op.
*/
ConditionalCase,
}
/**

View file

@ -23,7 +23,7 @@ export type Expression =
LexicalReadExpr|ReferenceExpr|ContextExpr|NextContextExpr|GetCurrentViewExpr|RestoreViewExpr|
ResetViewExpr|ReadVariableExpr|PureFunctionExpr|PureFunctionParameterExpr|PipeBindingExpr|
PipeBindingVariadicExpr|SafePropertyReadExpr|SafeKeyedReadExpr|SafeInvokeFunctionExpr|EmptyExpr|
AssignTemporaryExpr|ReadTemporaryExpr|SanitizerExpr|SlotLiteralExpr;
AssignTemporaryExpr|ReadTemporaryExpr|SanitizerExpr|SlotLiteralExpr|ConditionalCaseExpr;
/**
* Transformer type which converts expressions into general `o.Expression`s (which may be an
@ -762,6 +762,44 @@ export class SlotLiteralExpr extends ExpressionBase {
override transformInternalExpressions(): void {}
}
export class ConditionalCaseExpr extends ExpressionBase {
override readonly kind = ExpressionKind.ConditionalCase;
/**
* Create an expression for one branch of a conditional.
* @param expr The expression to be tested for this case. Might be null, as in an `else` case.
* @param target The Xref of the view to be displayed if this condition is true.
*/
constructor(public expr: o.Expression|null, readonly target: XrefId) {
super();
}
override visitExpression(visitor: o.ExpressionVisitor, context: any): any {
if (this.expr !== null) {
this.expr.visitExpression(visitor, context);
}
}
override isEquivalent(e: Expression): boolean {
return e instanceof ConditionalCaseExpr && e.expr === this.expr;
}
override isConstant() {
return true;
}
override clone(): ConditionalCaseExpr {
return new ConditionalCaseExpr(this.expr, this.target);
}
override transformInternalExpressions(transform: ExpressionTransform, flags: VisitorContextFlag):
void {
if (this.expr !== null) {
this.expr = transformExpressionsInExpression(this.expr, transform, flags);
}
}
}
/**
* Visits all `Expression`s in the AST of `op` with the `visitor` function.
*/
@ -831,11 +869,11 @@ export function transformExpressionsInOp(
break;
case OpKind.Conditional:
for (const condition of op.conditions) {
if (condition[1] === null) {
if (condition.expr === null) {
// This is a default case.
continue;
}
condition[1] = transformExpressionsInExpression(condition[1]!, transform, flags);
condition.expr = transformExpressionsInExpression(condition.expr, transform, flags);
}
if (op.processed !== null) {
op.processed = transformExpressionsInExpression(op.processed, transform, flags);

View file

@ -11,6 +11,7 @@ import * as i18n from '../../../../../i18n/i18n_ast';
import * as o from '../../../../../output/output_ast';
import {ParseSourceSpan} from '../../../../../parse_util';
import {BindingKind, OpKind} from '../enums';
import type {ConditionalCaseExpr} from '../expression';
import {Op, XrefId} from '../operations';
import {ConsumesVarsTrait, DependsOnSlotContextOpTrait, TRAIT_CONSUMES_VARS, TRAIT_DEPENDS_ON_SLOT_CONTEXT, TRAIT_USES_SLOT_INDEX, UsesSlotIndexTrait} from '../traits';
@ -496,7 +497,7 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
* Each possible embedded view that could be displayed has a condition (or is default). This
* structure maps each view xref to its corresponding condition.
*/
conditions: Array<[XrefId, o.Expression|null]>;
conditions: Array<ConditionalCaseExpr>;
/**
* After processing, this will be a single collapsed Joost-expression that evaluates to the right
@ -511,13 +512,13 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
* Create a conditional op, which will display an embedded view according to a condtion.
*/
export function createConditionalOp(
target: XrefId, test: o.Expression|null, conditions: Array<[XrefId, o.Expression | null]>,
target: XrefId, test: o.Expression|null, conditions: Array<ConditionalCaseExpr>,
sourceSpan: ParseSourceSpan): ConditionalOp {
return {
kind: OpKind.Conditional,
target,
test,
conditions: conditions,
conditions,
processed: null,
sourceSpan,
...NEW_OP,

View file

@ -269,7 +269,7 @@ function ingestBoundText(unit: ViewCompilationUnit, text: t.BoundText): void {
*/
function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
let firstXref: ir.XrefId|null = null;
let conditions: Array<[ir.XrefId, o.Expression | null]> = [];
let conditions: Array<ir.ConditionalCaseExpr> = [];
for (const ifCase of ifBlock.branches) {
const cView = unit.job.allocateView(unit.xref);
if (firstXref === null) {
@ -278,7 +278,8 @@ function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
unit.create.push(
ir.createTemplateOp(cView.xref, 'Conditional', ir.Namespace.HTML, true, ifCase.sourceSpan));
const caseExpr = ifCase.expression ? convertAst(ifCase.expression, unit.job, null) : null;
conditions.push([cView.xref, caseExpr]);
const conditionalCaseExpr = new ir.ConditionalCaseExpr(caseExpr, cView.xref);
conditions.push(conditionalCaseExpr);
ingestNodes(cView, ifCase.children);
}
const conditional = ir.createConditionalOp(firstXref!, null, conditions, ifBlock.sourceSpan);
@ -290,7 +291,7 @@ function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
*/
function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock): void {
let firstXref: ir.XrefId|null = null;
let conditions: Array<[ir.XrefId, o.Expression | null]> = [];
let conditions: Array<ir.ConditionalCaseExpr> = [];
for (const switchCase of switchBlock.cases) {
const cView = unit.job.allocateView(unit.xref);
if (firstXref === null) {
@ -301,7 +302,8 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
const caseExpr = switchCase.expression ?
convertAst(switchCase.expression, unit.job, switchBlock.startSourceSpan) :
null;
conditions.push([cView.xref, caseExpr]);
const conditionalCaseExpr = new ir.ConditionalCaseExpr(caseExpr, cView.xref);
conditions.push(conditionalCaseExpr);
ingestNodes(cView, switchCase.children);
}
const conditional = ir.createConditionalOp(

View file

@ -23,9 +23,9 @@ export function phaseConditionals(job: ComponentCompilationJob): void {
let test: o.Expression;
// Any case with a `null` condition is `default`. If one exists, default to it instead.
const defaultCase = op.conditions.findIndex(([xref, cond]) => cond === null);
const defaultCase = op.conditions.findIndex((cond) => cond.expr === null);
if (defaultCase >= 0) {
const [xref, cond] = op.conditions.splice(defaultCase, 1)[0];
const xref = op.conditions.splice(defaultCase, 1)[0].target;
test = new ir.SlotLiteralExpr(xref);
} else {
// By default, a switch evaluates to `-1`, causing no template to be displayed.
@ -37,15 +37,17 @@ export function phaseConditionals(job: ComponentCompilationJob): void {
// For each remaining condition, test whether the temporary satifies the check.
for (let i = op.conditions.length - 1; i >= 0; i--) {
let [xref, check]: [ir.XrefId, o.Expression|null] = op.conditions[i];
if (check === null) {
let conditionalCase = op.conditions[i];
if (conditionalCase.expr === null) {
continue;
}
if (tmp !== null) {
const useTmp = i === 0 ? tmp : new ir.ReadTemporaryExpr(tmp.xref);
check = new o.BinaryOperatorExpr(o.BinaryOperator.Identical, useTmp, check);
conditionalCase.expr =
new o.BinaryOperatorExpr(o.BinaryOperator.Identical, useTmp, conditionalCase.expr);
}
test = new o.ConditionalExpr(check, new ir.SlotLiteralExpr(xref), test);
test = new o.ConditionalExpr(
conditionalCase.expr, new ir.SlotLiteralExpr(conditionalCase.target), test);
}
// Save the resulting aggregate Joost-expression.