Improve handling of AndTypes on the LHS of subtype comparisons #18235
Conversation
Fixes scala#18226 Might fix some other reported issues with AndTypes as well.
|
The PR currently causes zio to fail. This is a pretty horrible error message, and I will not have the time to debug this further. I can't reproduce the problem either on my M1. When I try, it crashes before I get to the previous error message with So, what do we do? It looks like the change fixes a clear problem in subtyping and type inference, but zio relied in some way on the previous behavior. I won't be able to sort this out further. If someone else could take it over this would be great! |
|
@odersky I see some |
|
Thanks for the info! It would be great to get a PR submitting zio2 to the community build! |
|
I asked if anyone has some time to make this PR in the ZIO Discord. Let's see if someone picks up this task. If not, I can give it a try but not before this weekend, sorry. |
|
Minimized: class Has[A]
trait Foo
class TestAspect[+LowerR, -UpperR]
class Spec[-R] {
def foo[R1 <: R](aspect: TestAspect[R1, R1]): Unit = {}
}
class SuiteBuilder[R <: Has[_]] {
def toSpec(
spec: Spec[R & Has[Foo]],
aspect: TestAspect[
R & Has[Foo],
R & Has[Foo]
]
) =
spec.foo(aspect)
}-- [E007] Type Mismatch Error: try/MRS.scala:18:13 -----------------------------
18 | spec.foo(aspect)
| ^^^^^^
| Found: (aspect : TestAspect[R & Has[Foo], R & Has[Foo]])
| Required: TestAspect[R1, R1]
|
| where: R1 is a type variable with constraint <: R & Has[Foo] |
I'm gonna be honest, I just realized what the community build even is, but I might give it a shot, are these the steps? https://dotty.epfl.ch/docs/contributing/community-build.html |
Fixes a new problem exhibited in the ZIO 1 code base that got uncovered by the previous fix to AndTypes.
yeah that's correct, you can also look at an old PR that added a project e.g. #14911 |
|
Thanks for the minimization @smarter. That illustrated another problem that was previously hidden by the wrong baseType handling. |
| def tryBaseType(cls2: Symbol) = | ||
|
|
||
| def computeBase(tp: Type): Type = tp.widenDealias match | ||
| case tp @ AndType(tp1, tp2) => |
There was a problem hiding this comment.
I would call the operands tp11 and tp12 to avoid shadowing the existing tp1/tp2.
|
That fixed it! In light of the problem it found we can still keep ZIO 1 in frozen state in the community build, until there's something new that would have to be changed in ZIO itself. But it would be good to also get ZIO 2. |
|
How I fixed it I compiled with -explain, which gave me a subtype trace, but that one came too late since the problematic type variable with type variable R uninstantiated. The goal should have succeeded in But I knew that change would be too restrictive. We sometimes need the base type computation, as in here: class Row[+X]
class A
class B
class C extends Row[A]
class D extends Row[B]
C & D <: Row[A & B]Here we compute the base type of So I then tried to identified exactly those cases where the base type would lose information, and proceed to |
|
Writing my previous comment showed me that the logic can be simplified: We can always try to do the basetype computation, but if that fails we can see whether LHS was an AndType and proceed to fourthTry. Thinking more about it ... On the other hand, if we do that, the computation with the approximated base type might instantiate type variables on the right, which then could be instantiated to something less good than otherwise. So it's not a try base type, if that fails fourthTry, but a try both base type and fourthTry and pick the one that succeeds while instantiating the least amount of variables (or most amount for GADTs). Which is |
|
I also think backporting this to the LTS would be too risky. |
|
The main branch is still 3.3.x, we haven't made a branch for the lts yet |
Fixes #18226
Fixes #12077
Might fix some other reported issues with AndTypes as well.