Pinging @ahejlsberg, @vladima, and @JsonFreeman 😄

It's definitely better to check the symbol associated with type, except not all types have symbols associated with them. For example, tuple types have no associated symbols. I'd suggest you modify the logic to check if either the type objects match or the symbols associated with the types match and are non-undefined.

I'm not sure it entirely fixes the problem, though. For example:

type TreeNode = {
    name: string;
    parent: TreeNode;
}
function foo<U>(x: TreeNode) { }
var n: TreeNode;
foo(n);  // Error

This reports an error after running into the depth === 100 safe guard we have in objectTypeRelatedTo.

I'm still thinking about a better way to catch the runaway instantiation. The key issue is that instantiateType assumes anonymous object types never reference themselves circularly. That used to be the case before we introduced type aliases, but obviously it isn't anymore.

@ahejlsberg this is true, but that scenario isn't exacerbated by this fix. At the very least, the compiler won't crash now.

I'd suggest you modify the logic to check if either the type objects match or the symbols associated with the types match and are non-undefined.

Would it be better to check for mutual "matchedness"? For instance, if the source type has a symbol, the source "matches" if its symbol is the same as the stack symbol, otherwise it matches if it is the same type. Then check if both the source and target match:

for (let i = 0; i < depth; i++) {
    let sourceStackType = sourceStack[i];
    let targetStackType = targetStack[i];

    let sourcesMatch = sourceSymbol ?
        sourceSymbol === sourceStackType.symbol :
        source === sourceStackType;

    let targetsMatch = targetSymbol ?
        targetSymbol === targetStackType.symbol :
        target === targetStackType;

    if (sourcesMatch && targetsMatch) {
        return true;
    }
}

I'm not sure specifically which examples this would come up in, but it doesn't sound out of the question.

Actually, neither of those fixes this case:

type TreeNode = { leftRight: [/*left*/ TreeNode, /*right*/ TreeNode] };

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

@DanielRosenwasser Why is type argument inference diving into properties in your original repro? Seems like it should just stop immediately because it encounters a type parameter on the target side, no?

Checking for the symbol is what we switched to for isDeeplyNestedGeneric, so that it could handle infinitely expanding anonymous types. So it seems similar. For @ahejlsberg's example and your last example, I think you'd have to make a similar fix, but in objectTypeRelatedTo rather than inferFromTypes. It may involve using symbol id instead of type id in the maybe stack.

You may also have to do something similar for printing types - it may use a similar mechanism.

And it's true about tuple types having no symbol, but that means it's a problem in isDeeplyNestedGeneric too, because we have been comparing the symbols in that function.

Actually, the tuples not having symbols thing is not an issue for isDeeplyNestedGeneric because we check that the type is TypeFlags.Reference or TypeFlags.Instantiated. But I don't think it would be possible to do a similar kind of check in isInProcess. You can have recursive tuple types like:

type tuple = [tuple];

I have a better fix for this issue. The core of the problem is that instantiateType endlessly creates new instantiations of the same type, so I decided to address that by adding a cache to the TypeMapper object that keeps track of instantiations of anonymous object types already created using that mapper. It fixes the original issue and passes all tests. I will put up a new PR soon.

Should this PR be closed?

Yup thanks for the heads up Jason