Refine instantiation scheme in the case of non-variance

In a non-variant situation, we have to guess, which is difficult.
This commit tries to improve guessing. With it, we can compile the
either case of the TraverseTest in the collection strawman.

Author: odersky

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

@@ -230,12 +230,47 @@ trait ConstraintHandling {
   }
 
   /** The instance type of `param` in the current constraint (which contains `param`).
-   *  If `fromBelow` is true, the instance type is the lub of the parameter's
-   *  lower bounds; otherwise it is the glb of its upper bounds. However,
-   *  a lower bound instantiation can be a singleton type only if the upper bound
-   *  is also a singleton type.
+   *  The instance type TI is computed depending on the given variance as follows:
+   *
+   *  If `variance < 0`, `TI` is the glb of `param`'s upper bounds.
+   *  If `variance > 0`, `TI` is the lub of `param`'s lower bounds.
+   *                      However, if `TI` would be a singleton type, and the upper bound is
+   *                      not a singleton type, `TI` is widened to a non-singleton type.
+   *  If `variance = 0`, let `Lo1` be the lower bound of `param` and let `Lo2` be the
+   *                     |-dominator of `Lo1`. If `Lo2` is not more ugly than `Lo1`, we add the
+   *                     additional constraint that `param` should be a supertype of `Lo2`.
+   *                     We then proceed as if `variance > 0`.
+   *
+   *  The reason for tweaking the `variance = 0` case is that in this case we have to make
+   *  a guess, since no possible instance type is better than the other. We apply the heuristic
+   *  that usually an |-type is not what is intended. E.g. given two cases of types
+   *  `Some[Int]` and `None`, we'd naturally want `Option[Int]` as the inferred type, not
+   *  `Some[Int] | None`. If `variance > 0` it's OK to infer the smaller `|-type` since
+   *  we can always widen later to the intended type. But in non-variant situations, we
+   *  have to stick to the initial guess.
+   *
+   *  On the other hand, sometimes the |-dominator is _not_ what we want. For instance, taking
+   *  run/hmap.scala as an example, we have a lower bound
+   *
+   *     HEntry[$param$5, V] | HEntry[String("name"), V]
+   *
+   *  Its |-dominator is
+   *
+   *     HEntry[_ >: $param$11 & String("cat") <: String, V]
+   *
+   *  If we apply the additional constraint we get compilation failures because while
+   *  we can find implicits for the |-type above, we cannot find implicits for the |-dominator.
+   *  There's no hard criterion what should be considered ugly or not. For now we
+   *  pick the degree of undefinedness of type parameters, i.e. how many type
+   *  parameters are instantiated with wildcard types. Maybe we have to refine this
+   *  in the future.
+   *
+   *  The whole thing is clearly not nice, and I would love to have a better criterion.
+   *  In principle we are grappling with the fundamental shortcoming that local type inference
+   *  sometimes has to guess what was intended. The more refined our type lattice becomes,
+   *  the harder it is to make a choice.
    */
