Yet another Maximum call stack size exceeded (w/ fix!)

[jcormont]

TypeScript Version: 2.4.x

Code
Large library with complex recursive types, works fine with 2.3.x.

Compiling with 2.4.x the crash occurs in various places. After commenting out one whole piece of code, another area gets affected so there are a few different patterns that cause this, perhaps. I didn't manage to find a minimal example before finding the fix...

Actual behavior:

RangeError: Maximum call stack size exceeded
    at mapper (C:\...\tsc.js:28270:35)
    at instantiateTypeNoAlias (C:\...\tsc.js:28455:24)
    at instantiateType (C:\...\tsc.js:28449:24)
    at mapper (C:\...\tsc.js:28270:48)
    at mapper (C:\...\tsc.js:28270:64)
    at instantiateTypeNoAlias (C:\...\tsc.js:28455:24)
    at instantiateType (C:\...\tsc.js:28449:24)
    at mapper (C:\...\tsc.js:28270:48)
    at mapper (C:\...\tsc.js:28270:64)
    at instantiateTypeNoAlias (C:\...\tsc.js:28455:24)

NOTE: The fix below does not directly solve the other infinite recursion issues that I found here, but this is possibly related in some way.

I fixed my own issue in https://github.com/Microsoft/TypeScript/blob/master/src/compiler/checker.ts

Two observations:

• This code is a mess. It lacks comments about what these 100s of functions actually do at the function level, and is utterly unintelligible for an outsider. Also no exception handlers apparently. Good candidate for some refactoring here guys...? :)
• At least in my case, this code goes into an infite loop while "inferring" or "matching" or "resolving" or "concatenating" two types that are actually the same (===) against each other. Not sure why this happens but it does. In the case of the other infinite recursion issues this may be happening with types that refer to the same node but where the Types are not strictly equal to each other? Would there be some way to check for the same source node?
  In my case, I added a line in inferTypes and that solved it. Seems like a forgotten case:

        function inferTypes(inferences: InferenceInfo[], originalSource: Type, originalTarget: Type, priority: InferencePriority = 0) {
            let symbolStack: Symbol[];
            let visited: Map<boolean>;
            inferFromTypes(originalSource, originalTarget);

            function inferFromTypes(source: Type, target: Type) {
                if (source === target) return;  // <=== ADD THIS LINE
                // ...
            }
        }

---

EDIT by @weswigham

On a tangent:

• This code is a mess. It lacks comments about what these 100s of functions actually do at the function level, and is utterly unintelligible for an outsider. Also no exception handlers apparently. Good candidate for some refactoring here guys...? :)

checker.ts has grown from almost 10000 dense lines of code on release to almost 25000 denser lines of code in the years hence. I think most of the team is in agreement that it aught to be refactored into easier-to-understand subsystems - it's just that there are lower-hanging less-difficult fruit in the refactoring domain (normally with more tangible bugs-fixed-as-a-bonus) and only limited time to spend, and no complete plan set in stone on how to structure the refactoring (there are mostly standalone bits - the node builder, the symbol writer, the flow narrower, the relationship checker - but there are also many small bits that combine to make the big blob) without sacrificing performance or introducing regressions. For example, we just replaced our brittle conditional emitter with a tree transformer - that was a long time in the making. We're still working on replacing the declaration emitter with a transformer, too. We also want to swap from namespaces to es6 modules at some point. I mean, this is mostly just excuses for not wanting to personally be the one to pay down the technical debt that's accrued - but that's not to say we haven't experimented with trying in the past.

I definitely commend you for diving in and discovering a solution to the issue in your own!

[weswigham]

Ach! I accidentally typed my response into the edit box! (How did I do that) and there's no way to find the old text! I'll just open a PR with your fix for you... Sorry! Sorry! Sorry!

[weswigham]

@jcormont Do you have a repro you could share of your original issue; it seems like the fix you proposed actually breaks our handling of type parameters, sometimes (actually it breaks our test build), so the short circuit needs to be moved elsewhere - specifically to only cover the inference case which leads to the stack overflow. (Since we apparently extract useful information from the apparent type of the target in some situations or something even when the types are the same) I've taken a guess at the correct place (it's one of the only ones which doesn't break our build), but I can't know for sure if it still fixes your issue without a repro.

[jcormont]

