Skip to content

Commit

Permalink
refactor(compiler-cli): effective narrowing of signal reads in templates
Browse files Browse the repository at this point in the history
This commit is an experiment to see how the template type-check block
can be modified to support effective narrowing of signal reads.

The prototype exposes one problem in particular: calling a type guard method
no longer narrows according to the type-guard, as the method call is indirected
though an additional variable.

Other problems with this approach is that diagnostics will not be reported for
all usages, given that identical call expressions are deduplicated into a single
call expression.

The implementation of this prototype is not as efficient as it could be.
In particular, computing the identifier of a call expression does not leverage
a potentially existing identifier for another call expression in its LHS.
  • Loading branch information
JoostK committed Apr 14, 2024
1 parent 571a61b commit c5cf68a
Show file tree
Hide file tree
Showing 10 changed files with 469 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const FS_NATIVE = 'Native';
const FS_OS_X = 'OS/X';
const FS_UNIX = 'Unix';
const FS_WINDOWS = 'Windows';
const FS_ALL = [FS_OS_X, FS_WINDOWS, FS_UNIX, FS_NATIVE];
const FS_ALL = [FS_NATIVE];

function runInEachFileSystemFn(callback: (os: string) => void) {
FS_ALL.forEach(os => runInFileSystem(os, callback, false));
Expand Down
4 changes: 3 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,9 @@ export class TypeCheckContextImpl implements TypeCheckContext {
}
result.appendLeft(update.pos, update.text);
}
return result.toString();
const code = result.toString();
console.log(code);
return code;
}

finalize(): Map<AbsoluteFsPath, FileUpdate> {
Expand Down
61 changes: 57 additions & 4 deletions packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import ts from 'typescript';
import {TypeCheckingConfig} from '../api';

import {addParseSpanInfo, wrapForDiagnostics, wrapForTypeChecker} from './diagnostics';
import {PotentialSignalCall} from './signal_calls';
import {tsCastToAny, tsNumericExpression} from './ts_util';

export const NULL_AS_ANY = ts.factory.createAsExpression(
Expand Down Expand Up @@ -44,21 +45,40 @@ const BINARY_OPS = new Map<string, ts.BinaryOperator>([
['??', ts.SyntaxKind.QuestionQuestionToken],
]);

export interface SignalCall {
identifier: ts.Identifier;
}


export interface SignalCallOutlining {
allocateTarget(call: PotentialSignalCall): ts.Identifier|null;

resolveTarget(call: PotentialSignalCall): ts.Identifier|null;
}


export enum EmitMode {
Regular,
Narrowing,
}

/**
* Convert an `AST` to TypeScript code directly, without going through an intermediate `Expression`
* AST.
*/
export function astToTypescript(
ast: AST, maybeResolve: (ast: AST) => (ts.Expression | null),
config: TypeCheckingConfig): ts.Expression {
const translator = new AstTranslator(maybeResolve, config);
signalCallOutlining: SignalCallOutlining, config: TypeCheckingConfig,
mode: EmitMode): ts.Expression {
const translator = new AstTranslator(maybeResolve, signalCallOutlining, config, mode);
return translator.translate(ast);
}

class AstTranslator implements AstVisitor {
constructor(
private maybeResolve: (ast: AST) => (ts.Expression | null),
private config: TypeCheckingConfig) {}
private signalCallOutlining: SignalCallOutlining, private config: TypeCheckingConfig,
private mode: EmitMode) {}

translate(ast: AST): ts.Expression {
// Skip over an `ASTWithSource` as its `visit` method calls directly into its ast's `visit`,
Expand Down Expand Up @@ -324,6 +344,19 @@ class AstTranslator implements AstVisitor {
visitCall(ast: Call): ts.Expression {
const args = ast.args.map(expr => this.translate(expr));

let signalAssignmentTarget: ts.Identifier|null = null;
if (args.length === 0) {
const target = this.signalCallOutlining.resolveTarget(ast);
if (target !== null) {
// TODO: using the outlined result here means that diagnostics won't be repeated
return target;
}

if (this.mode === EmitMode.Narrowing) {
signalAssignmentTarget = this.signalCallOutlining.allocateTarget(ast);
}
}

let expr: ts.Expression;
const receiver = ast.receiver;

Expand Down Expand Up @@ -353,14 +386,34 @@ class AstTranslator implements AstVisitor {
node = ts.factory.createCallExpression(expr, undefined, args);
}

if (signalAssignmentTarget !== null) {
node = ts.factory.createAssignment(signalAssignmentTarget, node);
}

addParseSpanInfo(node, ast.sourceSpan);
return node;
}

visitSafeCall(ast: SafeCall): ts.Expression {
const args = ast.args.map(expr => this.translate(expr));

let signalAssignmentTarget: ts.Identifier|null = null;
if (args.length === 0 && this.mode === EmitMode.Narrowing) {
const target = this.signalCallOutlining.resolveTarget(ast);
if (target !== null) {
// TODO: using the outlined result here means that diagnostics won't be repeated
return target;
}
signalAssignmentTarget = this.signalCallOutlining.allocateTarget(ast);
}

const expr = wrapForDiagnostics(this.translate(ast.receiver));
const node = this.convertToSafeCall(ast, expr, args);
let node = this.convertToSafeCall(ast, expr, args);

if (signalAssignmentTarget !== null) {
node = ts.factory.createAssignment(signalAssignmentTarget, node);
}

addParseSpanInfo(node, ast.sourceSpan);
return node;
}
Expand Down
182 changes: 182 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/src/signal_calls.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Call, Chain, Conditional, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, SafeCall, SafeKeyedRead, SafePropertyRead, ThisReceiver, Unary,} from '@angular/compiler';

export type SignalCallIdentity = string&{__brand: 'SignalCallIdentity'};

export type PotentialSignalCall = Call|SafeCall;

export function computeSignalCallIdentity(
call: PotentialSignalCall,
recurse: (receiverCall: PotentialSignalCall) => string | null,
identifyImplicitRead: (expr: AST) => string | null,
): SignalCallIdentity {
const identifier =
call.receiver.visit(new SignalCallIdentification(recurse, identifyImplicitRead));
return identifier as SignalCallIdentity;
}

class SignalCallIdentification implements AstVisitor {
constructor(
private recurse: (receiverCall: PotentialSignalCall) => string | null,
private identifyImplicitRead: (expr: AST) => string | null,
) {}

visitUnary(ast: Unary): string|null {
const expr = this.forAst(ast.expr);
if (expr === null) {
return null;
}
return `${ast.operator}${expr}`;
}

visitBinary(ast: Binary): string|null {
const left = this.forAst(ast.left);
const right = this.forAst(ast.right);
if (left === null || right === null) {
return null;
}
return `${left}${ast.operation}${right}`;
}

visitChain(ast: Chain): string|null {
return null;
}

visitConditional(ast: Conditional): string|null {
return null;
}

visitThisReceiver(ast: ThisReceiver): string|null {
return 'this';
}

visitImplicitReceiver(ast: ImplicitReceiver): string|null {
return 'this';
}

visitInterpolation(ast: Interpolation): string|null {
return null;
}

visitKeyedRead(ast: KeyedRead): string|null {
const receiver = this.forAst(ast.receiver);
const key = this.forAst(ast.key);
if (receiver === null || key === null) {
return null;
}
return `${receiver}[${key}]`;
}

visitKeyedWrite(ast: KeyedWrite): string|null {
return null;
}

visitLiteralArray(ast: LiteralArray): string|null {
return null;
}

visitLiteralMap(ast: LiteralMap): string|null {
return null;
}

visitLiteralPrimitive(ast: LiteralPrimitive): string|null {
return `${ast.value}`;
}

visitPipe(ast: BindingPipe): string|null {
return null;
}

visitPrefixNot(ast: PrefixNot): string|null {
const expression = this.forAst(ast.expression);
if (expression === null) {
return expression;
}
return `!${expression}`;
}

visitNonNullAssert(ast: NonNullAssert): string|null {
return this.forAst(ast.expression);
}

visitPropertyRead(ast: PropertyRead): string|null {
const receiver = this.identifyReceiver(ast);
if (receiver === null) {
return null;
}
return `${receiver}.${ast.name}`;
}

visitPropertyWrite(ast: PropertyWrite): string|null {
return null;
}

visitSafePropertyRead(ast: SafePropertyRead): string|null {
const receiver = this.identifyReceiver(ast);
if (receiver === null) {
return null;
}
return `${receiver}?.${ast.name}`;
}

visitSafeKeyedRead(ast: SafeKeyedRead): string|null {
const receiver = this.forAst(ast.receiver)
if (receiver === null) {
return null;
}
return `${receiver}?.[${this.forAst(ast.key)}]`;
}

visitCall(ast: Call): string|null {
if (ast.args.length > 0) {
return null;
}
const receiver = this.forAst(ast.receiver);
if (receiver === null) {
return null;
}
return `${receiver}()`;
}

visitSafeCall(ast: SafeCall): string|null {
if (ast.args.length > 0) {
return null;
}
const receiver = this.forAst(ast.receiver);
if (receiver === null) {
return null;
}
return `${receiver}?.()`;
}

visitASTWithSource(ast: ASTWithSource): string|null {
return this.forAst(ast.ast);
}

private identifyReceiver(ast: PropertyRead|SafePropertyRead): string|null {
if (ast.receiver instanceof ImplicitReceiver && !(ast.receiver instanceof ThisReceiver)) {
const implicitIdentity = this.identifyImplicitRead(ast);
if (implicitIdentity !== null) {
return implicitIdentity;
}
}
return this.forAst(ast.receiver);
}

private forAst(ast: AST): string|null {
if (ast instanceof Call || ast instanceof SafeCall) {
const result = this.recurse(ast);
if (result !== null) {
return ${result}`;
}
}
return ast.visit(this);
}
}
10 changes: 7 additions & 3 deletions packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,18 @@ export function tsDeclareVariable(id: ts.Identifier, type: ts.TypeNode): ts.Vari
const initializer: ts.Expression = ts.factory.createAsExpression(
ts.factory.createNonNullExpression(ts.factory.createNull()), type);

return tsCreateVariable(id, initializer);
}

export function tsDeclareLetVariable(id: ts.Identifier, type?: ts.TypeNode): ts.VariableStatement {
const decl = ts.factory.createVariableDeclaration(
/* name */ id,
/* exclamationToken */ undefined,
/* type */ undefined,
/* initializer */ initializer);
/* type */ type,
/* initializer */ undefined);
return ts.factory.createVariableStatement(
/* modifiers */ undefined,
/* declarationList */[decl]);
/* declarationList */ ts.factory.createVariableDeclarationList([decl], ts.NodeFlags.Let));
}

/**
Expand Down

0 comments on commit c5cf68a

Please sign in to comment.