-  def instanceType(param: TypeParamRef, fromBelow: Boolean): Type = {
+  def instanceType(param: TypeParamRef, variance: Int): Type = {
     def upperBound = constraint.fullUpperBound(param)
     def isSingleton(tp: Type): Boolean = tp match {
       case tp: SingletonType => true
@@ -257,22 +292,44 @@ trait ConstraintHandling {
       case _ => false
     }
 
-    // First, solve the constraint.
+    // First, consider whether we need to constrain with |-dominator
+    if (variance == 0) {
+      val lo1 = ctx.typeComparer.bounds(param).lo
+      val lo2 = ctx.orDominator(lo1)
+      if (lo1 ne lo2) {
+        val ugliness = new TypeAccumulator[Int] {
+          def apply(x: Int, tp: Type) = tp match {
+            case tp: TypeBounds if !tp.isAlias => apply(apply(x + 1, tp.lo), tp.hi)
+            case tp: AndOrType => apply(x, tp.tp1) max apply(x, tp.tp2)
+            case _ => foldOver(x, tp)
+          }
+        }
+        if (ugliness(0, lo2) <= ugliness(0, lo1)) {
+          constr.println(i"apply additional constr $lo2 <: $param, was $lo1")
+          lo2 <:< param
+        }
+      }
+    }
+
+    // Then, solve the constraint.
+    val fromBelow = variance >= 0
     var inst = approximation(param, fromBelow)
 
-    // Then, approximate by (1.) - (3.) and simplify as follows.
-    // 1. If instance is from below and is a singleton type, yet
+    // Then, if instance is from below and is a singleton type, yet
     // upper bound is not a singleton type, widen the instance.
     if (fromBelow && isSingleton(inst) && !isSingleton(upperBound))
       inst = inst.widen
 
+    // Then, simplify.
     inst = inst.simplified
 
-    // 2. If instance is from below and is a fully-defined union type, yet upper bound
-    // is not a union type, approximate the union type from above by an intersection
-    // of all common base types.
-    if (fromBelow && isOrType(inst) && isFullyDefined(inst) && !isOrType(upperBound))
+    // Finally, if the instance is from below and is a fully-defined union type, yet upper bound
+    // is not a union type, harmonize the union type.
+    // TODO: See whether we can merge this with the special treatment of dependent params in
+    // simplified.
+    if (fromBelow && isOrType(inst) && isFullyDefined(inst) && !isOrType(upperBound)) {
       inst = ctx.harmonizeUnion(inst)
+    }
 
     inst
   }

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

@@ -3073,14 +3073,14 @@ object Types {
     }
 
     /** Instantiate variable from the constraints over its `origin`.
-     *  If `fromBelow` is true, the variable is instantiated to the lub
+     *  If `variance >= 0` the variable is instantiated to the lub
      *  of its lower bounds in the current constraint; otherwise it is
-     *  instantiated to the glb of its upper bounds. However, a lower bound
-     *  instantiation can be a singleton type only if the upper bound
-     *  is also a singleton type.
+     *  instantiated to the glb of its upper bounds. However, lower bound
+     *  singleton types and |-types are sometimes widened; for details see
+     *  TypeComparer#instanceType.
      */
-    def instantiate(fromBelow: Boolean)(implicit ctx: Context): Type =
-      instantiateWith(ctx.typeComparer.instanceType(origin, fromBelow))
+    def instantiate(variance: Int)(implicit ctx: Context): Type =
+      instantiateWith(ctx.typeComparer.instanceType(origin, variance))
 
     /** Unwrap to instance (if instantiated) or origin (if not), until result
      *  is no longer a TypeVar

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

@@ -72,8 +72,8 @@ object Inferencing {
    *  to their upper bound.
    */
   private class IsFullyDefinedAccumulator(force: ForceDegree.Value)(implicit ctx: Context) extends TypeAccumulator[Boolean] {
-    private def instantiate(tvar: TypeVar, fromBelow: Boolean): Type = {
-      val inst = tvar.instantiate(fromBelow)
+    private def instantiate(tvar: TypeVar, v: Int): Type = {
+      val inst = tvar.instantiate(v)
       typr.println(i"forced instantiation of ${tvar.origin} = $inst")
       inst
     }
@@ -84,29 +84,20 @@ object Inferencing {
       case tvar: TypeVar
       if !tvar.isInstantiated && ctx.typerState.constraint.contains(tvar) =>
         force.appliesTo(tvar) && {
-          if (force.implicitMode) {
-            // instantiate to or-dominator of lower bound; without this tweak we'd
-            // fail to find an implicit in situations like this:
-            //
-            //   def f[T: Ordering](xs: T*) = ...
-            //   f(Some(1), None)
-            //
-            // The problem is that there is no implicit Ordering instance for the otherwise inferred
-            // lower bound of T, which is `Some[Int] | None`.
-            ctx.orDominator(ctx.typeComparer.bounds(tvar.origin).lo) <:< tvar.origin
-          }
-          val direction = instDirection(tvar.origin)
-          if (direction != 0) {
+          var constrVariance = instVariance(tvar.origin)
+          if (variance == 0) constrVariance = constrVariance min 0
+          if (constrVariance != 0) {
             //if (direction > 0) println(s"inst $tvar dir = up")
-            instantiate(tvar, direction < 0)
+            instantiate(tvar, constrVariance)
           }
           else {
-            val minimize =
-              force.implicitMode ||
-              variance >= 0 && !(
-                force == ForceDegree.noBottom &&
-                defn.isBottomType(ctx.typeComparer.approximation(tvar.origin, fromBelow = true)))
-            if (minimize) instantiate(tvar, fromBelow = true)
+            val effectiveVariance =
+              if (force.implicitMode) 0
+              else if (force == ForceDegree.noBottom &&
+                       defn.isBottomType(ctx.typeComparer.approximation(tvar.origin, fromBelow = true)))
+                -1
+              else variance
+            if (effectiveVariance >= 0) instantiate(tvar, effectiveVariance)
             else toMaximize = true
           }
           foldOver(x, tvar)
@@ -119,7 +110,7 @@ object Inferencing {
       def apply(x: Unit, tp: Type): Unit = {
         tp match {
           case tvar: TypeVar if !tvar.isInstantiated =>
-            instantiate(tvar, fromBelow = false)
+            instantiate(tvar, -1)
           case _ =>
         }
         foldOver(x, tp)
@@ -184,21 +175,23 @@ object Inferencing {
     occurring(tree, boundVars(tree, Nil), Nil)
   }
 
-  /** The instantiation direction for given poly param computed
+  /** The preferred instantiation variance for given param ref computed
    *  from the constraint:
-   *  @return   1 (maximize) if constraint is uniformly from above,
-   *           -1 (minimize) if constraint is uniformly from below,
+   *  @return  -1 (maximize) if constraint is uniformly from above,
+   *           +1 (minimize) if constraint is uniformly from below,
    *            0 if unconstrained, or constraint is from below and above.
+   *  Note that the instantiation direction of the parameter is the reverse
+   *  of the variance: -1 means maximize, +1 means minimize.
    */
-  private def instDirection(param: TypeParamRef)(implicit ctx: Context): Int = {
+  private def instVariance(param: TypeParamRef)(implicit ctx: Context): Int = {
     val constrained = ctx.typerState.constraint.fullBounds(param)
     val original = param.binder.paramInfos(param.paramNum)
     val cmp = ctx.typeComparer
     val approxBelow =
       if (!cmp.isSubTypeWhenFrozen(constrained.lo, original.lo)) 1 else 0
     val approxAbove =
       if (!cmp.isSubTypeWhenFrozen(original.hi, constrained.hi)) 1 else 0
-    approxAbove - approxBelow
+    approxBelow - approxAbove
   }
 
   /** Recursively widen and also follow type declarations and type aliases. */
@@ -277,13 +270,13 @@ object Inferencing {
         vs foreachBinding { (tvar, v) =>
           if (v != 0) {
             typr.println(s"interpolate ${if (v == 1) "co" else "contra"}variant ${tvar.show} in ${tp.show}")
-            tvar.instantiate(fromBelow = v == 1)
+            tvar.instantiate(v)
           }
         }
       for (tvar <- constraint.uninstVars)
         if (!(vs contains tvar) && qualifies(tvar)) {
           typr.println(s"instantiating non-occurring ${tvar.show} in ${tp.show} / $tp")
-          tvar.instantiate(fromBelow = true)
+          tvar.instantiate(0)
         }
     }
     if (constraint.uninstVars exists qualifies) interpolate()
@@ -297,12 +290,11 @@ object Inferencing {
     val vs = variances(tp, alwaysTrue)
     var result: Option[TypeVar] = None
     vs foreachBinding { (tvar, v) =>
-      if (v == 1) tvar.instantiate(fromBelow = false)
-      else if (v == -1) tvar.instantiate(fromBelow = true)
+      if (v != 0) tvar.instantiate(-v)
       else {
         val bounds = ctx.typerState.constraint.fullBounds(tvar.origin)
         if (!(bounds.hi <:< bounds.lo)) result = Some(tvar)
-        tvar.instantiate(fromBelow = false)
+        tvar.instantiate(-1)
       }
     }
     result