Improve type inference for functions like fold #18780
Conversation
|
@smarter I think this is similar to what you once proposed. I first tried a different approach, suggested by Max Heiber. He wrote a type inferencer in Scala at Meta that is used on their complete Erlang codebase. He solved the fold problem by re-typechecking with improved info after a failure. This approach is potentially more general, since it could also fix problems where we first guess a type that is too small, but is not related to So the current approach fixes the most annoying shortcomings with low complexity and no retyping. |
| assert(!myInst.exists, i"$origin is already instantiated to $myInst but we attempted to instantiate it to $tp") | ||
| typr.println(i"instantiating $this with $tp") | ||
| if !myInst.exists then | ||
| typr.println(i"instantiating $this with $tp") |
There was a problem hiding this comment.
Is removing the assert actually needed? The code doesn't seem to rely on instantiating the same variable multiple times (and if it did it would likely be fragile since myInst may or may not be set depending on the typerstate).
There was a problem hiding this comment.
In fact no, that was left in from an earlier attempt where the assertion triggered. I re-instantiated it.
| @@ -4916,11 +4919,7 @@ object Types { | |||
| * is also a singleton type. | |||
| */ | |||
| def instantiate(fromBelow: Boolean)(using Context): Type = | |||
| val tp = TypeComparer.instanceType(origin, fromBelow, widenUnions, nestingLevel) | |||
| if myInst.exists then // The line above might have triggered instantiation of the current type variable | |||
There was a problem hiding this comment.
This check used to be necessary so I would prefer to keep it now that the assert in instantiateWith has been added back, unless we have some strong reason to believe it no longer is.
When calling a fold with an accumulator like `Nil` or `List()` one used to have add an explicit type ascription. This is now no longer necessary. When instantiating type variables that occur invariantly in the expected type of a lambda, we now replace covariant occurrences of `Nothing` in the (possibly widened) type of the accumulator with fresh type variables. The idea is that a fresh type variable in such places is always better than Nothing. For module values such as `Nil` we widen to `List[<fresh var>]`. This does possibly cause a new type error if the fold really wanted a `Nil` instance. But that case seems very rare, so it looks like a good bet in general to do the widening.
| instantiateWith(typeToInstantiateWith(fromBelow)) | ||
| val tp = typeToInstantiateWith(fromBelow) | ||
| if myInst.exists then // The line above might have triggered instantiation of the current type variable | ||
| Member |
When calling a fold with an accumulator like
NilorList()one used to have add an explicit type ascription. This is now no longer necessary. When instantiating type variables that occur invariantly in the expected type of a lambda, we now replace covariant occurrences ofNothingin the (possibly widened) instance type of the type variable with fresh type variables.In the case of fold, the accumulator determines the instance type of a type variable that appears both in the parameter list and in the result type of the closure, which makes it invariant. So the accumulator type is improved in the way described above.
The idea is that a fresh type variable in such places is always better than Nothing. For module values such as
Nilwe widen toList[<fresh var>]. This does possibly cause a new type error if the fold really wanted aNilinstance. But that case seems very rare, so it looks like a good bet in general to do the widening.