Improve subtyping check for not yet eta-expanded higher kinded types #17139
Conversation
|
The CI is failing because of some infrastructural problems #17119 (comment) but the compilation tests passed |
prolativ
force-pushed
the
higher-kinded-type-better-subtyping
branch
from
9b92d10
to
03a8f64
Compare
|
@odersky the build is green now. Could you have a look? |
| @@ -580,7 +580,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling | |||
| val base = nonExprBaseType(tp1, cls2) | |||
| if (base.typeSymbol == cls2) return true | |||
| } | |||
| else if tp1.isLambdaSub && !tp1.isAnyKind then | |||
| else if tp1.typeParams.nonEmpty && !tp1.isAnyKind then | |||
There was a problem hiding this comment.
Not sure about this, this could be quite a drastic change that needs more motivation. Can you push to staging, then I can try some things?
There was a problem hiding this comment.
@odersky pushed.
The problem I'm trying to fix here is that when the compiler tries to compare two higher-kinded types like Foo1 and Foo2 in the test example, it returns false too early without trying to eta-expand the types. In the other cases from the linked issue, where everything compiles successfully, for some reason at least one of the compared types gets already eta-expanded at some earlier stage, which leads the compiler in the right direction to prove that the subtyping relation does occur.
To be more precise, the 2 alternative working workarounds are:
- remove the theoretically redundant package prefix, changing
implicit val bar: Bar[pkg.Foo2]into
implicit val bar: Bar[Foo2]- compile both files in a single compiler run (e.g. by renaming
Test_2.scalatoTest_1.scalawhen compiling with vulpix)
There was a problem hiding this comment.
@odersky have you managed to experiment with these changes? If there any chance to push this forward?
There was a problem hiding this comment.
This does avoid the fourthTry logic for the cases where tp1 is a generic class without type parameters. But I checked that this logic does not do anything in this case, so I think the change is OK.
Fixes #16183