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.
Further join refinements

Author: odersky

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

@@ -131,7 +131,11 @@ 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`.
+   *
+   *  Before we do that, we try to find a common non-class supertype of T1 | ... | Tn
+   *  in a "best effort", ad-hoc way by selectively widening types in `T1, ..., Tn`
+   *  and stopping if the resulting union simplifies to a type that is not a disjunction.
    */
   def orDominator(tp: Type): Type = {
 
@@ -188,29 +192,82 @@ 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 incomplete.
+      // An illustration is in test pos/padTo.scala: Here we need 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 try to widen both sides (once), and then proceed as follows:
+      //
+      //  0. If no widening succeeds, proceed with step 3.
+      //  1. If only one widening succeeds, pick that one.
+      //  2. If the two widened types are in a subtype relationship, pick the smaller one.
+      //  3. If exactly one of the two types is a singleton type, pick that one.
+      //  4. If the widened tp1 is a supertype of tp2, pick widened tp1.
+      //  5. If the widened tp2 is a supertype of tp1, pick widened tp2.
+      //  6. Otherwise, pick tp1
+      //
+      // At steps 4-6 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.
+      //
+      // However, this alone is still not enough. There are other sources of incompleteness,
+      // for instance arising from mis-aligned refinements.
+      val tp1w = tp1 match {
+        case tp1: TypeProxy if !isClassRef(tp1) => tp1.superType.widenExpr
+        case _ => tp1
+      }
+      val tp2w = tp2 match {
+        case tp2: TypeProxy if !isClassRef(tp2) => tp2.superType.widenExpr
+        case _ => tp2
+      }
+      if ((tp1w ne tp1) || (tp2w ne tp2)) {
+        val isSingle1 = tp1.isInstanceOf[SingletonType]
+        val isSingle2 = tp2.isInstanceOf[SingletonType]
+        return {
+          if (tp2w eq tp2) orDominator(tp1w | tp2)
+          else if (tp1w eq tp1) orDominator(tp1 | tp2w)
+          else if (tp1w frozen_<:< tp2w) orDominator(tp1w | tp2)
+          else if (tp2w frozen_<:< tp1w) orDominator(tp1 | tp2w)
+          else if (isSingle1 && !isSingle2) orDominator(tp1w | tp2)
+          else if (isSingle2 && !isSingle1) orDominator(tp1 | tp2w)
+          else if (tp1 frozen_<:< tp2w) 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
+      }
+    }
+  }
+}