Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(compiler): remove container index from conditional instruction #55190

Closed
wants to merge 1 commit into from
Closed

fix(compiler): remove container index from conditional instruction #55190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...3_compiler_compliance/components_and_directives/content_projection/ng_content_fallback.js
View file
Open in desktop
Expand Up @@ -69,7 +69,7 @@ $r3$.ɵɵdefineComponent({
}
if (rf & 2) {
$r3$.ɵɵadvance(5);
$r3$.ɵɵconditional(5, ctx.hasFooter ? 5 : -1);
$r3$.ɵɵconditional(ctx.hasFooter ? 5 : -1);
$r3$.ɵɵadvance();
$r3$.ɵɵproperty("ngIf", ctx.hasStructural);
}
Expand Down
2 changes: 1 addition & 1 deletion ...cli/test/compliance/test_cases/r3_view_compiler_control_flow/basic_if_else_if_template.js
View file
Open in desktop
Expand Up @@ -33,6 +33,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ctx.value() === 1 ? 2 : ctx.otherValue() === 2 ? 3 : ctx.message ? 4 : 5);
$r3$.ɵɵconditional(ctx.value() === 1 ? 2 : ctx.otherValue() === 2 ? 3 : ctx.message ? 4 : 5);
}
}
2 changes: 1 addition & 1 deletion ...er-cli/test/compliance/test_cases/r3_view_compiler_control_flow/basic_if_else_template.js
View file
Open in desktop
Expand Up @@ -21,6 +21,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ctx.value() ? 2 : 3);
$r3$.ɵɵconditional(ctx.value() ? 2 : 3);
}
}
2 changes: 1 addition & 1 deletion ...ompiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/basic_if_template.js
View file
Open in desktop
Expand Up @@ -15,6 +15,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ctx.value() ? 2 : -1);
$r3$.ɵɵconditional(ctx.value() ? 2 : -1);
}
}
2 changes: 1 addition & 1 deletion ...ler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/basic_switch_template.js
View file
Open in desktop
Expand Up @@ -34,6 +34,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : 5);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : 5);
}
}
6 changes: 3 additions & 3 deletions ...e/test_cases/r3_view_compiler_control_flow/if_nested_alias_listeners_template.pipeline.js
View file
Open in desktop
Expand Up @@ -30,7 +30,7 @@ function MyApp_Conditional_0_Conditional_1_Conditional_1_Template(rf, ctx) {
let $MyApp_Conditional_0_Conditional_1_contFlowTmp$;
const $ctx_r2$ = $r3$.ɵɵnextContext(2);
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(1, ($MyApp_Conditional_0_Conditional_1_contFlowTmp$ = $ctx_r2$.value()) ? 1 : -1, $MyApp_Conditional_0_Conditional_1_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_Conditional_0_Conditional_1_contFlowTmp$ = $ctx_r2$.value()) ? 1 : -1, $MyApp_Conditional_0_Conditional_1_contFlowTmp$);
}
}

Expand All @@ -50,7 +50,7 @@ function MyApp_Conditional_0_Conditional_1_Conditional_1_Template(rf, ctx) {
let $MyApp_Conditional_0_contFlowTmp$;
const $ctx_r0$ = $r3$.ɵɵnextContext();
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(1, ($MyApp_Conditional_0_contFlowTmp$ = $ctx_r0$.value()) ? 1 : -1, $MyApp_Conditional_0_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_Conditional_0_contFlowTmp$ = $ctx_r0$.value()) ? 1 : -1, $MyApp_Conditional_0_contFlowTmp$);
}
}
…
Expand All @@ -60,6 +60,6 @@ function MyApp_Conditional_0_Conditional_1_Conditional_1_Template(rf, ctx) {
}
if (rf & 2) {
let $MyApp_contFlowTmp$;
$r3$.ɵɵconditional(0, ($MyApp_contFlowTmp$ = ctx.value()) ? 0 : -1, $MyApp_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) ? 0 : -1, $MyApp_contFlowTmp$);
}
}
6 changes: 3 additions & 3 deletions ...-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_nested_alias_template.js
View file
Open in desktop
Expand Up @@ -25,7 +25,7 @@ function MyApp_Conditional_0_Conditional_1_Template(rf, ctx) {
…
$r3$.ɵɵtextInterpolate3(" Inner: ", $ctx_r2$.value(), "/", $root_r1$, "/", ctx, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(1, ($MyApp_Conditional_0_Conditional_1_contFlowTmp$ = $ctx_r2$.value()) ? 1 : -1, $MyApp_Conditional_0_Conditional_1_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_Conditional_0_Conditional_1_contFlowTmp$ = $ctx_r2$.value()) ? 1 : -1, $MyApp_Conditional_0_Conditional_1_contFlowTmp$);
}
}

