Skip to content

Commit 3c42760

Browse files
sandersnsandersn
authored
Cache JS inferred class type symbol (#33010)
* Cache JS inferred class type symbol Note that many sources merge into a single target, so the *source* [links] is the one that caches the merged target. The reason this is a problem is not that many sources merge into a single target, but that both getTypeOfSymbol and getDeclaredTypeOfSymbol end up calling mergeJSSymbols with the same [source,target] pair. The merge should not happen twice. * Remove more verbose debug assertion message * Fix isJSConstructor check + update baselines * inferClassSymbol cache now track multiple targets
1 parent 016884d commit 3c42760

10 files changed

+230
-47
lines changed

Diff for: ‎src/compiler/checker.ts

+17-12
Original file line numberDiff line numberDiff line change
@@ -22879,24 +22879,29 @@ namespace ts {
2287922879

2288022880
// If the symbol of the node has members, treat it like a constructor.
2288122881
const symbol = getSymbolOfNode(func);
22882-
return !!symbol && symbol.members !== undefined;
22882+
return !!symbol && hasEntries(symbol.members);
2288322883
}
2288422884
return false;
2288522885
}
2288622886

2288722887
function mergeJSSymbols(target: Symbol, source: Symbol | undefined) {
2288822888
if (source && (hasEntries(source.exports) || hasEntries(source.members))) {
22889-
target = cloneSymbol(target);
22890-
if (hasEntries(source.exports)) {
22891-
target.exports = target.exports || createSymbolTable();
22892-
mergeSymbolTable(target.exports, source.exports);
22893-
}
22894-
if (hasEntries(source.members)) {
22895-
target.members = target.members || createSymbolTable();
22896-
mergeSymbolTable(target.members, source.members);
22897-
}
22898-
target.flags |= source.flags & SymbolFlags.Class;
22899-
return target as TransientSymbol;
22889+
const links = getSymbolLinks(source);
22890+
if (!links.inferredClassSymbol || !links.inferredClassSymbol.has("" + getSymbolId(target))) {
22891+
const inferred = isTransientSymbol(target) ? target : cloneSymbol(target) as TransientSymbol;
22892+
inferred.exports = inferred.exports || createSymbolTable();
22893+
inferred.members = inferred.members || createSymbolTable();
22894+
inferred.flags |= source.flags & SymbolFlags.Class;
22895+
if (hasEntries(source.exports)) {
22896+
mergeSymbolTable(inferred.exports, source.exports);
22897+
}
22898+
if (hasEntries(source.members)) {
22899+
mergeSymbolTable(inferred.members, source.members);
22900+
}
22901+
(links.inferredClassSymbol || (links.inferredClassSymbol = createMap<TransientSymbol>())).set("" + getSymbolId(inferred), inferred);
22902+
return inferred;
22903+
}
22904+
return links.inferredClassSymbol.get("" + getSymbolId(target));
2290022905
}
2290122906
}
2290222907

Diff for: ‎src/compiler/types.ts

+24-23
Original file line numberDiff line numberDiff line change
@@ -3770,30 +3770,31 @@ namespace ts {
37703770

37713771
/* @internal */
37723772
export interface SymbolLinks {
3773-
immediateTarget?: Symbol; // Immediate target of an alias. May be another alias. Do not access directly, use `checker.getImmediateAliasedSymbol` instead.
3774-
target?: Symbol; // Resolved (non-alias) target of an alias
3775-
type?: Type; // Type of value symbol
3776-
uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol
3777-
declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter
3778-
resolvedJSDocType?: Type; // Resolved type of a JSDoc type reference
3779-
typeParameters?: TypeParameter[]; // Type parameters of type alias (undefined if non-generic)
3780-
outerTypeParameters?: TypeParameter[]; // Outer type parameters of anonymous object type
3781-
instantiations?: Map<Type>; // Instantiations of generic type alias (undefined if non-generic)
3782-
mapper?: TypeMapper; // Type mapper for instantiation alias
3783-
referenced?: boolean; // True if alias symbol has been referenced as a value
3784-
containingType?: UnionOrIntersectionType; // Containing union or intersection type for synthetic property
3785-
leftSpread?: Symbol; // Left source for synthetic spread property
3786-
rightSpread?: Symbol; // Right source for synthetic spread property
3787-
syntheticOrigin?: Symbol; // For a property on a mapped or spread type, points back to the original property
3788-
isDiscriminantProperty?: boolean; // True if discriminant synthetic property
3789-
resolvedExports?: SymbolTable; // Resolved exports of module or combined early- and late-bound static members of a class.
3790-
resolvedMembers?: SymbolTable; // Combined early- and late-bound members of a symbol
3791-
exportsChecked?: boolean; // True if exports of external module have been checked
3792-
typeParametersChecked?: boolean; // True if type parameters of merged class and interface declarations have been checked.
3773+
immediateTarget?: Symbol; // Immediate target of an alias. May be another alias. Do not access directly, use `checker.getImmediateAliasedSymbol` instead.
3774+
target?: Symbol; // Resolved (non-alias) target of an alias
3775+
type?: Type; // Type of value symbol
3776+
uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol
3777+
declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter
3778+
resolvedJSDocType?: Type; // Resolved type of a JSDoc type reference
3779+
typeParameters?: TypeParameter[]; // Type parameters of type alias (undefined if non-generic)
3780+
outerTypeParameters?: TypeParameter[]; // Outer type parameters of anonymous object type
3781+
instantiations?: Map<Type>; // Instantiations of generic type alias (undefined if non-generic)
3782+
inferredClassSymbol?: Map<TransientSymbol>; // Symbol of an inferred ES5 constructor function
3783+
mapper?: TypeMapper; // Type mapper for instantiation alias
3784+
referenced?: boolean; // True if alias symbol has been referenced as a value
3785+
containingType?: UnionOrIntersectionType; // Containing union or intersection type for synthetic property
3786+
leftSpread?: Symbol; // Left source for synthetic spread property
3787+
rightSpread?: Symbol; // Right source for synthetic spread property
3788+
syntheticOrigin?: Symbol; // For a property on a mapped or spread type, points back to the original property
3789+
isDiscriminantProperty?: boolean; // True if discriminant synthetic property
3790+
resolvedExports?: SymbolTable; // Resolved exports of module or combined early- and late-bound static members of a class.
3791+
resolvedMembers?: SymbolTable; // Combined early- and late-bound members of a symbol
3792+
exportsChecked?: boolean; // True if exports of external module have been checked
3793+
typeParametersChecked?: boolean; // True if type parameters of merged class and interface declarations have been checked.
37933794
isDeclarationWithCollidingName?: boolean; // True if symbol is block scoped redeclaration
3794-
bindingElement?: BindingElement; // Binding element associated with property symbol
3795-
exportsSomeValue?: boolean; // True if module exports some value (not just types)
3796-
enumKind?: EnumKind; // Enum declaration classification
3795+
bindingElement?: BindingElement; // Binding element associated with property symbol
3796+
exportsSomeValue?: boolean; // True if module exports some value (not just types)
3797+
enumKind?: EnumKind; // Enum declaration classification
37973798
originatingImport?: ImportDeclaration | ImportCall; // Import declaration which produced the symbol, present if the symbol is marked as uncallable but had call signatures in `resolveESModuleSymbol`
37983799
lateSymbol?: Symbol; // Late-bound symbol for a computed property
37993800
specifierCache?: Map<string>; // For symbols corresponding to external modules, a cache of incoming path -> module specifier name mappings

Diff for: ‎tests/baselines/reference/chainedPrototypeAssignment.errors.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ tests/cases/conformance/salsa/use.js(6,5): error TS2345: Argument of type '"not
1919
declare var exports: any;
2020
==== tests/cases/conformance/salsa/mod.js (0 errors) ====
2121
/// <reference path='./types.d.ts'/>
22-
var A = function() {
22+
var A = function A() {
2323
this.a = 1
2424
}
25-
var B = function() {
25+
var B = function B() {
2626
this.b = 2
2727
}
2828
exports.A = A

Diff for: ‎tests/baselines/reference/chainedPrototypeAssignment.symbols

+6-4
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,19 @@ declare var exports: any;
3737

3838
=== tests/cases/conformance/salsa/mod.js ===
3939
/// <reference path='./types.d.ts'/>
40-
var A = function() {
40+
var A = function A() {
4141
>A : Symbol(A, Decl(mod.js, 1, 3), Decl(mod.js, 8, 13))
42+
>A : Symbol(A, Decl(mod.js, 1, 7))
4243

4344
this.a = 1
44-
>a : Symbol(A.a, Decl(mod.js, 1, 20))
45+
>a : Symbol(A.a, Decl(mod.js, 1, 22))
4546
}
46-
var B = function() {
47+
var B = function B() {
4748
>B : Symbol(B, Decl(mod.js, 4, 3), Decl(mod.js, 9, 13))
49+
>B : Symbol(B, Decl(mod.js, 4, 7))
4850

4951
this.b = 2
50-
>b : Symbol(B.b, Decl(mod.js, 4, 20))
52+
>b : Symbol(B.b, Decl(mod.js, 4, 22))
5153
}
5254
exports.A = A
5355
>exports.A : Symbol(A, Decl(mod.js, 6, 1))

Diff for: ‎tests/baselines/reference/chainedPrototypeAssignment.types

+6-4
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ declare var exports: any;
4444

4545
=== tests/cases/conformance/salsa/mod.js ===
4646
/// <reference path='./types.d.ts'/>
47-
var A = function() {
47+
var A = function A() {
48+
>A : typeof A
49+
>function A() { this.a = 1} : typeof A
4850
>A : typeof A
49-
>function() { this.a = 1} : typeof A
5051

5152
this.a = 1
5253
>this.a = 1 : 1
@@ -55,9 +56,10 @@ var A = function() {
5556
>a : any
5657
>1 : 1
5758
}
58-
var B = function() {
59+
var B = function B() {
60+
>B : typeof B
61+
>function B() { this.b = 2} : typeof B
5962
>B : typeof B
60-
>function() { this.b = 2} : typeof B
6163

6264
this.b = 2
6365
>this.b = 2 : 2

Diff for: ‎tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.errors.txt

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
tests/cases/conformance/salsa/other.js(5,5): error TS2339: Property 'wat' does not exist on type 'One'.
2+
tests/cases/conformance/salsa/other.js(10,5): error TS2339: Property 'wat' does not exist on type 'Two'.
3+
4+
5+
==== tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.js (0 errors) ====
6+
var Ns = {}
7+
Ns.One = function() {};
8+
Ns.Two = function() {};
9+
10+
Ns.One.prototype = {
11+
ok() {},
12+
};
13+
Ns.Two.prototype = {
14+
}
15+
16+
==== tests/cases/conformance/salsa/other.js (2 errors) ====
17+
/**
18+
* @type {Ns.One}
19+
*/
20+
var one;
21+
one.wat;
22+
~~~
23+
!!! error TS2339: Property 'wat' does not exist on type 'One'.
24+
/**
25+
* @type {Ns.Two}
26+
*/
27+
var two;
28+
two.wat;
29+
~~~
30+
!!! error TS2339: Property 'wat' does not exist on type 'Two'.
31+

Diff for: ‎tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.symbols

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
=== tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.js ===
2+
var Ns = {}
3+
>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2))
4+
5+
Ns.One = function() {};
6+
>Ns.One : Symbol(Ns.One, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 3))
7+
>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2))
8+
>One : Symbol(Ns.One, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 3))
9+
10+
Ns.Two = function() {};
11+
>Ns.Two : Symbol(Ns.Two, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 7, 3))
12+
>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2))
13+
>Two : Symbol(Ns.Two, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 7, 3))
14+
15+
Ns.One.prototype = {
16+
>Ns.One.prototype : Symbol(Ns.One.prototype, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23))
17+
>Ns.One : Symbol(Ns.One, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 3))
18+
>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2))
19+
>One : Symbol(Ns.One, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 3))
20+
>prototype : Symbol(Ns.One.prototype, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23))
21+
22+
ok() {},
23+
>ok : Symbol(ok, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 4, 20))
24+
25+
};
26+
Ns.Two.prototype = {
27+
>Ns.Two.prototype : Symbol(Ns.Two.prototype, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2))
28+
>Ns.Two : Symbol(Ns.Two, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 7, 3))
29+
>Ns : Symbol(Ns, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 3), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 0, 11), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 2, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2))
30+
>Two : Symbol(Ns.Two, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 1, 23), Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 7, 3))
31+
>prototype : Symbol(Ns.Two.prototype, Decl(prototypePropertyAssignmentMergeAcrossFiles2.js, 6, 2))
32+
}
33+
34+
=== tests/cases/conformance/salsa/other.js ===
35+
/**
36+
* @type {Ns.One}
37+
*/
38+
var one;
39+
>one : Symbol(one, Decl(other.js, 3, 3))
40+
41+
one.wat;
42+
>one : Symbol(one, Decl(other.js, 3, 3))
43+
44+
/**
45+
* @type {Ns.Two}
46+
*/
47+
var two;
48+
>two : Symbol(two, Decl(other.js, 8, 3))
49+
50+
two.wat;
51+
>two : Symbol(two, Decl(other.js, 8, 3))
52+

Diff for: ‎tests/baselines/reference/prototypePropertyAssignmentMergeAcrossFiles2.types

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
=== tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.js ===
2+
var Ns = {}
3+
>Ns : typeof Ns
4+
>{} : {}
5+
6+
Ns.One = function() {};
7+
>Ns.One = function() {} : typeof One
8+
>Ns.One : typeof One
9+
>Ns : typeof Ns
10+
>One : typeof One
11+
>function() {} : typeof One
12+
13+
Ns.Two = function() {};
14+
>Ns.Two = function() {} : typeof Two
15+
>Ns.Two : typeof Two
16+
>Ns : typeof Ns
17+
>Two : typeof Two
18+
>function() {} : typeof Two
19+
20+
Ns.One.prototype = {
21+
>Ns.One.prototype = { ok() {},} : { ok(): void; }
22+
>Ns.One.prototype : { ok(): void; }
23+
>Ns.One : typeof One
24+
>Ns : typeof Ns
25+
>One : typeof One
26+
>prototype : { ok(): void; }
27+
>{ ok() {},} : { ok(): void; }
28+
29+
ok() {},
30+
>ok : () => void
31+
32+
};
33+
Ns.Two.prototype = {
34+
>Ns.Two.prototype = {} : {}
35+
>Ns.Two.prototype : {}
36+
>Ns.Two : typeof Two
37+
>Ns : typeof Ns
38+
>Two : typeof Two
39+
>prototype : {}
40+
>{} : {}
41+
}
42+
43+
=== tests/cases/conformance/salsa/other.js ===
44+
/**
45+
* @type {Ns.One}
46+
*/
47+
var one;
48+
>one : One
49+
50+
one.wat;
51+
>one.wat : any
52+
>one : One
53+
>wat : any
54+
55+
/**
56+
* @type {Ns.Two}
57+
*/
58+
var two;
59+
>two : Two
60+
61+
two.wat;
62+
>two.wat : any
63+
>two : Two
64+
>wat : any
65+

Diff for: ‎tests/cases/conformance/salsa/chainedPrototypeAssignment.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ declare function require(name: string): any;
77
declare var exports: any;
88
// @Filename: mod.js
99
/// <reference path='./types.d.ts'/>
10-
var A = function() {
10+
var A = function A() {
1111
this.a = 1
1212
}
13-
var B = function() {
13+
var B = function B() {
1414
this.b = 2
1515
}
1616
exports.A = A

Diff for: ‎tests/cases/conformance/salsa/prototypePropertyAssignmentMergeAcrossFiles2.ts

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// @Filename: prototypePropertyAssignmentMergeAcrossFiles2.js
2+
// @allowJs: true
3+
// @checkJs: true
4+
// @noEmit: true
5+
var Ns = {}
6+
Ns.One = function() {};
7+
Ns.Two = function() {};
8+
9+
Ns.One.prototype = {
10+
ok() {},
11+
};
12+
Ns.Two.prototype = {
13+
}
14+
15+
// @Filename: other.js
16+
/**
17+
* @type {Ns.One}
18+
*/
19+
var one;
20+
one.wat;
21+
/**
22+
* @type {Ns.Two}
23+
*/
24+
var two;
25+
two.wat;

0 commit comments

Comments
 (0)