Skip to content

Commit

Permalink
Fix #4032: Fix direction of instantiation.
Browse files Browse the repository at this point in the history
Intrepolation is a pretty delicate scenario. It's difficult to even say what is right and what
is wrong. In #4032 there's an unconstrained type variable that should be interpolated and it's a coin
flip whether we instantiate it to the lower or upper bound. A completely unrelated change in
#3981 meant that we instantiated the variable to the upper instead of the lower bound which
caused the program to fail. The failure was because the type variable appeared covariantly
in the lower bound of some other type variable. So maximizing the type first type variable
overconstrained the second. We avoid this problem now by computing the variance of the type
variable that's eliminated in the rest of the constraint. We take this into account to
instantiate the variable so that it narrows the overall constraint the least, defaulting
again to lower bound if there is not best direction. Since the test is expensive (linear
in the constraint size), we do this only if the variable's lower bound is unconstrained.
We could do it for all non-occurring type variables, but have to see how that would affect
performance.
  • Loading branch information
odersky committed Mar 2, 2018
1 parent 7655503 commit 4b1b9e1
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 5 deletions.
7 changes: 3 additions & 4 deletions compiler/src/dotty/tools/dotc/core/Constraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,11 @@ abstract class Constraint extends Showable {
/** Check whether predicate holds for all parameters in constraint */
def forallParams(p: TypeParamRef => Boolean): Boolean

/** Perform operation `op` on all typevars, or only on uninstantiated
* typevars, depending on whether `uninstOnly` is set or not.
*/
/** Perform operation `op` on all typevars that do not have their `inst` field set. */
def foreachTypeVar(op: TypeVar => Unit): Unit

/** The uninstantiated typevars of this constraint */
/** The uninstantiated typevars of this constraint, which still have a bounds constraint
*/
def uninstVars: collection.Seq[TypeVar]

/** The weakest constraint that subsumes both this constraint and `other` */
Expand Down
26 changes: 25 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Inferencing.scala
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,29 @@ object Inferencing {

propagate(accu(SimpleIdentityMap.Empty, tp))
}

private def varianceInContext(tvar: TypeVar)(implicit ctx: Context): FlagSet = {
object accu extends TypeAccumulator[FlagSet] {
def apply(fs: FlagSet, t: Type): FlagSet =
if (fs == EmptyFlags) fs
else if (t eq tvar)
if (variance > 0) fs &~ Contravariant
else if (variance < 0) fs &~ Covariant
else EmptyFlags
else foldOver(fs, t)
}
val constraint = ctx.typerState.constraint
val tparam = tvar.origin
(VarianceFlags /: constraint.uninstVars) { (fs, tv) =>
if ((tv `eq` tvar) || (fs == EmptyFlags)) fs
else {
val otherParam = tv.origin
val fs1 = if (constraint.isLess(tparam, otherParam)) fs &~ Covariant else fs
val fs2 = if (constraint.isLess(otherParam, tparam)) fs1 &~ Contravariant else fs1
accu(fs2, constraint.entry(otherParam))
}
}
}
}

trait Inferencing { this: Typer =>
Expand Down Expand Up @@ -389,7 +412,8 @@ trait Inferencing { this: Typer =>
if (!(vs contains tvar) && qualifies(tvar)) {
typr.println(s"instantiating non-occurring ${tvar.show} in ${tp.show} / $tp")
ensureConstrained()
tvar.instantiate(fromBelow = tvar.hasLowerBound)
tvar.instantiate(
fromBelow = tvar.hasLowerBound || !varianceInContext(tvar).is(Covariant))
}
}
if (constraint.uninstVars exists qualifies) interpolate()
Expand Down
19 changes: 19 additions & 0 deletions tests/pos/i4032.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import scala.concurrent.Future

class Gen[+T] {
def map[U](f: T => U): Gen[U] = ???
}

object Gen {
def oneOf[T](t0: T, t1: T): Gen[T] = ??? // Compile with this line commented
def oneOf[T](g0: Gen[T], g1: Gen[T]): Gen[T] = ???
}

class Arbitrary[T]

object Arbitrary {
def arbitrary[T]: Gen[T] = ???

def arbFuture[X]: Gen[Future[X]] =
Gen.oneOf(arbitrary[Future[X]], arbitrary[Throwable].map(Future.failed))
}

0 comments on commit 4b1b9e1

Please sign in to comment.