Expand All @@ -40,7 +40,7 @@ function MyApp_Conditional_0_Template(rf, ctx) {
…
$r3$.ɵɵtextInterpolate2(" Root: ", $ctx_r0$.value(), "/", ctx, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(1, ($MyApp_Conditional_0_contFlowTmp$ = $ctx_r0$.value()) ? 1 : -1, $MyApp_Conditional_0_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_Conditional_0_contFlowTmp$ = $ctx_r0$.value()) ? 1 : -1, $MyApp_Conditional_0_contFlowTmp$);
}
}
…
Expand All @@ -50,6 +50,6 @@ function MyApp_Template(rf, ctx) {
}
if (rf & 2) {
let $MyApp_contFlowTmp$;
$r3$.ɵɵconditional(0, ($MyApp_contFlowTmp$ = ctx.value()) ? 0 : -1, $MyApp_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) ? 0 : -1, $MyApp_contFlowTmp$);
}
}
2 changes: 1 addition & 1 deletion ...er-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_with_alias_template.js
View file
Open in desktop
Expand Up @@ -20,6 +20,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.value()) ? 2 : -1, $MyApp_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) ? 2 : -1, $MyApp_contFlowTmp$);
}
}
2 changes: 1 addition & 1 deletion ...est/compliance/test_cases/r3_view_compiler_control_flow/if_with_pipe_template.pipeline.js
View file
Open in desktop
Expand Up @@ -30,6 +30,6 @@ function $MyApp_Conditional_3_Template$(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, $r3$.ɵɵpipeBind1(3, 2, ctx.val) === 1 ? 2 : $r3$.ɵɵpipeBind1(4, 4, ctx.val) === 2 ? 5 : 6);
$r3$.ɵɵconditional($r3$.ɵɵpipeBind1(3, 2, ctx.val) === 1 ? 2 : $r3$.ɵɵpipeBind1(4, 4, ctx.val) === 2 ? 5 : 6);
}
}
4 changes: 2 additions & 2 deletions ...mpiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_if_template.js
View file
Open in desktop
Expand Up @@ -40,7 +40,7 @@ function MyApp_Conditional_4_Template(rf, ctx) {
}
if (rf & 2) {
const $ctx_r2$ = $r3$.ɵɵnextContext();
$r3$.ɵɵconditional(0, $ctx_r2$.innerVal === 0 ? 0 : $ctx_r2$.innerVal === 1 ? 1 : $ctx_r2$.innerVal === 2 ? 2 : 3);
$r3$.ɵɵconditional($ctx_r2$.innerVal === 0 ? 0 : $ctx_r2$.innerVal === 1 ? 1 : $ctx_r2$.innerVal === 2 ? 2 : 3);
}
}

Expand All @@ -61,6 +61,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ctx.val === 0 ? 2 : ctx.val === 1 ? 3 : ctx.val === 2 ? 4 : 5);
$r3$.ɵɵconditional(ctx.val === 0 ? 2 : ctx.val === 1 ? 3 : ctx.val === 2 ? 4 : 5);
}
}
4 changes: 2 additions & 2 deletions ...er-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_switch_template.js
View file
Open in desktop
Expand Up @@ -30,7 +30,7 @@ function MyApp_Case_3_Template(rf, ctx) {
…
const $ctx_r1$ = $r3$.ɵɵnextContext();
…
$r3$.ɵɵconditional(0, ($MyApp_Case_3_contFlowTmp$ = $ctx_r1$.nestedValue()) === 0 ? 0 : $MyApp_Case_3_contFlowTmp$ === 1 ? 1 : $MyApp_Case_3_contFlowTmp$ === 2 ? 2 : -1);
$r3$.ɵɵconditional(($MyApp_Case_3_contFlowTmp$ = $ctx_r1$.nestedValue()) === 0 ? 0 : $MyApp_Case_3_contFlowTmp$ === 1 ? 1 : $MyApp_Case_3_contFlowTmp$ === 2 ? 2 : -1);
}
}

Expand All @@ -52,6 +52,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : -1);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : -1);
}
}
2 changes: 1 addition & 1 deletion ...compliance/test_cases/r3_view_compiler_control_flow/switch_with_pipe_template.pipeline.js
View file
Open in desktop
Expand Up @@ -14,6 +14,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = $r3$.ɵɵpipeBind1(3, 2, ctx.value())) === $r3$.ɵɵpipeBind1(4, 4, 0) ? 2 : $MyApp_contFlowTmp$ === $r3$.ɵɵpipeBind1(5, 6, 1) ? 6 : 7);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = $r3$.ɵɵpipeBind1(3, 2, ctx.value())) === $r3$.ɵɵpipeBind1(4, 4, 0) ? 2 : $MyApp_contFlowTmp$ === $r3$.ɵɵpipeBind1(5, 6, 1) ? 6 : 7);
}
}
2 changes: 1 addition & 1 deletion ...st/compliance/test_cases/r3_view_compiler_control_flow/switch_without_default_template.js
View file
Open in desktop
Expand Up @@ -28,6 +28,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : -1);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : -1);
}
}
4 changes: 2 additions & 2 deletions ...piler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional_template.js
View file
Open in desktop
Expand Up @@ -88,9 +88,9 @@ MyApp.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
}
if (rf & 2) {
$r3$.ɵɵadvance(2);
$r3$.ɵɵconditional(2, ctx.count === 0 ? 2 : ctx.count === 1 ? 3 : 4);
$r3$.ɵɵconditional(ctx.count === 0 ? 2 : ctx.count === 1 ? 3 : 4);
$r3$.ɵɵadvance(3);
$r3$.ɵɵconditional(5, ctx.count === 7 ? 5 : -1);
$r3$.ɵɵconditional(ctx.count === 7 ? 5 : -1);
}
},
…
Expand Down
2 changes: 1 addition & 1 deletion ...s/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/switch_template.js
View file
Open in desktop
Expand Up @@ -77,7 +77,7 @@ MyApp.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
if (rf & 2) {
let $MyApp_contFlowTmp$;
$r3$.ɵɵadvance(2);
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.count) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : 4);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.count) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : 4);
}
},
…
Expand Down
10 changes: 2 additions & 8 deletions packages/compiler/src/template/pipeline/ir/src/ops/update.ts
View file
Open in desktop
Expand Up @@ -598,11 +598,6 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
*/
target: XrefId;

/**
* The slot of the target, to be populated during slot allocation.
*/
targetSlot: SlotHandle;

/**
* The main test expression (for a switch), or `null` (for an if, which has no test
* expression).
Expand Down Expand Up @@ -634,12 +629,11 @@ 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, targetSlot: SlotHandle, test: o.Expression|null,
conditions: Array<ConditionalCaseExpr>, sourceSpan: ParseSourceSpan): ConditionalOp {
target: XrefId, test: o.Expression|null, conditions: Array<ConditionalCaseExpr>,
sourceSpan: ParseSourceSpan): ConditionalOp {
return {
kind: OpKind.Conditional,
target,
targetSlot,
test,
conditions,
processed: null,
Expand Down
15 changes: 4 additions & 11 deletions packages/compiler/src/template/pipeline/src/ingest.ts
View file
Open in desktop
Expand Up @@ -355,7 +355,6 @@ function ingestBoundText(
*/
function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
let firstXref: ir.XrefId|null = null;
let firstSlotHandle: ir.SlotHandle|null = null;
let conditions: Array<ir.ConditionalCaseExpr> = [];
for (let i = 0; i < ifBlock.branches.length; i++) {
const ifCase = ifBlock.branches[i];
Expand All @@ -381,7 +380,6 @@ function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {

if (firstXref === null) {
firstXref = cView.xref;
firstSlotHandle = templateOp.handle;
}

const caseExpr = ifCase.expression ? convertAst(ifCase.expression, unit.job, null) : null;
Expand All @@ -390,9 +388,7 @@ function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
conditions.push(conditionalCaseExpr);
ingestNodes(cView, ifCase.children);
}
const conditional =
ir.createConditionalOp(firstXref!, firstSlotHandle!, null, conditions, ifBlock.sourceSpan);
unit.update.push(conditional);
unit.update.push(ir.createConditionalOp(firstXref!, null, conditions, ifBlock.sourceSpan));
}

/**
Expand All @@ -405,7 +401,6 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
}

let firstXref: ir.XrefId|null = null;
let firstSlotHandle: ir.SlotHandle|null = null;
let conditions: Array<ir.ConditionalCaseExpr> = [];
for (const switchCase of switchBlock.cases) {
const cView = unit.job.allocateView(unit.xref);
Expand All @@ -424,7 +419,6 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
unit.create.push(templateOp);
if (firstXref === null) {
firstXref = cView.xref;
firstSlotHandle = templateOp.handle;
}
const caseExpr = switchCase.expression ?
convertAst(switchCase.expression, unit.job, switchBlock.startSourceSpan) :
Expand All @@ -434,10 +428,9 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
conditions.push(conditionalCaseExpr);
ingestNodes(cView, switchCase.children);
}
const conditional = ir.createConditionalOp(
firstXref!, firstSlotHandle!, convertAst(switchBlock.expression, unit.job, null), conditions,
switchBlock.sourceSpan);
unit.update.push(conditional);
unit.update.push(ir.createConditionalOp(
firstXref!, convertAst(switchBlock.expression, unit.job, null), conditions,
switchBlock.sourceSpan));
}

function ingestDeferView(
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler/src/template/pipeline/src/instruction.ts
View file
Open in desktop
Expand Up @@ -560,9 +560,9 @@ function call<OpT extends ir.CreateOp|ir.UpdateOp>(
}

export function conditional(
slot: number, condition: o.Expression, contextValue: o.Expression|null,
condition: o.Expression, contextValue: o.Expression|null,
sourceSpan: ParseSourceSpan|null): ir.UpdateOp {
const args = [o.literal(slot), condition];
const args = [condition];
if (contextValue !== null) {
args.push(contextValue);
}
Expand Down
6 changes: 1 addition & 5 deletions packages/compiler/src/template/pipeline/src/phases/reify.ts
View file
Open in desktop
Expand Up @@ -416,11 +416,7 @@ function reifyUpdateOperations(_unit: CompilationUnit, ops: ir.OpList<ir.UpdateO
if (op.processed === null) {
throw new Error(`Conditional test was not set.`);
}
if (op.targetSlot.slot === null) {
throw new Error(`Conditional slot was not set.`);
}
ir.OpList.replace(
op, ng.conditional(op.targetSlot.slot, op.processed, op.contextValue, op.sourceSpan));
ir.OpList.replace(op, ng.conditional(op.processed, op.contextValue, op.sourceSpan));
break;
case ir.OpKind.Repeater:
ir.OpList.replace(op, ng.repeater(op.collection, op.sourceSpan));
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/render3/instructions/control_flow.ts
View file
Open in desktop
Expand Up @@ -33,12 +33,12 @@ import {declareTemplate} from './template';
* built-in "if" and "switch". On the high level this instruction is responsible for adding and
* removing views selected by a conditional expression.
*
* @param matchingTemplateIndex index of a template TNode representing a conditional view to be
* @param matchingTemplateIndex Index of a template TNode representing a conditional view to be
* inserted; -1 represents a special case when there is no view to insert.
* @param contextValue Value that should be exposed as the context of the conditional.
* @codeGenApi
*/
export function ɵɵconditional<T>(containerIndex: number, matchingTemplateIndex: number, value?: T) {
// TODO: we could remove the containerIndex argument to this instruction now (!)
export function ɵɵconditional<T>(matchingTemplateIndex: number, contextValue?: T) {
performanceMarkFeature('NgControlFlow');

const hostLView = getLView();
Expand Down Expand Up @@ -69,7 +69,7 @@ export function ɵɵconditional<T>(containerIndex: number, matchingTemplateIndex
const dehydratedView =
findMatchingDehydratedView(nextContainer, templateTNode.tView!.ssrId);
const embeddedLView =
createAndRenderEmbeddedLView(hostLView, templateTNode, value, {dehydratedView});
createAndRenderEmbeddedLView(hostLView, templateTNode, contextValue, {dehydratedView});

addLViewToLContainer(
nextContainer, embeddedLView, viewInContainerIdx,
Expand All @@ -83,7 +83,7 @@ export function ɵɵconditional<T>(containerIndex: number, matchingTemplateIndex
// changed - re-bind in context.
const lView = getLViewFromLContainer<T|undefined>(prevContainer, viewInContainerIdx);
if (lView !== undefined) {
lView[CONTEXT] = value;
lView[CONTEXT] = contextValue;
}
}
}
Expand Down