Fixed the original issue text for you! I had an unrefreshed browser window open so copied it back from there and added your "edit" underneath.

Didn't realize my shortcut fix would have side effects (does that mean the compiler can infer a type from... itself? That's some dark magic!). Actually, a minimal fix for me is in inferFromObjectTypes, which is called down the tree somewhere, and has the same (source, target) parameters. There's a condition on top about mapped types, which seems to do something useful and may be necessary even with equal Type parameters (in the tests?). I moved the source === target condition underneath that block and it still works.

If I have some time today I will try to investigate and see if I can run the tsc tests myself.

Re "diving in" to this 1.35M file... 😱 It's hard to understand what's going on in the first place, unless someone takes the time to comment the heck out of this solution first. But that may be a waste if a whole new paradigm is needed.

It seems to me that the core of the issue (and probably other issues with infinite recursion) is in function instantiateType(type: Type, mapper: TypeMapper). Here, mapper is a function that may very well be recurive and has no protection against calling into itself while instantiating a type that refers to itself (and tries to instantiate itself)...?

Off topic: if it were up to me, the entire TypeScript codebase but especially the Type subsystem would move to OO (I cringe when I see something like function getAllPossiblePropertiesOfType(type: Type): Symbol[] -- wouldn't it be nice to use type.getAllPossibleProperties() or even a memoized getter on type.allPossibleProperties...) but I'm sure that's out of the question 😉

[weswigham]

@jcormont We would really appreciate a code same which reproduces your bug. It's hard to justify the change without the regression test proving its need.

[jcormont]

Agreed, this makes sense.

I'm trying to figure out where in my code the compiler crashes so I can find out what exact type causes the issue, but I can't really figure it out.

If I were to wrap the failing compiler code in a try/catch block, and just console.log the location (.ts file name, line, pos) of the symbol that the compiler is working on, how would I do that?

I.e. given the source and target instances of Type (e.g. in the inferFromTypes function), how would I get the source location from there? I know there's ts.getLineAndCharacterOfPosition but it needs a reference to the sourceFile (there are many)...

Edit: if Type doesn't have enough information, then where do I put the try/catch block at a position where the node, file, etc. are available from the context?

[weswigham]

I imagine your best bet is just to log the `typeToString` of both types.

…

On Wed, Jul 19, 2017, 1:57 AM Jelmer Cormont ***@***.***> wrote: Agreed, this makes sense. I'm trying to figure out where in *my* code the compiler crashes so I can find out what exact type causes the issue, but I can't really figure it out. If I were to wrap the failing compiler code in a try/catch block, and just console.log the location (.ts file name, line, pos) of the symbol that the compiler is working on, how would I do that? I.e. given the source and target instances of Type (e.g. in the inferFromTypes function), how would I get the source location from there? I know there's ts.getLineAndCharacterOfPosition but it needs a reference to the sourceFile (there are many)... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17234 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACzAMkSZbNsCtSfXlNT2Elv7gHrScDd8ks5sPcT2gaJpZM4OZWsj> .

[jcormont]

This is really curious. I added the return stop gap in my global tsc.js file as above. With that, the compiler at least didn't crash so I could fix some of the legitimate errors that v2.4.1 comes up with.

After fixing those issues (which were... semi-valid? see below) I decided to try removing the return from tsc.js again and hey presto, no more crash! So while that means that the crash was due to an error in my code, it also means I can't really find a good repro anymore because I got rid of the error.

I'll close the issue since this is hopeless without a repro. If anyone else encounters this exact issue they may want to try adding in the return statement as well and deduce a minimal repro from there. I'd be happy to help out if any other cases pop up.

---

Sidenote: The "legitimate" issue that I fixed is related to #16985, where given a class B<Bar> extends A<Foo>, in v2.4.1 we get a new error on var a: typeof A = B... The thinking is that a will hold a reference to any class derived from A (or A itself). But with generics, even though B is derived from A, it has a different type (including parameters) and cannot be assigned. There's a fix in the thread, but I decided to get rid of these class types altogether and use an (even more generic) interface instead.

Probably the above code results in infinite recursion when combined with default type parameters and/or function calls with inferred type parameters, and the compiler gets stuck wondering if B<inferredType?> is indeed assignable to A<T>. Tried to come up with a minimal example but gave up :)