Skip to content

Commit 4b1b9e1

Browse files
committed
Fix #4032: Fix direction of instantiation.
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.
1 parent 7655503 commit 4b1b9e1

File tree

3 files changed

+47
-5
lines changed

3 files changed

+47
-5
lines changed

compiler/src/dotty/tools/dotc/core/Constraint.scala

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,11 @@ abstract class Constraint extends Showable {
128128
/** Check whether predicate holds for all parameters in constraint */
129129
def forallParams(p: TypeParamRef => Boolean): Boolean
130130

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

136-
/** The uninstantiated typevars of this constraint */
134+
/** The uninstantiated typevars of this constraint, which still have a bounds constraint
135+
*/
137136
def uninstVars: collection.Seq[TypeVar]
138137

139138
/** The weakest constraint that subsumes both this constraint and `other` */

compiler/src/dotty/tools/dotc/typer/Inferencing.scala

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,29 @@ object Inferencing {
313313

314314
propagate(accu(SimpleIdentityMap.Empty, tp))
315315
}
316+
317+
private def varianceInContext(tvar: TypeVar)(implicit ctx: Context): FlagSet = {
318+
object accu extends TypeAccumulator[FlagSet] {
319+
def apply(fs: FlagSet, t: Type): FlagSet =
320+
if (fs == EmptyFlags) fs
321+
else if (t eq tvar)
322+
if (variance > 0) fs &~ Contravariant
323+
else if (variance < 0) fs &~ Covariant
324+
else EmptyFlags
325+
else foldOver(fs, t)
326+
}
327+
val constraint = ctx.typerState.constraint
328+
val tparam = tvar.origin
329+
(VarianceFlags /: constraint.uninstVars) { (fs, tv) =>
330+
if ((tv `eq` tvar) || (fs == EmptyFlags)) fs
331+
else {
332+
val otherParam = tv.origin
333+
val fs1 = if (constraint.isLess(tparam, otherParam)) fs &~ Covariant else fs
334+
val fs2 = if (constraint.isLess(otherParam, tparam)) fs1 &~ Contravariant else fs1
335+
accu(fs2, constraint.entry(otherParam))
336+
}
337+
}
338+
}
316339
}
317340

318341
trait Inferencing { this: Typer =>
@@ -389,7 +412,8 @@ trait Inferencing { this: Typer =>
389412
if (!(vs contains tvar) && qualifies(tvar)) {
390413
typr.println(s"instantiating non-occurring ${tvar.show} in ${tp.show} / $tp")
391414
ensureConstrained()
392-
tvar.instantiate(fromBelow = tvar.hasLowerBound)
415+
tvar.instantiate(
416+
fromBelow = tvar.hasLowerBound || !varianceInContext(tvar).is(Covariant))
393417
}
394418
}
395419
if (constraint.uninstVars exists qualifies) interpolate()

tests/pos/i4032.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import scala.concurrent.Future
2+
3+
class Gen[+T] {
4+
def map[U](f: T => U): Gen[U] = ???
5+
}
6+
7+
object Gen {
8+
def oneOf[T](t0: T, t1: T): Gen[T] = ??? // Compile with this line commented
9+
def oneOf[T](g0: Gen[T], g1: Gen[T]): Gen[T] = ???
10+
}
11+
12+
class Arbitrary[T]
13+
14+
object Arbitrary {
15+
def arbitrary[T]: Gen[T] = ???
16+
17+
def arbFuture[X]: Gen[Future[X]] =
18+
Gen.oneOf(arbitrary[Future[X]], arbitrary[Throwable].map(Future.failed))
19+
}

0 commit comments

Comments
 (0)