Fix #16405 - wildcards prematurely resolving to Nothing #16625
Conversation
|
Never mind, forgot about the bootstrapped only tests, I'll look into this tomorrow |
jchyb
force-pushed
the
fix-i16405
branch
4 times, most recently
from
c6b8e2c to
f595650
Compare
|
Changed approach due to the above one failing the tests. |
This was a problem because it could it get in the way of some metaprogramming techniques. The main issue was the fact that when typing functions, the type inference would first look at the types from the source method (resolving type wildcards to Nothing) and only after that, it could look at the target method. Now, in the case of wildcards we save that fact for later (while still resolving the prototype parameter to Nothing) and we in that case we prioritize according to the target method, after which we fallback to the default procedure.
|
It looks a bit heavyweight for what it achieves, but I am not yet sure whether one can do better. I'll take a closer look. |
|
@jchyb Can you push it to staging please? That makes it easier for me to work with it. |
|
Done: https://github.com/dotty-staging/dotty/tree/fix-i16405 |
| @@ -1206,7 +1207,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer | |||
| * def double(x: Char): String = s"$x$x" | |||
| * "abc" flatMap double | |||
| */ | |||
| private def decomposeProtoFunction(pt: Type, defaultArity: Int, pos: SrcPos)(using Context): (List[Type], untpd.Tree) = { | |||
| private def decomposeProtoFunction(pt: Type, defaultArity: Int, pos: SrcPos)(using Context): (List[Type], Option[List[Boolean]], untpd.Tree) = { | |||
There was a problem hiding this comment.
I think I found another way that achieves the same outcome and that does not need a separate list of booleans.
| // wildcards, in case of types inferred from target being more specific | ||
|
|
||
| val fromWildcards = pt1.argInfos.init.map{ | ||
| case bounds @ TypeBounds(nt, at) if nt == defn.NothingType && at == defn.AnyType => true |
There was a problem hiding this comment.
Type comparison via == or != is not recommended. One can compare types with eq, or pattern match the structure and compare symbols. Or else use =:= which is semantic equality, equivalent to mutual subtyping, but this one is expensive.
In this specific case, there are detectors for the types your are interested in: isExactlyNothing and isAny.
| // from below, then in the case that the argument was also identified as | ||
| // a wildcard type we try to prioritize inferring from target, if possible. | ||
| // See issue 16405 (tests/run/16405.scala) | ||
| val (usingFormal, paramType) = |
There was a problem hiding this comment.
Not sure why usingFormal is needed here. Why isknownFormal not the right condition?
Fixes #16405, which was a problem because it could it get in the way of some metaprogramming techniques. The main issue was the fact that when typing functions, the type inference would first look at the types from the source method (in decomposeProtoFunction resolving found type wildcards to Nothing) and only after that, it could look at the target method. This is a continuation and simplification of #16625
Fixes #16405, which was a problem because it could it get in the way of some metaprogramming techniques. The main issue was the fact that when typing functions, the type inference would first look at the types from the source method (in
decomposeProtoFunctionresolving found type wildcards toNothing) and only after that, it could look at the target method.Now, in the case of wildcards we delay the resolution from the source method until later, after which it is resolved according to the target method (ininferredFromTarget). We also modify the target type resolution method (inferredFromTarget) a bit, as just applying the above procedure would fail some of the tests, eg.val y1: Function1[_, Nothing] = x => x
would be typed as Function1[Any, Nothing], which was incorrect.
On a side note I will fully admit that that setting the minimizeSelected option forisFullyDefinedinside ofinferredFromTargetwas a bit of an experiment on my part, nevertheless this seems to instantly work well with all of the test cases. An alternative would be to allowdecomposeProtoFunctionto proceed as before, and check ifformaltype was resolved toNothingininferredFromTargetand handle things that way, but that brings with itself its own set of issues (that I hope to avoid here).