Skip to content

Commit 8151dd3

Browse files
committed
fix(compiler): do not use constants for colocated shallow references
Currently the TS generated output (not JIT), breaks at runtime because we emit all variables using the `Final` modifier. i.e. constants. This breaks shallow references when we run our acceptance signal tests using AOT.
1 parent 51ce1d6 commit 8151dd3

File tree

5 files changed

+25
-8
lines changed

5 files changed

+25
-8
lines changed

packages/compiler/src/template/pipeline/ir/src/ops/shared.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,25 @@ export interface VariableOp<OpT extends Op<OpT>> extends Op<OpT> {
6868
* Expression representing the value of the variable.
6969
*/
7070
initializer: o.Expression;
71+
72+
/**
73+
* Whether the variable created is a constant.
74+
*/
75+
isConstant: boolean;
7176
}
7277

7378
/**
7479
* Create a `VariableOp`.
7580
*/
7681
export function createVariableOp<OpT extends Op<OpT>>(
77-
xref: XrefId, variable: SemanticVariable, initializer: o.Expression): VariableOp<OpT> {
82+
xref: XrefId, variable: SemanticVariable, initializer: o.Expression,
83+
isConstant: boolean): VariableOp<OpT> {
7884
return {
7985
kind: OpKind.Variable,
8086
xref,
8187
variable,
8288
initializer,
89+
isConstant,
8390
...NEW_OP,
8491
};
8592
}

packages/compiler/src/template/pipeline/src/phases/creation_var_colocation.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ function processUnit(unit: CompilationUnit): void {
7979
// initializer to `undefined`, and set the variable to its value after its declaration.
8080
const initializer = declOp.initializer;
8181
declOp.initializer = o.literal(undefined);
82+
declOp.isConstant = false;
83+
8284
const readVar = new ir.ReadVariableExpr(declOp.xref);
8385
// TODO: variable naming should run after this and take care of this for us.
8486
readVar.name = declOp.variable.name;

packages/compiler/src/template/pipeline/src/phases/generate_variables.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,8 @@ function generateVariablesInScopeForView<OpT extends ir.Op<OpT>>(
179179
// view with a `nextContext` expression. This context switching operation itself declares a
180180
// variable, because the context of the view may be referenced directly.
181181
newOps.push(ir.createVariableOp(
182-
view.job.allocateXrefId(), scope.viewContextVariable, new ir.NextContextExpr()));
182+
view.job.allocateXrefId(), scope.viewContextVariable, new ir.NextContextExpr(),
183+
/* isConstant */ true));
183184
}
184185

185186
// Add variables for all context variables available in this scope's view.
@@ -189,13 +190,15 @@ function generateVariablesInScopeForView<OpT extends ir.Op<OpT>>(
189190
const variable = value === ir.CTX_REF ? context : new o.ReadPropExpr(context, value);
190191
// Add the variable declaration.
191192
newOps.push(ir.createVariableOp(
192-
view.job.allocateXrefId(), scope.contextVariables.get(name)!, variable));
193+
view.job.allocateXrefId(), scope.contextVariables.get(name)!, variable,
194+
/* isConstant */ true));
193195
}
194196

195197
// Add variables for all local references declared for elements in this scope.
196198
for (const ref of scope.references) {
197199
newOps.push(ir.createVariableOp(
198-
view.job.allocateXrefId(), ref.variable, new ir.ReferenceExpr(ref.targetId, ref.offset)));
200+
view.job.allocateXrefId(), ref.variable, new ir.ReferenceExpr(ref.targetId, ref.offset),
201+
/* isConstant */ true));
199202
}
200203

201204
if (scope.parent !== null) {

packages/compiler/src/template/pipeline/src/phases/reify.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,17 @@ function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList<ir.CreateOp
122122
if (op.variable.name === null) {
123123
throw new Error(`AssertionError: unnamed variable ${op.xref}`);
124124
}
125+
// Optimization for variable co-location. If we know the initializer is undefined,
126+
// we don't need to set an explicit one to avoid e.g. a `let b = undefined`.
125127
let initializer: o.Expression|undefined = op.initializer;
126128
if (initializer instanceof o.LiteralExpr && initializer.value === undefined) {
127129
initializer = undefined;
128130
}
129131
ir.OpList.replace<ir.CreateOp>(
130132
op,
131133
ir.createStatementOp(new o.DeclareVarStmt(
132-
op.variable.name, initializer, undefined, o.StmtModifier.Final)));
134+
op.variable.name, initializer, undefined,
135+
op.isConstant ? o.StmtModifier.Final : undefined)));
133136
break;
134137
case ir.OpKind.Namespace:
135138
switch (op.active) {
@@ -280,7 +283,8 @@ function reifyUpdateOperations(_unit: CompilationUnit, ops: ir.OpList<ir.UpdateO
280283
ir.OpList.replace<ir.UpdateOp>(
281284
op,
282285
ir.createStatementOp(new o.DeclareVarStmt(
283-
op.variable.name, op.initializer, undefined, o.StmtModifier.Final)));
286+
op.variable.name, op.initializer, undefined,
287+
op.isConstant ? o.StmtModifier.Final : undefined)));
284288
break;
285289
case ir.OpKind.Conditional:
286290
if (op.processed === null) {

packages/compiler/src/template/pipeline/src/phases/save_restore_view.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ export function phaseSaveRestoreView(job: ComponentCompilationJob): void {
1919
name: null,
2020
view: view.xref,
2121
},
22-
new ir.GetCurrentViewExpr()),
22+
new ir.GetCurrentViewExpr(), /* isConstant */ true),
23+
2324
]);
2425

2526
for (const op of view.create) {
@@ -56,7 +57,7 @@ function addSaveRestoreViewOperationToListener(unit: ViewCompilationUnit, op: ir
5657
name: null,
5758
view: unit.xref,
5859
},
59-
new ir.RestoreViewExpr(unit.xref)),
60+
new ir.RestoreViewExpr(unit.xref), /* isConstant */ true),
6061
]);
6162

6263
// The "restore view" operation in listeners requires a call to `resetView` to reset the

0 commit comments

Comments
 (0)