Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache instantiation expression types early enough to prevent reentrancy during printback #59931

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37400,9 +37400,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (exprType === silentNeverType || isErrorType(exprType) || !some(typeArguments)) {
return exprType;
}
const links = getNodeLinks(node);
if (!links.instantiationExpressionTypes) {
links.instantiationExpressionTypes = new Map();
}
if (links.instantiationExpressionTypes.has(exprType.id)) {
return links.instantiationExpressionTypes.get(exprType.id)!;
Comment on lines +37404 to +37408
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!links.instantiationExpressionTypes) {
links.instantiationExpressionTypes = new Map();
}
if (links.instantiationExpressionTypes.has(exprType.id)) {
return links.instantiationExpressionTypes.get(exprType.id)!;
links.instantiationExpressionTypes ??= new Map();
const cachedType = links.instantiationExpressionTypes.get(exprType.id);
if (cachedType) {
return cachedType;

}
let hasSomeApplicableSignature = false;
let nonApplicableType: Type | undefined;
const result = getInstantiatedType(exprType);
links.instantiationExpressionTypes.set(exprType.id, result);
const errorType = hasSomeApplicableSignature ? nonApplicableType : exprType;
if (errorType) {
diagnostics.add(createDiagnosticForNodeArray(getSourceFileOfNode(node), typeArguments, Diagnostics.Type_0_has_no_signatures_for_which_the_type_argument_list_is_applicable, typeToString(errorType)));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root cause of the stack overflow was basically that the typeToString on this line eventually recursively calls into getInstantiationExpressionType to make the version of the type that's a member of the mapped type being printed, which is identical, but re-triggers the error emit and then spins.

Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6238,6 +6238,7 @@ export interface NodeLinks {
potentialReflectCollisions?: Node[];
potentialUnusedRenamedBindingElementsInTypes?: BindingElement[];
externalHelpersModule?: Symbol; // Resolved symbol for the external helpers module
instantiationExpressionTypes?: Map<number, Type>; // Cache of instantiation expression types for the node
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason you cache by IDs instead of by Type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historic convention and habit, mostly. I can swap it if you'd like it.

}

/** @internal */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
instantiationExpressionErrorNoCrash.ts(15,38): error TS2344: Type 'typeof createCacheReducer<QR>' does not satisfy the constraint '(...args: any) => any'.
Type 'typeof createCacheReducer<QR>' provides no match for the signature '(...args: any): any'.
instantiationExpressionErrorNoCrash.ts(15,64): error TS2635: Type '<N extends string, QR>(queries: { [QK in keyof QR]: any; }) => (state?: { queries: QR; }) => { queries: QR; }' has no signatures for which the type argument list is applicable.


==== instantiationExpressionErrorNoCrash.ts (2 errors) ====
const createCacheReducer = <N extends string, QR>(
queries: Cache<N, QR>["queries"],
) => {
const queriesMap = {} as QR;

const initialState = {
queries: queriesMap,
};

return (state = initialState) => state;
};

export type Cache<N extends string, QR> = {
queries: {
[QK in keyof QR]: ReturnType<typeof createCacheReducer<QR>>;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2344: Type 'typeof createCacheReducer<QR>' does not satisfy the constraint '(...args: any) => any'.
!!! error TS2344: Type 'typeof createCacheReducer<QR>' provides no match for the signature '(...args: any): any'.
~~
!!! error TS2635: Type '<N extends string, QR>(queries: { [QK in keyof QR]: any; }) => (state?: { queries: QR; }) => { queries: QR; }' has no signatures for which the type argument list is applicable.
};
};
34 changes: 34 additions & 0 deletions tests/baselines/reference/instantiationExpressionErrorNoCrash.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//// [tests/cases/compiler/instantiationExpressionErrorNoCrash.ts] ////

//// [instantiationExpressionErrorNoCrash.ts]
const createCacheReducer = <N extends string, QR>(
queries: Cache<N, QR>["queries"],
) => {
const queriesMap = {} as QR;

const initialState = {
queries: queriesMap,
};

return (state = initialState) => state;
};

export type Cache<N extends string, QR> = {
queries: {
[QK in keyof QR]: ReturnType<typeof createCacheReducer<QR>>;
};
};

//// [instantiationExpressionErrorNoCrash.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var createCacheReducer = function (queries) {
var queriesMap = {};
var initialState = {
queries: queriesMap,
};
return function (state) {
if (state === void 0) { state = initialState; }
return state;
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//// [tests/cases/compiler/instantiationExpressionErrorNoCrash.ts] ////

=== instantiationExpressionErrorNoCrash.ts ===
const createCacheReducer = <N extends string, QR>(
>createCacheReducer : Symbol(createCacheReducer, Decl(instantiationExpressionErrorNoCrash.ts, 0, 5))
>N : Symbol(N, Decl(instantiationExpressionErrorNoCrash.ts, 0, 28))
>QR : Symbol(QR, Decl(instantiationExpressionErrorNoCrash.ts, 0, 45))

queries: Cache<N, QR>["queries"],
>queries : Symbol(queries, Decl(instantiationExpressionErrorNoCrash.ts, 0, 50))
>Cache : Symbol(Cache, Decl(instantiationExpressionErrorNoCrash.ts, 10, 2))
>N : Symbol(N, Decl(instantiationExpressionErrorNoCrash.ts, 0, 28))
>QR : Symbol(QR, Decl(instantiationExpressionErrorNoCrash.ts, 0, 45))

) => {
const queriesMap = {} as QR;
>queriesMap : Symbol(queriesMap, Decl(instantiationExpressionErrorNoCrash.ts, 3, 9))
>QR : Symbol(QR, Decl(instantiationExpressionErrorNoCrash.ts, 0, 45))

const initialState = {
>initialState : Symbol(initialState, Decl(instantiationExpressionErrorNoCrash.ts, 5, 9))

queries: queriesMap,
>queries : Symbol(queries, Decl(instantiationExpressionErrorNoCrash.ts, 5, 26))
>queriesMap : Symbol(queriesMap, Decl(instantiationExpressionErrorNoCrash.ts, 3, 9))

};

return (state = initialState) => state;
>state : Symbol(state, Decl(instantiationExpressionErrorNoCrash.ts, 9, 12))
>initialState : Symbol(initialState, Decl(instantiationExpressionErrorNoCrash.ts, 5, 9))
>state : Symbol(state, Decl(instantiationExpressionErrorNoCrash.ts, 9, 12))

};

export type Cache<N extends string, QR> = {
>Cache : Symbol(Cache, Decl(instantiationExpressionErrorNoCrash.ts, 10, 2))
>N : Symbol(N, Decl(instantiationExpressionErrorNoCrash.ts, 12, 18))
>QR : Symbol(QR, Decl(instantiationExpressionErrorNoCrash.ts, 12, 35))

queries: {
>queries : Symbol(queries, Decl(instantiationExpressionErrorNoCrash.ts, 12, 43))

[QK in keyof QR]: ReturnType<typeof createCacheReducer<QR>>;
>QK : Symbol(QK, Decl(instantiationExpressionErrorNoCrash.ts, 14, 9))
>QR : Symbol(QR, Decl(instantiationExpressionErrorNoCrash.ts, 12, 35))
>ReturnType : Symbol(ReturnType, Decl(lib.es5.d.ts, --, --))
>createCacheReducer : Symbol(createCacheReducer, Decl(instantiationExpressionErrorNoCrash.ts, 0, 5))
>QR : Symbol(QR, Decl(instantiationExpressionErrorNoCrash.ts, 12, 35))

};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//// [tests/cases/compiler/instantiationExpressionErrorNoCrash.ts] ////

=== instantiationExpressionErrorNoCrash.ts ===
const createCacheReducer = <N extends string, QR>(
>createCacheReducer : <N extends string, QR>(queries: Cache<N, QR>["queries"]) => (state?: { queries: QR; }) => { queries: QR; }
> : ^ ^^^^^^^^^ ^^ ^^ ^^ ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
><N extends string, QR>( queries: Cache<N, QR>["queries"],) => { const queriesMap = {} as QR; const initialState = { queries: queriesMap, }; return (state = initialState) => state;} : <N extends string, QR>(queries: Cache<N, QR>["queries"]) => (state?: { queries: QR; }) => { queries: QR; }
> : ^ ^^^^^^^^^ ^^ ^^ ^^ ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

queries: Cache<N, QR>["queries"],
>queries : { [QK in keyof QR]: any; }
> : ^^^ ^^^^^^^^^^^^^^^^^^^^^

) => {
const queriesMap = {} as QR;
>queriesMap : QR
> : ^^
>{} as QR : QR
> : ^^
>{} : {}
> : ^^

const initialState = {
>initialState : { queries: QR; }
> : ^^^^^^^^^^^^^^^^
>{ queries: queriesMap, } : { queries: QR; }
> : ^^^^^^^^^^^^^^^^

queries: queriesMap,
>queries : QR
> : ^^
>queriesMap : QR
> : ^^

};

return (state = initialState) => state;
>(state = initialState) => state : (state?: { queries: QR; }) => { queries: QR; }
> : ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>state : { queries: QR; }
> : ^^^^^^^^^^^^^^^^
>initialState : { queries: QR; }
> : ^^^^^^^^^^^^^^^^
>state : { queries: QR; }
> : ^^^^^^^^^^^^^^^^

};

export type Cache<N extends string, QR> = {
>Cache : Cache<N, QR>
> : ^^^^^^^^^^^^

queries: {
>queries : { [QK in keyof QR]: any; }
> : ^^^ ^^^^^^^^^^^^^^^^^^^^^

[QK in keyof QR]: ReturnType<typeof createCacheReducer<QR>>;
>createCacheReducer : <N_1 extends string, QR_1>(queries: Cache<N_1, QR_1>["queries"]) => (state?: { queries: QR_1; }) => { queries: QR_1; }
> : ^^^^^^^^^^^^^ ^^^^^^^^ ^^ ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

};
};
4 changes: 2 additions & 2 deletions tests/baselines/reference/instantiationExpressionErrors.types
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ const c1 = g<string> || ((x: string) => x);
>c1 : (x: string) => string
> : ^ ^^^^^^^^^^^^^^^^^^^
>g<string> || ((x: string) => x) : (x: string) => string
> : ^ ^^ ^^^^^^^^^^^
> : ^ ^^^^^^^^^^^^^^^^^^^
>g<string> : ((x: string) => string) | undefined
> : ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>g : (<T>(x: T) => T) | undefined
Expand All @@ -197,7 +197,7 @@ const c2 = g<string> ?? ((x: string) => x);
>c2 : (x: string) => string
> : ^ ^^^^^^^^^^^^^^^^^^^
>g<string> ?? ((x: string) => x) : (x: string) => string
> : ^ ^^ ^^^^^^^^^^^
> : ^ ^^^^^^^^^^^^^^^^^^^
>g<string> : ((x: string) => string) | undefined
> : ^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>g : (<T>(x: T) => T) | undefined
Expand Down
17 changes: 17 additions & 0 deletions tests/cases/compiler/instantiationExpressionErrorNoCrash.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const createCacheReducer = <N extends string, QR>(
queries: Cache<N, QR>["queries"],
) => {
const queriesMap = {} as QR;

const initialState = {
queries: queriesMap,
};

return (state = initialState) => state;
};

export type Cache<N extends string, QR> = {
queries: {
[QK in keyof QR]: ReturnType<typeof createCacheReducer<QR>>;
};
};