Fix type inference for HLists and HMaps #2045
Conversation
If an applied type has a refinement, it was printed before as one large refinement type including the type parameter bindings.
No reason why we should not - normalize handles implicit methods just fine. This fixes type errors in test HLists.scala.
We approximate dependencies to parameters by Wildcards. This was already done in one place, is now done in other places as well, instead of doing nothing for dependent methods.
We previously tried to force S1 and S2 be the same type
when encountering a lub like `T1 { A = S1 } & T2 { A = S2 }`.
The comments in this commit explain why this is unsound, so
this rewrite is now made subject to a new config option, which
is off by default.
I verified that the new behavior does not affect the performance of
the junit tests.
These now compile with the changes to dependent methods, except for one which is invalid under dotty.
Variance changes quite a few things for type inference, so it's good to check a non-variant version as well.
Need to use fresh PolyParams instead of WildcardTypes if constraint is committable.
Turned out hmaps.scala requires the arg alignment to compile. So we have our first counterexample that we cannot drop this hack. Now it is made safe in the sense that no constraints get lost anymore.
I believe this worked only accidentally because we matched more things with wildcards which turned out to be flawed. The test errors show some funky _#_ types, so not sure whether the tests are still valid or not. Moved back to pending awaiting further resolution.
Type inference tends to take quite different paths for non-variant and variant data structures. Since, non-variant hmap has already exposed quite a few problems, it's good to test it also in the covariant case.
|
You might like to include some tests from SI-5070. Scalac is too pessimistic it determining if a method is dependent, dotty's implementation looks better. // should compile
trait Web {
type LocalName
}
trait Companion1[A]
trait WebDSL[W <: Web] {
trait LocalNameCompanion extends Companion1[W#LocalName] {
type A = String
}
implicit val LocalName: LocalNameCompanion
}
object Test {
def t[W <: Web](implicit webDSL: WebDSL[W]): Unit = {
import webDSL._
implicitly[LocalNameCompanion] // succeeds
implicitly[Companion1[W#LocalName]] // fails
}
}I see from trying to compile the first example that this is not allowed in dotty for other reasons: "W#LocalName: W is not a legal path". trait A {
type T
}
object O {
implicit def b(implicit x: A): x.T = error("")
}
class Test {
import O._
implicit val a: A = new A {}
implicitly[a.T]
implicitly[a.T](b(a))
}// variation that typechecks in dotty but fails in scalac. Due to scalac's approximation of `isImmediatelyDependent`
class Test {
def foo(implicit x: A) = {
object O {
implicit def b(implicit y: x.type): y.T = ???
}
import O._
b(x): x.T
implicitly[x.T]
}
}
trait A {
type T
}FYI Here's an attempted fix for SI-5070 in scalac scala/scala@2.12.x...retronym:ticket/5070 |
| // But without it we would replace the two aliases by | ||
| // `T { X >: S1 | S2 <: S1 & S2 }`, which looks weird and is probably | ||
| // not what's intended. | ||
| // Given two refinements `T1 { X = S1 }` and `T2 { X = S2 }` rwrite to |
and a typo fixed
|
@retronym I added the original SI-5070 as well as an adapted version of SI-5643 as new tests. |
| try | ||
| if (isUpper) oldBounds.derivedTypeBounds(lo, hi & bound) | ||
| else oldBounds.derivedTypeBounds(lo | bound, hi) | ||
| finally homogenizeArgs = saved |
There was a problem hiding this comment.
Could this state be stored in Context instead?
There was a problem hiding this comment.
I think this would make the logic more complicated.
| } | ||
| if (homogenizeArgs && | ||
| isNonvariantAlias(rinfo1) && isNonvariantAlias(rinfo2)) | ||
| isSameType(rinfo1, rinfo2) |
There was a problem hiding this comment.
else clause missing?
There was a problem hiding this comment.
No, we perform sameType only for its effect on the constraint
| @@ -18,7 +17,7 @@ object Sessions { | |||
|
|
|||
| // friendly interface to the theory | |||
| def runSession[S, D: Session[S]#HasDual](session: S, dual: D) = | |||
| ?[Session[S]#HasDual[D]].run(session, dual) | |||
| implicitly[Session[S]#HasDual[D]].run(session, dual) | |||
There was a problem hiding this comment.
Was keeping this test pending intentional? Does this PR change how implicitly deals with type member?
I remember seeing something similar to this def ? being use to get more precise types:
trait A { type Out }
implicit val a = new A { type Out = String }
val i1: A { type Out = String } = implicitly[A]
// error: type mismatch;
// found : A
// required: A{type Out = String}
def ?[T <: AnyRef](implicit w: T): w.type = w
val i2: A { type Out = String } = ?[A] // works fine!There was a problem hiding this comment.
No, that's an oversight. We should keep the test as it was in pending. It worked after the first commits but then stopped working again after the final commits. Given the error message it could well be something related to existential types which dotty no longer supports, so I did not investigate further. I'll change the test to what it was an add a comment.
|
/cc @adriaanm, this might be a slice of dotc that interests you. |
Two main fixes and some cleanups: