Repository navigation
Cache JS inferred class type symbol - #33010
Conversation
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.
| typeParameters?: TypeParameter[]; // Type parameters of type alias (undefined if non-generic) | ||
| outerTypeParameters?: TypeParameter[]; // Outer type parameters of anonymous object type | ||
| instantiations?: Map<Type>; // Instantiations of generic type alias (undefined if non-generic) | ||
| inferredClassSymbol?: TransientSymbol; // Symbol of an inferred ES5 constructor function |
There was a problem hiding this comment.
ignore whitespace to highlight the actual change
(I added some whitespace on the other lines)
|
Uggg breaks chainedPrototypeAssignment.ts A.prototype = B.prototype = { ... }I'm not convinced this is sane code, though. Accepting the baselines for now while I think about it. |
|
I thought about it. It's not real code, just a test case I came up with, so I don't feel bad about breaking it. |
|
Adding Andrew Branch (@andrewbranch) and Orta Therox (@orta) in case they want to think about caching in the compiler. =) |
Wesley Wigham (weswigham)
left a comment
Wesley Wigham (weswigham)
left a comment
There was a problem hiding this comment.
So long as the change to chainedPrototypeAssignment.types seems good to you, I think this is good.
I think the new behavior is arguably more sane - references to both Actually, wait.... they're still different constructors, just with separate prototypes, which means... ick. If Nathan Shively-Sanders (@sandersn) I think the |
|
Wesley Wigham (@weswigham) mind taking another look? |
| >b : A | ||
| >new mod.B() : A | ||
| >b : B | ||
| >new mod.B() : B |
There was a problem hiding this comment.
This looks better 😄
|
Hooray! My insane test case continues to work~~ 👽 🦑 🦀 |
* 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
3 participants
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.
Fixes #32997