diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/nullish_coalescing/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/nullish_coalescing/TEST_CASES.json index 23a03209de6..3bfadccf874 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/nullish_coalescing/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/nullish_coalescing/TEST_CASES.json @@ -3,9 +3,7 @@ "cases": [ { "description": "should handle nullish coalescing inside interpolations", - "inputFiles": [ - "nullish_coalescing_interpolation.ts" - ], + "inputFiles": ["nullish_coalescing_interpolation.ts"], "expectations": [ { "files": [ @@ -16,14 +14,11 @@ ], "failureMessage": "Incorrect template" } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should handle nullish coalescing inside property bindings", - "inputFiles": [ - "nullish_coalescing_property.ts" - ], + "inputFiles": ["nullish_coalescing_property.ts"], "expectations": [ { "files": [ @@ -34,14 +29,11 @@ ], "failureMessage": "Incorrect template" } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should handle nullish coalescing inside host bindings", - "inputFiles": [ - "nullish_coalescing_host.ts" - ], + "inputFiles": ["nullish_coalescing_host.ts"], "expectations": [ { "files": [ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/TEST_CASES.json index 9b37a8e75cd..9d89d267308 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/TEST_CASES.json @@ -3,9 +3,7 @@ "cases": [ { "description": "should handle safe keyed reads inside templates", - "inputFiles": [ - "safe_keyed_read.ts" - ], + "inputFiles": ["safe_keyed_read.ts"], "expectations": [ { "files": [ @@ -20,9 +18,7 @@ }, { "description": "should handle deep safe property reads inside templates", - "inputFiles": [ - "safe_access_deep.ts" - ], + "inputFiles": ["safe_access_deep.ts"], "expectations": [ { "files": [ @@ -30,21 +26,19 @@ "expected": "safe_access_deep_template.js", "generated": "safe_access_deep.js" } - ], - "failureMessage": "Incorrect template" + ] } ] }, { "description": "should handle property reads requiring temporary variables", - "inputFiles": [ - "safe_access_temporaries.ts" - ], + "inputFiles": ["safe_access_temporaries.ts"], "expectations": [ { "files": [ { "expected": "safe_access_temporaries_template.js", + "templatePipelineExpected": "safe_access_temporaries_template.pipeline.js", "generated": "safe_access_temporaries.js" } ], @@ -54,9 +48,7 @@ }, { "description": "should handle safe method calls inside templates", - "inputFiles": [ - "safe_method_call.ts" - ], + "inputFiles": ["safe_method_call.ts"], "expectations": [ { "files": [ @@ -67,14 +59,11 @@ ], "failureMessage": "Incorrect template" } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should handle safe calls inside templates", - "inputFiles": [ - "safe_call.ts" - ], + "inputFiles": ["safe_call.ts"], "expectations": [ { "files": [ @@ -90,9 +79,7 @@ }, { "description": "should handle non-null assertions after a safe access", - "inputFiles": [ - "safe_access_non_null.ts" - ], + "inputFiles": ["safe_access_non_null.ts"], "expectations": [ { "files": [ @@ -107,4 +94,4 @@ "skipForTemplatePipeline": true } ] -} \ No newline at end of file +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_access_temporaries_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_access_temporaries_template.pipeline.js new file mode 100644 index 00000000000..bc0509c3737 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_access_temporaries_template.pipeline.js @@ -0,0 +1,18 @@ +} if (rf & 2) { + let $tmp_0_0$; + let $tmp_1_0$; + let $tmp_2_0$; + let $tmp_2_1$; + let $tmp_3_0$; + let $tmp_3_1$; + let $tmp_3_2$; + let $tmp_3_3$; + i0.ɵɵadvance(1); + i0.ɵɵtextInterpolate1("Safe Propery with Calls: ", ($tmp_0_0$ = ctx.p()) == null ? null : ($tmp_0_0$ = $tmp_0_0$.a()) == null ? null : ($tmp_0_0$ = $tmp_0_0$.b()) == null ? null : ($tmp_0_0$ = $tmp_0_0$.c()) == null ? null : $tmp_0_0$.d(), ""); + i0.ɵɵadvance(2); + i0.ɵɵtextInterpolate1("Safe and Unsafe Property with Calls: ", ctx.p == null ? null : ($tmp_1_0$ = ctx.p.a()) == null ? null : ($tmp_1_0$ = $tmp_1_0$.b().c().d()) == null ? null : ($tmp_1_0$ = $tmp_1_0$.e()) == null ? null : $tmp_1_0$.f == null ? null : $tmp_1_0$.f.g.h == null ? null : ($tmp_1_0$ = $tmp_1_0$.f.g.h.i()) == null ? null : ($tmp_1_0$ = $tmp_1_0$.j()) == null ? null : $tmp_1_0$.k().l, ""); + i0.ɵɵadvance(2); + i0.ɵɵtextInterpolate1("Nested Safe with Calls: ", ($tmp_2_0$ = ctx.f1()) == null ? null : $tmp_2_0$[($tmp_2_1$ = ctx.f2()) == null ? null : $tmp_2_1$.a] == null ? null : $tmp_2_0$[($tmp_2_1$ = $tmp_2_1$) == null ? null : $tmp_2_1$.a].b, ""); + i0.ɵɵadvance(2); + i0.ɵɵtextInterpolate1("Deep Nested Safe with Calls: ", ($tmp_3_0$ = ctx.f1()) == null ? null : $tmp_3_0$[($tmp_3_1$ = ctx.f2()) == null ? null : ($tmp_3_2$ = $tmp_3_1$.f3()) == null ? null : $tmp_3_2$[($tmp_3_3$ = ctx.f4()) == null ? null : $tmp_3_3$.f5()]] == null ? null : $tmp_3_0$[($tmp_3_1$ = $tmp_3_1$) == null ? null : ($tmp_3_2$ = $tmp_3_2$) == null ? null : $tmp_3_2$[($tmp_3_3$ = $tmp_3_3$) == null ? null : $tmp_3_3$.f5()]].f6(), ""); +} diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index 92df21bc619..a8c7b4b578b 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -996,7 +996,7 @@ export class ReadKeyExpr extends Expression { } override clone(): ReadKeyExpr { - return new ReadKeyExpr(this.receiver, this.index.clone(), this.type, this.sourceSpan); + return new ReadKeyExpr(this.receiver.clone(), this.index.clone(), this.type, this.sourceSpan); } } diff --git a/packages/compiler/src/template/pipeline/ir/src/expression.ts b/packages/compiler/src/template/pipeline/ir/src/expression.ts index 025c6102a1e..019ceec0bec 100644 --- a/packages/compiler/src/template/pipeline/ir/src/expression.ts +++ b/packages/compiler/src/template/pipeline/ir/src/expression.ts @@ -675,7 +675,7 @@ export class AssignTemporaryExpr extends ExpressionBase { } override clone(): AssignTemporaryExpr { - const a = new AssignTemporaryExpr(this.expr, this.xref); + const a = new AssignTemporaryExpr(this.expr.clone(), this.xref); a.name = this.name; return a; } diff --git a/packages/compiler/src/template/pipeline/src/phases/temporary_variables.ts b/packages/compiler/src/template/pipeline/src/phases/temporary_variables.ts index c95a58a04a8..5fcef42358c 100644 --- a/packages/compiler/src/template/pipeline/src/phases/temporary_variables.ts +++ b/packages/compiler/src/template/pipeline/src/phases/temporary_variables.ts @@ -24,37 +24,56 @@ export function phaseTemporaryVariables(cpl: CompilationJob): void { let opCount = 0; let generatedStatements: Array> = []; for (const op of unit.ops()) { + // Identify the final time each temp var is read. + const finalReads = new Map(); + ir.visitExpressionsInOp(op, expr => { + if (expr instanceof ir.ReadTemporaryExpr) { + finalReads.set(expr.xref, expr); + } + }); + + // Name the temp vars, accounting for the fact that a name can be reused after it has been + // read for the final time. let count = 0; - let xrefs = new Set(); - let defs = new Map(); - + const assigned = new Set(); + const released = new Set(); + const defs = new Map(); ir.visitExpressionsInOp(op, expr => { - if (expr instanceof ir.ReadTemporaryExpr || expr instanceof ir.AssignTemporaryExpr) { - xrefs.add(expr.xref); - } - }); - - for (const xref of xrefs) { - // TODO: Exactly replicate the naming scheme used by `TemplateDefinitionBuilder`. It seems - // to rely on an expression index instead of an op index. - defs.set(xref, `tmp_${opCount}_${count++}`); - } - - ir.visitExpressionsInOp(op, expr => { - if (expr instanceof ir.ReadTemporaryExpr || expr instanceof ir.AssignTemporaryExpr) { - const name = defs.get(expr.xref); - if (name === undefined) { - throw new Error('Found xref with unassigned name'); + if (expr instanceof ir.AssignTemporaryExpr) { + if (!assigned.has(expr.xref)) { + assigned.add(expr.xref); + // TODO: Exactly replicate the naming scheme used by `TemplateDefinitionBuilder`. + // It seems to rely on an expression index instead of an op index. + defs.set(expr.xref, `tmp_${opCount}_${count++}`); } - expr.name = name; + assignName(defs, expr); + } else if (expr instanceof ir.ReadTemporaryExpr) { + if (finalReads.get(expr.xref) === expr) { + released.add(expr.xref); + count--; + } + assignName(defs, expr); } }); + // Add declarations for the temp vars. generatedStatements.push( - ...Array.from(defs.values()) + ...Array.from(new Set(defs.values())) .map(name => ir.createStatementOp(new o.DeclareVarStmt(name)))); opCount++; } unit.update.prepend(generatedStatements); } } + +/** + * Assigns a name to the temporary variable in the given temporary variable expression. + */ +function assignName( + names: Map, expr: ir.AssignTemporaryExpr|ir.ReadTemporaryExpr) { + const name = names.get(expr.xref); + if (name === undefined) { + throw new Error(`Found xref with unassigned name: ${expr.xref}`); + } + expr.name = name; +}