Skip to content

Commit

Permalink
fixup! refactor(compiler-cli): effective narrowing of signal reads in…
Browse files Browse the repository at this point in the history 
… templates WIP
  • Loading branch information
@JoostK
JoostK committed Apr 2, 2024
1 parent b081fd0 commit e6ea4b5
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 52 deletions.
44 changes: 42 additions & 2 deletions packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts
View file
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,20 +45,27 @@ const BINARY_OPS = new Map<string, ts.BinaryOperator>([
['??', ts.SyntaxKind.QuestionQuestionToken],
]);

export interface SignalCall {
identifier: ts.Identifier;
initialized: boolean;
}

/**
* 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),
identifySignalCall: (ast: PotentialSignalCall) => SignalCall | null,
config: TypeCheckingConfig): ts.Expression {
const translator = new AstTranslator(maybeResolve, config);
const translator = new AstTranslator(maybeResolve, identifySignalCall, config);
return translator.translate(ast);
}

class AstTranslator implements AstVisitor {
constructor(
private maybeResolve: (ast: AST) => (ts.Expression | null),
private identifySignalCall: (ast: PotentialSignalCall) => SignalCall | null,
private config: TypeCheckingConfig) {}

translate(ast: AST): ts.Expression {
Expand Down Expand Up @@ -324,6 +332,17 @@ 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 signalCall = this.identifySignalCall(ast);
if (signalCall !== null) {
if (signalCall.initialized) {
return signalCall.identifier;
}
signalAssignmentTarget = signalCall.identifier;
}
}

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

Expand Down Expand Up @@ -353,14 +372,35 @@ 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) {
const signalCall = this.identifySignalCall(ast);
if (signalCall !== null) {
if (signalCall.initialized) {
return signalCall.identifier;
}
signalAssignmentTarget = signalCall.identifier;
}
}

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
78 changes: 28 additions & 50 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {addExpressionIdentifier, ExpressionIdentifier, markIgnoreDiagnostics} fr
import {addParseSpanInfo, addTemplateId, wrapForDiagnostics, wrapForTypeChecker} from './diagnostics';
import {DomSchemaChecker} from './dom';
import {Environment} from './environment';
import {astToTypescript, NULL_AS_ANY} from './expression';
import {astToTypescript, NULL_AS_ANY, SignalCall} from './expression';
import {OutOfBandDiagnosticRecorder} from './oob';
import {computeSignalCallIdentity, PotentialSignalCall, SignalCallIdentity} from './signal_calls';
import {tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareLetVariable, tsDeclareVariable,} from './ts_util';
Expand Down Expand Up @@ -1631,11 +1631,6 @@ export class Context {
}
}

interface SignalCall {
identifier: ts.Identifier;
initialized: boolean;
}

/**
* Local scope within the type check block for a particular template.
*
Expand Down Expand Up @@ -1694,6 +1689,7 @@ class Scope {
*/
private varMap = new Map<TmplAstVariable, number|ts.Identifier>();

private signalCallIdentities = new Map<PotentialSignalCall, SignalCallIdentity>();
private signalCallMap = new Map<SignalCallIdentity, ts.Identifier>();

/**
Expand Down Expand Up @@ -1849,31 +1845,20 @@ class Scope {
}

tryResolveSignalCall(call: PotentialSignalCall): SignalCall|null {
if (call.args.length !== 0) {
const identity = this.determineSignalCallIdentity(call);
if (identity === null) {
return null;
}

const identity = computeSignalCallIdentity(call, receiverCall => {
const result = this.tryResolveSignalCall(receiverCall);
if (result === null) {
return null;
}
// FIXME: if `result.initialized` is false then no initialization would take place, since
// subsequent requests
// for the signal call will have a cache hit, causing it to assume that variable has been
// initialized.
return result.identifier.text;
});
if (this.signalCallMap.has(identity)) {
const identifier = this.signalCallMap.get(identity)!;
return {identifier, initialized: true};
const id = this.signalCallMap.get(identity)!;
return {identifier: id, initialized: true};
}

let parent = this.parent;
while (parent !== null) {
if (parent.signalCallMap.has(identity)) {
const identifier = parent.signalCallMap.get(identity)!;
return {identifier, initialized: true};
const id = parent.signalCallMap.get(identity)!;
return {identifier: id, initialized: true};
}
parent = parent.parent;
}
Expand All @@ -1884,6 +1869,22 @@ class Scope {
return {identifier: id, initialized: false};
}

private determineSignalCallIdentity(call: PotentialSignalCall): SignalCallIdentity|null {
if (call.args.length !== 0) {
return null;
}
if (this.signalCallIdentities.has(call)) {
return this.signalCallIdentities.get(call)!;
}

const identity = computeSignalCallIdentity(call, receiverCall => {
return this.determineSignalCallIdentity(receiverCall);
});

this.signalCallIdentities.set(call, identity);
return identity;
}

/**
* Add a statement to this scope.
*/
Expand Down Expand Up @@ -2321,7 +2322,9 @@ class TcbExpressionTranslator {
// `astToTypescript` actually does the conversion. A special resolver `tcbResolve` is passed
// which interprets specific expression nodes that interact with the `ImplicitReceiver`. These
// nodes actually refer to identifiers within the current scope.
return astToTypescript(ast, ast => this.resolve(ast), this.tcb.env.config);
return astToTypescript(
ast, ast => this.resolve(ast), call => this.scope.tryResolveSignalCall(call),
this.tcb.env.config);
}

/**
Expand Down Expand Up @@ -2430,11 +2433,6 @@ class TcbExpressionTranslator {
return result;
}

const signalCall = this.tryResolveSignalCall(ast);
if (signalCall !== null) {
return signalCall;
}

// Attempt to resolve a bound target for the method, and generate the method call if a
// target could be resolved. If no target is available, then the method is referencing the
// top-level component context, in which case `null` is returned to let the
Expand All @@ -2452,33 +2450,13 @@ class TcbExpressionTranslator {
return node;
}

return this.tryResolveSignalCall(ast);
return null;
} else {
// This AST isn't special after all.
return null;
}
}

private tryResolveSignalCall(ast: PotentialSignalCall): ts.Expression|null {
const signalCall = this.scope.tryResolveSignalCall(ast);
if (signalCall === null) {
return null;
}

if (signalCall.initialized) {
return signalCall.identifier;
}

const receiver = this.translate(ast.receiver);
const expr = ts.factory.createCallChain(
receiver,
ast instanceof SafeCall ? ts.factory.createToken(ts.SyntaxKind.QuestionDotToken) :
undefined,
/* typeArguments */ undefined, /* argumentsArray */ undefined);
return ts.factory.createParenthesizedExpression(
ts.factory.createAssignment(signalCall.identifier, expr));
}

/**
* Attempts to resolve a bound target for a given expression, and translates it into the
* appropriate `ts.Expression` that represents the bound target. If no target is available,
Expand Down

0 comments on commit e6ea4b5

Please sign in to comment.