Fix constraint merges with conflicting type variables

When merging two constraints we can end up in a situation where
a type lambda is associated with different type variables in the
two constraint.

This happened when compiling concat.scala after
introducing an additional tryEither for typechecking the function
part of an application. That caused a constraint merge of
two constraints over logically separate instances of `++`, which
shared the same type lambda for `++` but associated it with
different type variables. This situation can arise since for efficiency
we do not always clone a type lambda before adding it to a constraint.

The correct way to deal with the situation is to clone the
type lambda with the conflicting TypeVars in one of the constraints
before proceeding with the merge.

Author: odersky

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

@@ -17,8 +17,8 @@ abstract class Constraint extends Showable {
 
   type This <: Constraint
 
-  /** Does the constraint's domain contain the type parameters of `pt`? */
-  def contains(pt: TypeLambda): Boolean
+  /** Does the constraint's domain contain the type parameters of `tl`? */
+  def contains(tl: TypeLambda): Boolean
 
   /** Does the constraint's domain contain the type parameter `param`? */
   def contains(param: TypeParamRef): Boolean
@@ -106,14 +106,22 @@ abstract class Constraint extends Showable {
    */
   def replace(param: TypeParamRef, tp: Type)(implicit ctx: Context): This
 
-  /** Is entry associated with `pt` removable? This is the case if
+  /** Is entry associated with `tl` removable? This is the case if
    *  all type parameters of the entry are associated with type variables
    *  which have their `inst` fields set.
    */
-  def isRemovable(pt: TypeLambda): Boolean
+  def isRemovable(tl: TypeLambda): Boolean
 
-  /** A new constraint with all entries coming from `pt` removed. */
-  def remove(pt: TypeLambda)(implicit ctx: Context): This
+  /** A new constraint with all entries coming from `tl` removed. */
+  def remove(tl: TypeLambda)(implicit ctx: Context): This
+
+  /** A new constraint with entry `tl` renamed to a fresh type lambda */
+  def rename(tl: TypeLambda)(implicit ctx: Context): This
+
+  /** The given `tl` in case it is not contained in this constraint,
+   *  a fresh copy of `tl` otherwise.
+   */
+  def ensureFresh(tl: TypeLambda)(implicit ctx: Context): TypeLambda
 
   /** The type lambdas constrained by this constraint */
   def domainLambdas: List[TypeLambda]

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

@@ -8,6 +8,7 @@ import collection.mutable
 import printing.Printer
 import printing.Texts._
 import config.Config
+import config.Printers.constr
 import reflect.ClassTag
 import annotation.tailrec
 import annotation.internal.sharable
@@ -503,6 +504,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     }
 
   def & (other: Constraint, otherHasErrors: Boolean)(implicit ctx: Context): OrderingConstraint = {
+
     def merge[T](m1: ArrayValuedMap[T], m2: ArrayValuedMap[T], join: (T, T) => T): ArrayValuedMap[T] = {
       var merged = m1
       def mergeArrays(xs1: Array[T], xs2: Array[T]) = {
@@ -527,21 +529,71 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
         case (e1: TypeBounds, e2: TypeBounds) => e1 & e2
         case (e1: TypeBounds, _) if e1 contains e2 => e2
         case (_, e2: TypeBounds) if e2 contains e1 => e1
-        case (tv1: TypeVar, tv2: TypeVar) if tv1.instanceOpt eq tv2.instanceOpt => e1
+        case (tv1: TypeVar, tv2: TypeVar) if tv1 eq tv2 => e1
         case _ =>
           if (otherHasErrors)
             e1
           else
             throw new AssertionError(i"cannot merge $this with $other, mergeEntries($e1, $e2) failed")
       }
 
-    val that = other.asInstanceOf[OrderingConstraint]
+    /** Ensure that constraint `c` does not associate different TypeVars for the
+     *  same type lambda than this constraint. Do this by renaming type lambdas
+     *  in `c` where necessary.
+     */
+    def ensureNotConflicting(c: OrderingConstraint): OrderingConstraint = {
+      def hasConflictingTypeVarsFor(tl: TypeLambda) =
+        this.typeVarOfParam(tl.paramRefs(0)) ne c.typeVarOfParam(tl.paramRefs(0))
+          // Note: Since TypeVars are allocated in bulk for each type lambda, we only
+          // have to check the first one to find out if some of them are dufferent.
+      val conflicting = c.domainLambdas.find(tl =>
+        this.contains(tl) && hasConflictingTypeVarsFor(tl))
+      conflicting match {
+        case Some(tl) => ensureNotConflicting(c.rename(tl))
+        case None => c
+      }
+    }
+
+    val that = ensureNotConflicting(other.asInstanceOf[OrderingConstraint])
+
     new OrderingConstraint(
         merge(this.boundsMap, that.boundsMap, mergeEntries),
         merge(this.lowerMap, that.lowerMap, mergeParams),
         merge(this.upperMap, that.upperMap, mergeParams))
+  }.reporting(res => i"constraint merge $this with $other = $res", constr)
+
+  def rename(tl: TypeLambda)(implicit ctx: Context): OrderingConstraint = {
+    assert(contains(tl))
+    val tl1 = ensureFresh(tl)
+    def swapKey[T](m: ArrayValuedMap[T]) = m.remove(tl).updated(tl1, m(tl))
+    var current = newConstraint(swapKey(boundsMap), swapKey(lowerMap), swapKey(upperMap))
+    def subst[T <: Type](x: T): T = x.subst(tl, tl1).asInstanceOf[T]
+    current.foreachParam {(p, i) =>
+      current = boundsLens.map(this, current, p, i, subst)
+      current = lowerLens.map(this, current, p, i, _.map(subst))
+      current = upperLens.map(this, current, p, i, _.map(subst))
+    }
+    current.foreachTypeVar { tvar =>
+      val TypeParamRef(binder, n) = tvar.origin
+      if (binder eq tl) tvar.setOrigin(tl1.paramRefs(n))
+    }
+    constr.println(i"renamd $this to $current")
+    current
   }
 
+  def ensureFresh(tl: TypeLambda)(implicit ctx: Context): TypeLambda =
+    if (contains(tl)) {
+      var paramInfos = tl.paramInfos
+      if (tl.isInstanceOf[HKLambda]) {
+        // HKLambdas are hash-consed, need to create an artificial difference by adding
+        // a LazyRef to a bound.
+        val TypeBounds(lo, hi) :: pinfos1 = tl.paramInfos
+        paramInfos = TypeBounds(lo, LazyRef(_ => hi)) :: pinfos1
+      }
+      ensureFresh(tl.newLikeThis(tl.paramNames, paramInfos, tl.resultType))
+    }
+    else tl
+
   override def checkClosed()(implicit ctx: Context): Unit = {
     def isFreeTypeParamRef(tp: Type) = tp match {
       case TypeParamRef(binder: TypeLambda, _) => !contains(binder)

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

@@ -3667,7 +3667,14 @@ object Types {
    *  `owningTree` and `owner` are used to determine whether a type-variable can be instantiated
    *  at some given point. See `Inferencing#interpolateUndetVars`.
    */
-  final class TypeVar(val origin: TypeParamRef, creatorState: TyperState) extends CachedProxyType with ValueType {
+  final class TypeVar(private var _origin: TypeParamRef, creatorState: TyperState) extends CachedProxyType with ValueType {
+
+    def origin: TypeParamRef = _origin
+
+    /** Set origin to new parameter. Called if we merge two conflicting constraints.
+     *  See OrderingConstraint#merge, OrderingConstraint#rename
+     */
+    def setOrigin(p: TypeParamRef) = _origin = p
 
     /** The permanent instance type of the variable, or NoType is none is given yet */
     private[this] var myInst: Type = NoType

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

@@ -498,20 +498,7 @@ object ProtoTypes {
         tt.withType(tvar)
       }
 
-    /** Ensure that `tl` is not already in constraint, make a copy of necessary */
-    def ensureFresh(tl: TypeLambda): TypeLambda =
-      if (state.constraint contains tl) {
-      	var paramInfos = tl.paramInfos
-      	if (tl.isInstanceOf[HKLambda]) {
-      	  // HKLambdas are hash-consed, need to create an artificial difference by adding
-      	  // a LazyRef to a bound.
-          val TypeBounds(lo, hi) :: pinfos1 = tl.paramInfos
-          paramInfos = TypeBounds(lo, LazyRef(_ => hi)) :: pinfos1
-        }
-        ensureFresh(tl.newLikeThis(tl.paramNames, paramInfos, tl.resultType))
-      }
-      else tl
-    val added = ensureFresh(tl)
+    val added = state.constraint.ensureFresh(tl)
     val tvars = if (addTypeVars) newTypeVars(added) else Nil
     ctx.typeComparer.addToConstraint(added, tvars.tpes.asInstanceOf[List[TypeVar]])
     (added, tvars)

tests/pos/concats.scala

@@ -0,0 +1,11 @@
+object Test {
+  type Name = String
+  val CommonOpNames: Set[Name]   = Set("OR", "XOR")
+  val ConversionNames: Set[Name] = Set("toByte")
+  val BooleanOpNames: Set[Name]  = Set("ZOR") ++ CommonOpNames
+  val NumberOpNames: Set[Name]   = (
+       Set("ADD")
+    ++ Set("UNARY_+", "UNARY_-")
+    ++ CommonOpNames
+  )
+}