Avoid junk produced by Constraint#replace.

There were two instances where a constraint undergoing a replace would still refer
to poly params that are no longer bound after the replace.

1. In an ordering the replaced parameters was n ot removed from the bounds of the others.
2. When a parameter refers to the replaced parameter in a type, (not a TypeBounds), the
replaced parameter was not replaced.

We now have checking in place that in globally committable typer states, TypeVars are not instantiated
to PolyParams and (configurable) that constraints of such typer states are always closed.

Fixes #670.

5 files changed, 41 insertions, 6 deletions

diff --git a/src/dotty/tools/dotc/config/Config.scala b/src/dotty/tools/dotc/config/Config.scala
index 782a2f2d3..9e9974bdc 100644
--- a/src/dotty/tools/dotc/config/Config.scala
+++ b/src/dotty/tools/dotc/config/Config.scala
@@ -32,6 +32,9 @@ object Config {
    */
   final val checkConstraintsPropagated = false
 
+  /** Check that constraints of globally committable typer states are closed */
+  final val checkConstraintsClosed = true
+
   /** Check that no type appearing as the info of a SymDenotation contains
    *  skolem types.
    */
diff --git a/src/dotty/tools/dotc/core/Constraint.scala b/src/dotty/tools/dotc/core/Constraint.scala
index 5a758f144..19f93ce47 100644
--- a/src/dotty/tools/dotc/core/Constraint.scala
+++ b/src/dotty/tools/dotc/core/Constraint.scala
@@ -146,4 +146,7 @@ abstract class Constraint extends Showable {
 
   /** Check that no constrained parameter contains itself as a bound */
   def checkNonCyclic()(implicit ctx: Context): Unit
+
+  /** Check that constraint only refers to PolyParams bound by itself */
+  def checkClosed()(implicit ctx: Context): Unit
 }
diff --git a/src/dotty/tools/dotc/core/OrderingConstraint.scala b/src/dotty/tools/dotc/core/OrderingConstraint.scala
index 21d003451..4c52f58e8 100644
--- a/src/dotty/tools/dotc/core/OrderingConstraint.scala
+++ b/src/dotty/tools/dotc/core/OrderingConstraint.scala
@@ -399,7 +399,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       def removeParam(ps: List[PolyParam]) =
         ps.filterNot(p => p.binder.eq(poly) && p.paramNum == idx)
 
-      def replaceParam(tp: Type, atPoly: PolyType, atIdx: Int) = tp match {
+      def replaceParam(tp: Type, atPoly: PolyType, atIdx: Int): Type = tp match {
         case bounds @ TypeBounds(lo, hi) =>
 
           def recombine(andor: AndOrType, op: (Type, Boolean) => Type, isUpper: Boolean): Type = {
@@ -424,7 +424,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
           }
 
           bounds.derivedTypeBounds(replaceIn(lo, isUpper = false), replaceIn(hi, isUpper = true))
-        case _ => tp
+        case _ =>
+          tp.substParam(param, replacement)
       }
 
       var current =
@@ -438,8 +439,16 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     }
   }
 
-  def remove(pt: PolyType)(implicit ctx: Context): This =
-    newConstraint(boundsMap.remove(pt), lowerMap.remove(pt), upperMap.remove(pt))
+  def remove(pt: PolyType)(implicit ctx: Context): This = {
+    def removeFromOrdering(po: ParamOrdering) = {
+      def removeFromBoundss(key: PolyType, bndss: Array[List[PolyParam]]): Array[List[PolyParam]] = {
+        val bndss1 = bndss.map(bnds => bnds.filterConserve(_.binder ne pt))
+        if ((0 until bndss.length).forall(i => bndss(i) eq bndss1(i))) bndss else bndss1
+      }
+      po.remove(pt).mapValues(removeFromBoundss)
+    }
+    newConstraint(boundsMap.remove(pt), removeFromOrdering(lowerMap), removeFromOrdering(upperMap))
+  }
 
   def isRemovable(pt: PolyType, removedParam: Int = -1): Boolean = {
     val entries = boundsMap(pt)
@@ -491,6 +500,19 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       }
     }
 
+  override def checkClosed()(implicit ctx: Context): Unit = {
+    def isFreePolyParam(tp: Type) = tp match {
+      case PolyParam(binder, _) => !contains(binder)
+      case _ => false
+    }
+    def checkClosedType(tp: Type, where: String) =
+      if (tp != null)
+        assert(!tp.existsPart(isFreePolyParam), i"unclosed constraint: $this refers to $tp in $where")
+    boundsMap.foreachBinding((_, tps) => tps.foreach(checkClosedType(_, "bounds")))
+    lowerMap.foreachBinding((_, paramss) => paramss.foreach(_.foreach(checkClosedType(_, "lower"))))
+    upperMap.foreachBinding((_, paramss) => paramss.foreach(_.foreach(checkClosedType(_, "upper"))))
+  }
+
   private var myUninstVars: mutable.ArrayBuffer[TypeVar] = _
 
   /** The uninstantiated typevars of this constraint */
diff --git a/src/dotty/tools/dotc/core/TyperState.scala b/src/dotty/tools/dotc/core/TyperState.scala
index 91cda1dd8..ba7a6a806 100644
--- a/src/dotty/tools/dotc/core/TyperState.scala
+++ b/src/dotty/tools/dotc/core/TyperState.scala
@@ -9,6 +9,7 @@ import util.{SimpleMap, DotClass}
 import reporting._
 import printing.{Showable, Printer}
 import printing.Texts._
+import config.Config
 import collection.mutable
 
 class TyperState(r: Reporter) extends DotClass with Showable {
@@ -19,7 +20,7 @@ class TyperState(r: Reporter) extends DotClass with Showable {
   /** The current constraint set */
   def constraint: Constraint =
     new OrderingConstraint(SimpleMap.Empty, SimpleMap.Empty, SimpleMap.Empty)
-  def constraint_=(c: Constraint): Unit = {}
+  def constraint_=(c: Constraint)(implicit ctx: Context): Unit = {}
 
   /** The uninstantiated variables */
   def uninstVars = constraint.uninstVars
@@ -85,7 +86,10 @@ extends TyperState(r) {
   private var myConstraint: Constraint = previous.constraint
 
   override def constraint = myConstraint
-  override def constraint_=(c: Constraint) = myConstraint = c
+  override def constraint_=(c: Constraint)(implicit ctx: Context) = {
+    if (Config.checkConstraintsClosed && isGlobalCommittable) c.checkClosed()
+    myConstraint = c
+  }
 
   private var myEphemeral: Boolean = previous.ephemeral
 
diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala
index 026e69539..1270466e9 100644
--- a/src/dotty/tools/dotc/core/Types.scala
+++ b/src/dotty/tools/dotc/core/Types.scala
@@ -2458,6 +2458,9 @@ object Types {
       if (fromBelow && isOrType(inst) && isFullyDefined(inst) && !isOrType(upperBound))
         inst = inst.approximateUnion
 
+      if (ctx.typerState.isGlobalCommittable)
+        assert(!inst.isInstanceOf[PolyParam], i"bad inst $this := $inst, constr = ${ctx.typerState.constraint}")
+
       instantiateWith(inst)
     }