Skip to content

Cache anonymous object type instantiations#4464

Merged
ahejlsberg merged 3 commits into
masterfrom
cacheTypeInstantiations
Aug 26, 2015
Merged

Cache anonymous object type instantiations#4464
ahejlsberg merged 3 commits into
masterfrom
cacheTypeInstantiations

Conversation

@ahejlsberg
Copy link
Copy Markdown
Member

Fixes #4443.

This PR adds a cache to TypeMapper which caches instantiations of anonymous object types previously created using that type mapper. The PR supercedes #4456.

@ahejlsberg
Copy link
Copy Markdown
Member Author

Tests copied from #4456.

@DanielRosenwasser
Copy link
Copy Markdown
Member

Can you also add this test?

tests/cases/compiler/typeArgumentInferenceWithRecursivelyReferencedTypeAliasToTypeLiteral03.ts

type TreeNode = {
    leftRight: [TreeNode, TreeNode];
};

function foo<U>(x: TreeNode) { }
var n: TreeNode;
foo(n);

@ahejlsberg
Copy link
Copy Markdown
Member Author

@DanielRosenwasser Don't think we need that test. It doesn't really test anything new, rather it just reveals an issue with the solution in #4456 that doesn't apply here.

@DanielRosenwasser
Copy link
Copy Markdown
Member

👍

@JsonFreeman
Copy link
Copy Markdown
Contributor

👍
I remember when we tried this before, but dismissed it because it didn't work for infinitely expanding anonymous types. But it sure works for recursive types.

@DanielRosenwasser
Copy link
Copy Markdown
Member

@JsonFreeman what are the cases in which it didn't work?

@JsonFreeman
Copy link
Copy Markdown
Contributor

@DanielRosenwasser, we fixed those already. See PR #3316 and #3452 (there are links to related issues there).

ahejlsberg added a commit that referenced this pull request Aug 26, 2015
Cache anonymous object type instantiations
@ahejlsberg ahejlsberg merged commit bbaf322 into master Aug 26, 2015
@ahejlsberg ahejlsberg deleted the cacheTypeInstantiations branch August 26, 2015 20:30
@DanielRosenwasser
Copy link
Copy Markdown
Member

@ahejlsberg can you cherry pick this into release-1.6 if it hasn't been done already?

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants