From 4b1b9e17af9568e652c6241854b4295c76dae62d Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Fri, 2 Mar 2018 09:07:58 +0100
Subject: [PATCH] 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.
---
 .../dotty/tools/dotc/core/Constraint.scala    |  7 +++--
 .../dotty/tools/dotc/typer/Inferencing.scala  | 26 ++++++++++++++++++-
 tests/pos/i4032.scala                         | 19 ++++++++++++++
 3 files changed, 47 insertions(+), 5 deletions(-)
 create mode 100644 tests/pos/i4032.scala

diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala
index 57aaa8d68ee2..037cffa3256b 100644
--- a/compiler/src/dotty/tools/dotc/core/Constraint.scala
+++ b/compiler/src/dotty/tools/dotc/core/Constraint.scala
@@ -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` */
diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala
index 9219bcdd8979..76e810c0321c 100644
--- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala
@@ -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 =>
@@ -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()
diff --git a/tests/pos/i4032.scala b/tests/pos/i4032.scala
new file mode 100644
index 000000000000..0c274d08d79d
--- /dev/null
+++ b/tests/pos/i4032.scala
@@ -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))
+}
\ No newline at end of file