fix(compiler): update compiler to generate new slot allocations (angular#25607)

PR Close angular#25607

Author: kara

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();
             }

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) {

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};

packages/compiler/src/render3/view/template.ts

@@ -110,7 +110,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, 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<void>, 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<void>, 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<void>, 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<void>, 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<void>, 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<void>, 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<void>, 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 {

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,

packages/core/src/render3/index.ts

@@ -76,8 +76,6 @@ export {
 
   reference,
 
-  reserveSlots,
-
   embeddedViewStart,
   embeddedViewEnd,
   detectChanges,

packages/core/src/render3/instructions.ts

@@ -2559,9 +2559,6 @@ export function bind<T>(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.
  *

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,

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 {