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 scala#670.

Author: odersky

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.
    */

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
 }

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 */

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
 

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)
     }
 

tests/pending/pos/t8230a.scala renamed to tests/pos/t8230a.scala

@@ -15,12 +15,12 @@ object Test {
   object Okay {
     Arr("1")
 
-    import I.{ arrToTrav, longArrToTrav }
-    foo(Arr("2"))
-  }
+   import I.{ arrToTrav, longArrToTrav }
+   val x = foo(Arr("2"))
+ }
 
   object Fail {
-    import I.arrToTrav
-    foo(Arr("3")) // found String, expected Long
+//    import I.arrToTrav
+//    foo(Arr("3")) // found String, expected Long
   }
 }