Handle recursions in isFullyDefined

[odersky]

If one indulges in cycles too heavily it can happen that we run into
an infinite recursion in fullyDefinedType even before we had a chance
to run checkNonCyclic. In this case we now diagnose the stack overflow
as an error message instead of failing with an internal error.

Fixes #15311

[smarter]

Note that the original example can be turned into an infinite loop by replacing type Clone = by type Clone <::

trait Template[+T <: Template[T]]:
   type Clone <: T { type Clone <: Template.this.Clone }
   val self :Clone

type Food = Template[_]

class Ham extends Template[Ham]:
   type Clone = Ham
   val self = this

def eat[F <: Template[F]](food :F) :F = food.self.self

val ham = new Ham
val food :Food = ham

def test = 
  eat(ham)
  eat(food.self)

[odersky]

It's not an infinite loop, it just takes a minute or two to spit out the "Recursion limit exceeded" error.

[odersky]

How I fixed it:

That one took some time to diagnose the root cause. The error message was

java.lang.Error: internal error: type of right-hand side Playground.Template[Playground.food.T] is not fully defined, pos = [929..933..952]

I printed the type with -Yprint-debug, and then by displaying its toString directly at the site where the error occurred. But it looked OK, just a TypeRef. There was no TypeVar or WildcardType in that type that would have caused it to be not fully defined. After a lot more instrumentation I finally got to the root cause. In Inferencing.isFullyDefined:

    val result =
      try new IsFullyDefinedAccumulator(force)(using nestedCtx).process(tp)
      catch case ex: StackOverflowError =>
        false

Indeed there was a stack overflow. That caused isFullyDefined to return false, which caused the error.
Now, this is very old code. We now have better means to deal with stack overflow, using handleRecursive.
I added that to the apply method of the IsFullyDefinedAccumulator. The problem was how to recover from it in a way that's compatible with the way isFullyDefined is used. Normally handleRecursive will just throw
a RecursionOverflow exception, which is a subclass of TypeError. TypeErrors are reported in a summary manner at key points of type checking by catching that exception. In our case, we had to convert a RecursionOverflow back to a false result in isFullyDefined because that is its contract, but had to report it in the caller fullyDefinedType instead of throwing the previous Error.

That presented another problem: fullyDefinedType just got a span as a parameter instead of a full source position. That was done for performance, source positions are objects and creating them in operations like fullyDefinedType that are called very often incurs a cost. But since fullyDefinedType was written, we now have a cheap way to generate source positions using SrcPos (it's worthwhile to look that up to see how it is done!). So one could have kept the old signature and just converted a span to a source position using ctx.source.atSpan(span) in fullyDefinedType. But I thought it was more idiomatic to use SrcPos instead of Span since we do that almost everywhere else. That caused quite a few fixes all over the code base in the calls to fullyDefinedType.

[dwijnand]

I just re-read the diff and the "how I fixed it" comment (because I'm looking at handleRecursive and trace) and shouldn't the default here be true and fullDefinedType pass true so that it can do it's special handling? My assumption is that we would want all other calls of isFullyDefined to continue to return false instead of throwing.

Or are do we want to throw because isFullyDefined is call transitively from IsFullyDefinedAccumulator and we want those handled by the handleRecursive you added? My point is handleOverflow seems to never be non-default, was that for future possible uses?