From 27e20396302104f01296739e896529e5c4647372 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 21 Aug 2018 18:52:26 -0700 Subject: [PATCH] fix(compiler): update compiler to generate new slot allocations (#25607) PR Close #25607 --- .../compliance/r3_compiler_compliance_spec.ts | 34 +++++------- .../r3_view_compiler_template_spec.ts | 6 +-- .../compiler/src/render3/r3_identifiers.ts | 3 -- .../compiler/src/render3/view/template.ts | 54 ++++++++++++++----- .../core/src/core_render3_private_export.ts | 1 - packages/core/src/render3/index.ts | 2 - packages/core/src/render3/instructions.ts | 3 -- packages/core/src/render3/jit/environment.ts | 1 - packages/core/src/render3/pipe.ts | 10 ++-- 9 files changed, 62 insertions(+), 52 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index 0e268026ab..6ddae7974d 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -364,13 +364,12 @@ describe('compiler compliance', () => { if (rf & 1) { $r3$.ɵelement(0, "div"); $r3$.ɵpipe(1,"pipe"); - $r3$.ɵreserveSlots(10); } if (rf & 2) { - $r3$.ɵelementProperty(0, "ternary", $r3$.ɵbind((ctx.cond ? $r3$.ɵpureFunction1(6, _c0, ctx.a): _c1))); + $r3$.ɵelementProperty(0, "ternary", $r3$.ɵbind((ctx.cond ? $r3$.ɵpureFunction1(8, _c0, ctx.a): _c1))); $r3$.ɵelementProperty(0, "pipe", $r3$.ɵbind($r3$.ɵpipeBind3(1, 4, ctx.value, 1, 2))); - $r3$.ɵelementProperty(0, "and", $r3$.ɵbind((ctx.cond && $r3$.ɵpureFunction1(8, _c0, ctx.b)))); - $r3$.ɵelementProperty(0, "or", $r3$.ɵbind((ctx.cond || $r3$.ɵpureFunction1(10, _c0, ctx.c)))); + $r3$.ɵelementProperty(0, "and", $r3$.ɵbind((ctx.cond && $r3$.ɵpureFunction1(10, _c0, ctx.b)))); + $r3$.ɵelementProperty(0, "or", $r3$.ɵbind((ctx.cond || $r3$.ɵpureFunction1(12, _c0, ctx.c)))); } } `; @@ -656,7 +655,7 @@ describe('compiler compliance', () => { template: function MyComponent_Template(rf, ctx) { if (rf & 1) { $r3$.ɵelementStart(0, "ul", null, $c1$); - $r3$.ɵtemplate(2, MyComponent_li_Template_2, 2, 1, null, $c2$); + $r3$.ɵtemplate(2, MyComponent_li_Template_2, 2, 2, null, $c2$); $r3$.ɵelementEnd(); } }, @@ -718,10 +717,9 @@ describe('compiler compliance', () => { template: function MyApp_Template(rf, ctx) { if (rf & 1) { $r3$.ɵelement(0, "my-comp"); - $r3$.ɵreserveSlots(2); } if (rf & 2) { - $r3$.ɵelementProperty(0, "names", $r3$.ɵbind($r3$.ɵpureFunction1(2, $e0_ff$, ctx.customName))); + $r3$.ɵelementProperty(0, "names", $r3$.ɵbind($r3$.ɵpureFunction1(1, $e0_ff$, ctx.customName))); } }, directives: [MyComp] @@ -800,12 +798,11 @@ describe('compiler compliance', () => { template: function MyApp_Template(rf, ctx) { if (rf & 1) { $r3$.ɵelement(0, "my-comp"); - $r3$.ɵreserveSlots(10); } if (rf & 2) { $r3$.ɵelementProperty( 0, "names", - $r3$.ɵbind($r3$.ɵpureFunctionV(10, $e0_ff$, [ctx.n0, ctx.n1, ctx.n2, ctx.n3, ctx.n4, ctx.n5, ctx.n6, ctx.n7, ctx.n8]))); + $r3$.ɵbind($r3$.ɵpureFunctionV(1, $e0_ff$, [ctx.n0, ctx.n1, ctx.n2, ctx.n3, ctx.n4, ctx.n5, ctx.n6, ctx.n7, ctx.n8]))); } }, directives: [MyComp] @@ -864,10 +861,9 @@ describe('compiler compliance', () => { template: function MyApp_Template(rf, ctx) { if (rf & 1) { $r3$.ɵelement(0, "object-comp"); - $r3$.ɵreserveSlots(2); } if (rf & 2) { - $r3$.ɵelementProperty(0, "config", $r3$.ɵbind($r3$.ɵpureFunction1(2, $e0_ff$, ctx.name))); + $r3$.ɵelementProperty(0, "config", $r3$.ɵbind($r3$.ɵpureFunction1(1, $e0_ff$, ctx.name))); } }, directives: [ObjectComp] @@ -932,12 +928,11 @@ describe('compiler compliance', () => { template: function MyApp_Template(rf, ctx) { if (rf & 1) { $r3$.ɵelement(0, "nested-comp"); - $r3$.ɵreserveSlots(7); } if (rf & 2) { $r3$.ɵelementProperty( 0, "config", - $r3$.ɵbind($r3$.ɵpureFunction2(7, $e0_ff_2$, ctx.name, $r3$.ɵpureFunction1(4, $e0_ff_1$, $r3$.ɵpureFunction1(2, $e0_ff$, ctx.duration))))); + $r3$.ɵbind($r3$.ɵpureFunction2(5, $e0_ff_2$, ctx.name, $r3$.ɵpureFunction1(3, $e0_ff_1$, $r3$.ɵpureFunction1(1, $e0_ff$, ctx.duration))))); } }, directives: [NestedComp] @@ -1254,11 +1249,10 @@ describe('compiler compliance', () => { $r3$.ɵtext(4); $r3$.ɵpipe(5, "myPipe"); $r3$.ɵelementEnd(); - $r3$.ɵreserveSlots(15); } if (rf & 2) { - $r3$.ɵtextBinding(0, $r3$.ɵinterpolation1("", $r3$.ɵpipeBind2(1, 3, $r3$.ɵpipeBind2(2, 6, ctx.name, ctx.size), ctx.size), "")); - $r3$.ɵtextBinding(4, $r3$.ɵinterpolation1("", $r3$.ɵpipeBindV(5, 13 , $r3$.ɵpureFunction1(15, $c0$, ctx.name)), "")); + $r3$.ɵtextBinding(0, $r3$.ɵinterpolation1("", $r3$.ɵpipeBind2(1, 2, $r3$.ɵpipeBind2(2, 5, ctx.name, ctx.size), ctx.size), "")); + $r3$.ɵtextBinding(4, $r3$.ɵinterpolation1("", $r3$.ɵpipeBindV(5, 8, $r3$.ɵpureFunction1(15, $c0$, ctx.name)), "")); } }, pipes: [MyPurePipe, MyPipe] @@ -1373,7 +1367,7 @@ describe('compiler compliance', () => { if (rf & 1) { $r3$.ɵelementStart(0, "div"); $r3$.ɵtext(1); - $r3$.ɵtemplate(2, MyComponent_div_span_Template_2, 2, 1, null, $c2$); + $r3$.ɵtemplate(2, MyComponent_div_span_Template_2, 2, 3, null, $c2$); $r3$.ɵelement(3, "span", null, $c4$); $r3$.ɵelementEnd(); } @@ -1396,7 +1390,7 @@ describe('compiler compliance', () => { if (rf & 1) { $r3$.ɵelement(0, "div", null, $c1$); $r3$.ɵtext(2); - $r3$.ɵtemplate(3, MyComponent_div_Template_3, 5, 1, null, $c2$); + $r3$.ɵtemplate(3, MyComponent_div_Template_3, 5, 2, null, $c2$); $r3$.ɵelement(4, "div", null, $c3$); } if (rf & 2) { @@ -1460,7 +1454,7 @@ describe('compiler compliance', () => { if (rf & 1) { $i0$.ɵelementStart(0, "div"); $i0$.ɵelement(1, "div", null, $c1$); - $i0$.ɵtemplate(3, MyComponent_div_span_Template_3, 2, 1, null, $c2$); + $i0$.ɵtemplate(3, MyComponent_div_span_Template_3, 2, 2, null, $c2$); $i0$.ɵelementEnd(); } if (rf & 2) { @@ -1839,7 +1833,7 @@ describe('compiler compliance', () => { $r3$.ɵtext(2); $r3$.ɵelementEnd(); $r3$.ɵelementStart(3, "ul"); - $r3$.ɵtemplate(4, MyComponent_li_li_Template_4, 2, 1, null, $c1$); + $r3$.ɵtemplate(4, MyComponent_li_li_Template_4, 2, 2, null, $c1$); $r3$.ɵelementEnd(); $r3$.ɵelementEnd(); } diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts index 4cf10f9ed1..45623362ab 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts @@ -155,7 +155,7 @@ describe('compiler compliance: template', () => { // ... template:function MyComponent_Template(rf, ctx){ if (rf & 1) { - $i0$.ɵtemplate(0, MyComponent_span_Template_0, 2, 1, null, _c0); + $i0$.ɵtemplate(0, MyComponent_span_Template_0, 2, 2, null, _c0); } if (rf & 2) { $i0$.ɵelementProperty(0, "ngForOf", $i0$.ɵbind(ctx.items)); @@ -211,7 +211,7 @@ describe('compiler compliance: template', () => { function MyComponent_div_Template_0(rf, ctx) { if (rf & 1) { $i0$.ɵelementStart(0, "div"); - $i0$.ɵtemplate(1, MyComponent_div_span_Template_1, 2, 1, null, $c1$); + $i0$.ɵtemplate(1, MyComponent_div_span_Template_1, 2, 2, null, $c1$); $i0$.ɵelementEnd(); } if (rf & 2) { @@ -279,7 +279,7 @@ describe('compiler compliance: template', () => { function MyComponent_div_div_Template_1(rf, ctx) { if (rf & 1) { $i0$.ɵelementStart(0, "div"); - $i0$.ɵtemplate(1, MyComponent_div_div_div_Template_1, 2, 1, null, _c0); + $i0$.ɵtemplate(1, MyComponent_div_div_div_Template_1, 2, 2, null, _c0); $i0$.ɵelementEnd(); } if (rf & 2) { diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index c66d6fac30..90b4dda1ba 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -190,9 +190,6 @@ export class Identifiers { moduleName: CORE, }; - // Reserve slots for pure functions - static reserveSlots: o.ExternalReference = {name: 'ɵreserveSlots', moduleName: CORE}; - // sanitization-related functions static sanitizeHtml: o.ExternalReference = {name: 'ɵzh', moduleName: CORE}; static sanitizeStyle: o.ExternalReference = {name: 'ɵzs', moduleName: CORE}; diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index e0ad56d1b5..ffe9fe3801 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -110,7 +110,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this._valueConverter = new ValueConverter( constantPool, () => this.allocateDataSlot(), - (numSlots: number): number => this._pureFunctionSlots += numSlots, + (numSlots: number) => this.allocatePureFunctionSlots(numSlots), (name, localName, slot, value: o.ReadVarExpr) => { const pipeType = pipeTypeByName.get(name); if (pipeType) { @@ -171,24 +171,27 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // This is the initial pass through the nodes of this template. In this pass, we // queue all creation mode and update mode instructions for generation in the second // pass. It's necessary to separate the passes to ensure local refs are defined before - // resolving bindings. + // resolving bindings. We also count bindings in this pass as we walk bound expressions. t.visitAll(this, nodes); + // Add total binding count to pure function count so pure function instructions are + // generated with the correct slot offset when update instructions are processed. + this._pureFunctionSlots += this._bindingSlots; + + // Pipes are walked in the first pass (to enqueue `pipe()` creation instructions and + // `pipeBind` update instructions), so we have to update the slot offsets manually + // to account for bindings. + this._valueConverter.updatePipeSlotOffsets(this._bindingSlots); + // Nested templates must be processed before creation instructions so template() // instructions can be generated with the correct internal const count. this._nestedTemplateFns.forEach(buildTemplateFn => buildTemplateFn()); - // Generate all the update mode instructions (e.g. resolve property or text bindings) - const updateStatements = this._updateCodeFns.map((fn: () => o.Statement) => fn()); - // Generate all the creation mode instructions (e.g. resolve bindings in listeners) const creationStatements = this._creationCodeFns.map((fn: () => o.Statement) => fn()); - // To count slots for the reserveSlots() instruction, all bindings must have been visited. - if (this._pureFunctionSlots > 0) { - creationStatements.push( - instruction(null, R3.reserveSlots, [o.literal(this._pureFunctionSlots)]).toStmt()); - } + // Generate all the update mode instructions (e.g. resolve property or text bindings) + const updateStatements = this._updateCodeFns.map((fn: () => o.Statement) => fn()); // Variable declaration must occur after binding resolution so we can generate context // instructions that build on each other. e.g. const b = x().$implicit(); const b = x(); @@ -635,6 +638,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // TODO(chuckj): runtime: security context? const value = input.value.visit(this._valueConverter); + this.allocateBindingSlots(value); this.updateInstruction(input.sourceSpan, instruction, () => { return [ o.literal(elementIndex), o.literal(input.name), @@ -722,6 +726,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const context = o.variable(CONTEXT_NAME); template.inputs.forEach(input => { const value = input.value.visit(this._valueConverter); + this.allocateBindingSlots(value); this.updateInstruction(template.sourceSpan, R3.elementProperty, () => { return [ o.literal(templateIndex), o.literal(input.name), @@ -767,6 +772,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.creationInstruction(text.sourceSpan, R3.text, [o.literal(nodeIndex)]); const value = text.value.visit(this._valueConverter); + this.allocateBindingSlots(value); this.updateInstruction( text.sourceSpan, R3.textBinding, () => [o.literal(nodeIndex), this.convertPropertyBinding(o.variable(CONTEXT_NAME), value)]); @@ -800,7 +806,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver getConstCount() { return this._dataIndex; } - getVarCount() { return this._bindingSlots + this._pureFunctionSlots; } + getVarCount() { return this._pureFunctionSlots; } private bindingContext() { return `${this._bindingContext++}`; } @@ -829,10 +835,18 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.instructionFn(this._updateCodeFns, span, reference, paramsOrFn || []); } + private allocatePureFunctionSlots(numSlots: number): number { + const originalSlots = this._pureFunctionSlots; + this._pureFunctionSlots += numSlots; + return originalSlots; + } + + private allocateBindingSlots(value: AST) { + this._bindingSlots += value instanceof Interpolation ? value.expressions.length : 1; + } + private convertPropertyBinding(implicit: o.Expression, value: AST, skipBindFn?: boolean): o.Expression { - if (!skipBindFn) this._bindingSlots++; - const interpolationFn = value instanceof Interpolation ? interpolate : () => error('Unexpected interpolation'); @@ -875,6 +889,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } class ValueConverter extends AstMemoryEfficientTransformer { + private _pipeBindExprs: FunctionCall[] = []; + constructor( private constantPool: ConstantPool, private allocateSlot: () => number, private allocatePureFunctionSlots: (numSlots: number) => number, @@ -897,11 +913,21 @@ class ValueConverter extends AstMemoryEfficientTransformer { const convertedArgs: AST[] = isVarLength ? this.visitAll([new LiteralArray(pipe.span, args)]) : this.visitAll(args); - return new FunctionCall(pipe.span, target, [ + const pipeBindExpr = new FunctionCall(pipe.span, target, [ new LiteralPrimitive(pipe.span, slot), new LiteralPrimitive(pipe.span, pureFunctionSlot), ...convertedArgs, ]); + this._pipeBindExprs.push(pipeBindExpr); + return pipeBindExpr; + } + + updatePipeSlotOffsets(bindingSlots: number) { + this._pipeBindExprs.forEach((pipe: FunctionCall) => { + // update the slot offset arg (index 1) to account for binding slots + const slotOffset = pipe.args[1] as LiteralPrimitive; + (slotOffset.value as number) += bindingSlots; + }); } visitLiteralArray(array: LiteralArray, context: any): AST { diff --git a/packages/core/src/core_render3_private_export.ts b/packages/core/src/core_render3_private_export.ts index d5936a8dfc..897111f680 100644 --- a/packages/core/src/core_render3_private_export.ts +++ b/packages/core/src/core_render3_private_export.ts @@ -87,7 +87,6 @@ export { elementProperty as ɵelementProperty, projectionDef as ɵprojectionDef, reference as ɵreference, - reserveSlots as ɵreserveSlots, elementAttribute as ɵelementAttribute, elementStyling as ɵelementStyling, elementStylingMap as ɵelementStylingMap, diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts index 3b268aa97e..54a40c4500 100644 --- a/packages/core/src/render3/index.ts +++ b/packages/core/src/render3/index.ts @@ -76,8 +76,6 @@ export { reference, - reserveSlots, - embeddedViewStart, embeddedViewEnd, detectChanges, diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index d2592fd725..5e746db8dd 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -2559,9 +2559,6 @@ export function bind(value: T): T|NO_CHANGE { return bindingUpdated(viewData[BINDING_INDEX]++, value) ? value : NO_CHANGE; } -// TODO(kara): Remove this when updating the compiler (cannot remove without breaking JIT test) -export function reserveSlots(numSlots: number) {} - /** * Create interpolation bindings with a variable number of expressions. * diff --git a/packages/core/src/render3/jit/environment.ts b/packages/core/src/render3/jit/environment.ts index 87b490bbb8..d826596faf 100644 --- a/packages/core/src/render3/jit/environment.ts +++ b/packages/core/src/render3/jit/environment.ts @@ -90,7 +90,6 @@ export const angularCoreEnv: {[name: string]: Function} = { 'ɵquery': r3.query, 'ɵqueryRefresh': r3.queryRefresh, 'ɵregisterContentQuery': r3.registerContentQuery, - 'ɵreserveSlots': r3.reserveSlots, 'ɵreference': r3.reference, 'ɵelementStyling': r3.elementStyling, 'ɵelementStylingMap': r3.elementStylingMap, diff --git a/packages/core/src/render3/pipe.ts b/packages/core/src/render3/pipe.ts index d956528ede..b8af03f03d 100644 --- a/packages/core/src/render3/pipe.ts +++ b/packages/core/src/render3/pipe.ts @@ -68,7 +68,7 @@ function getPipeDef(name: string, registry: PipeDefList | null): PipeDefInternal * the pipe only when an input to the pipe changes. * * @param index Pipe index where the pipe was stored on creation. - * @param slotOffset the offset in the reserved slot space {@link reserveSlots} + * @param slotOffset the offset in the reserved slot space * @param v1 1st argument to {@link PipeTransform#transform}. */ export function pipeBind1(index: number, slotOffset: number, v1: any): any { @@ -84,7 +84,7 @@ export function pipeBind1(index: number, slotOffset: number, v1: any): any { * the pipe only when an input to the pipe changes. * * @param index Pipe index where the pipe was stored on creation. - * @param slotOffset the offset in the reserved slot space {@link reserveSlots} + * @param slotOffset the offset in the reserved slot space * @param v1 1st argument to {@link PipeTransform#transform}. * @param v2 2nd argument to {@link PipeTransform#transform}. */ @@ -101,7 +101,7 @@ export function pipeBind2(index: number, slotOffset: number, v1: any, v2: any): * the pipe only when an input to the pipe changes. * * @param index Pipe index where the pipe was stored on creation. - * @param slotOffset the offset in the reserved slot space {@link reserveSlots} + * @param slotOffset the offset in the reserved slot space * @param v1 1st argument to {@link PipeTransform#transform}. * @param v2 2nd argument to {@link PipeTransform#transform}. * @param v3 4rd argument to {@link PipeTransform#transform}. @@ -120,7 +120,7 @@ export function pipeBind3(index: number, slotOffset: number, v1: any, v2: any, v * the pipe only when an input to the pipe changes. * * @param index Pipe index where the pipe was stored on creation. - * @param slotOffset the offset in the reserved slot space {@link reserveSlots} + * @param slotOffset the offset in the reserved slot space * @param v1 1st argument to {@link PipeTransform#transform}. * @param v2 2nd argument to {@link PipeTransform#transform}. * @param v3 3rd argument to {@link PipeTransform#transform}. @@ -141,7 +141,7 @@ export function pipeBind4( * the pipe only when an input to the pipe changes. * * @param index Pipe index where the pipe was stored on creation. - * @param slotOffset the offset in the reserved slot space {@link reserveSlots} + * @param slotOffset the offset in the reserved slot space * @param values Array of arguments to pass to {@link PipeTransform#transform} method. */ export function pipeBindV(index: number, slotOffset: number, values: any[]): any {