Refine join algorithm

This was detected when failing to compile the 2.13 library.
The problem is minimized in test pos/padTo.scala. The failure
arose because with the introduction of singletons in unions
we get more "interesting" union types to approximate. But
it can be also produced without singleton types.

Author: odersky

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

@@ -131,7 +131,7 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
    *      class A extends C[A] with D
    *      class B extends C[B] with D with E
    *
-   *  we approximate `A | B` by `C[A | B] with D`
+   *  we approximate `A | B` by `C[A | B] with D`.
    */
   def orDominator(tp: Type): Type = {
 
@@ -188,29 +188,68 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
         case _ => false
       }
 
+      // Step 1: Get RecTypes and ErrorTypes out of the way,
       tp1 match {
-        case tp1: RecType =>
-          tp1.rebind(approximateOr(tp1.parent, tp2))
-        case tp1: TypeProxy if !isClassRef(tp1) =>
-          orDominator(tp1.superType | tp2)
-        case err: ErrorType =>
-          err
+        case tp1: RecType => return tp1.rebind(approximateOr(tp1.parent, tp2))
+        case err: ErrorType => return err
         case _ =>
-          tp2 match {
-            case tp2: RecType =>
-              tp2.rebind(approximateOr(tp1, tp2.parent))
-            case tp2: TypeProxy if !isClassRef(tp2) =>
-              orDominator(tp1 | tp2.superType)
-            case err: ErrorType =>
-              err
-            case _ =>
-              val commonBaseClasses = tp.mapReduceOr(_.baseClasses)(intersect)
-              val doms = dominators(commonBaseClasses, Nil)
-              def baseTp(cls: ClassSymbol): Type =
-                tp.baseType(cls).mapReduceOr(identity)(mergeRefinedOrApplied)
-              doms.map(baseTp).reduceLeft(AndType.apply)
-          }
       }
+      tp2 match {
+        case tp2: RecType => return tp2.rebind(approximateOr(tp1, tp2.parent))
+        case err: ErrorType => return err
+        case _ =>
+      }
+
+      // Step 2: Try to widen either side. This is tricky and currently incomplete.
+      // An illustration is in test pos/padTo.scala: Here we nee to compute the join of
+      //
+      //   `A | C` under the constraints `B >: A` and `C <: B`
+      //
+      // where `A, B, C` are type parameters.
+      // Widening `A` to its upper bound would give `Any | C`, i.e. `Any`.
+      // But widening `C` first would give `A | B` and then `B`.
+      // So we need to widen `C` first. But how to decide this in general?
+      // In the algorithm below, we widen both sides, and then check whether
+      // one widened type is a supertype of the other original type, in which
+      // case we can immediately pick that widened type as the join.
+      // If that yields no result, we pick the second widened type if it is
+      // a subtype of the first widened type and the first widened type otherwise.
+      // At this last step we lose possible solutions, since we have to make an
+      // arbitrary choice which side to widen. A better solution would look at
+      // the constituents of each operand (if the operand is an OrType again) and
+      // try to widen them selectively in turn. But this might lead to a combinatorial
+      // explosion of possibilities.
+      //
+      // Another approach could be to store information contained in lower bounds
+      // on both sides. So if `B >: A` we'd also record that `A <: B` and therefore
+      // widening `A` would yield `B` instead of `Any`, so we'd still be on the right track.
+      // This looks feasible if lower bounds are type parameters, but tricky if they
+      // are something else. We'd have to extract the strongest possible
+      // constraint over all type parameters that is implied by a lower bound.
+      // This looks related to an algorithmic problem arising in GADT matching.
+      val tp1w = tp1 match {
+        case tp1: TypeProxy if !isClassRef(tp1) => tp1.superType
+        case _ => tp1
+      }
+      val tp2w = tp2 match {
+        case tp2: TypeProxy if !isClassRef(tp2) => tp2.superType
+        case _ => tp1
+      }
+      if ((tp1w ne tp1) || (tp2w ne tp2)) {
+        return {
+          if (tp1 frozen_<:< tp2w) tp2w
+          else if (tp2 frozen_<:< tp1w) tp1w
+          else if (tp2w frozen_<:< tp1w) orDominator(tp1 | tp2w)
+          else orDominator(tp1w | tp2)
+        }
+      }
+
+      // Step 3: Intersect base classes of both sides
+      val commonBaseClasses = tp.mapReduceOr(_.baseClasses)(intersect)
+      val doms = dominators(commonBaseClasses, Nil)
+      def baseTp(cls: ClassSymbol): Type =
+        tp.baseType(cls).mapReduceOr(identity)(mergeRefinedOrApplied)
+      doms.map(baseTp).reduceLeft(AndType.apply)
     }
 
     tp match {

tests/pos/padTo.scala

@@ -0,0 +1,34 @@
+object Test {
+  abstract class AbstractIterator[A] extends Iterator[A] {
+    override def padTo[B >: A](len: Int, elem: B): Iterator[B] = {
+      val it = this
+      new AbstractIterator[B] {
+        private[this] var i = 0
+
+        // This illustrates a tricky situation for joins
+        // The RHS of `val b` has type `A | elem.type` where `elem: B`
+        // If we widen `A` first in the join we get a RHS of `Any` and a subsequent
+        // type error. The right thing to do is to widen `elem.type` to `B` first.
+        def next(): B = {
+          val b =
+            if (it.hasNext) it.next()
+            else if (i < len) elem
+            else Iterator.empty.next()
+          i += 1
+          b
+        }
+
+        // Same problem, but without singleton types.
+        // This one fails to compile in Scala 2.
+        def f[C <: B](c: () => C): B = {
+          val b =
+            if (it.hasNext) it.next()
+            else c()
+          b
+        }
+
+        def hasNext: Boolean = it.hasNext || i < len
+      }
+    }
+  }
+}