Merge pull request #678 from dotty-staging/fix/#670-orphan-polyparam

Avoid junk produced by Constraint#replace.

7 files changed, 51 insertions, 16 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..7e27ee628 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(_.filterConserve(_.binder ne pt))
+        if (bndss.corresponds(bndss1)(_ eq _)) bndss else bndss1
+      }
+      po.remove(pt).mapValuesNow(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)
     }
 
diff --git a/src/dotty/tools/dotc/util/SimpleMap.scala b/src/dotty/tools/dotc/util/SimpleMap.scala
index 7bd263f0f..b8668d7e4 100644
--- a/src/dotty/tools/dotc/util/SimpleMap.scala
+++ b/src/dotty/tools/dotc/util/SimpleMap.scala
@@ -8,7 +8,7 @@ abstract class SimpleMap[K <: AnyRef, +V >: Null <: AnyRef] extends (K => V) {
   def remove(k: K): SimpleMap[K, V]
   def updated[V1 >: V <: AnyRef](k: K, v: V1): SimpleMap[K, V1]
   def contains(k: K): Boolean = apply(k) != null
-  def mapValues[V1 >: V <: AnyRef](f: (K, V1) => V1): SimpleMap[K, V1]
+  def mapValuesNow[V1 >: V <: AnyRef](f: (K, V1) => V1): SimpleMap[K, V1]
   def foreachBinding(f: (K, V) => Unit): Unit
   def map2[T](f: (K, V) => T): List[T] = {
     val buf = new ListBuffer[T]
@@ -32,7 +32,7 @@ object SimpleMap {
     def apply(k: AnyRef) = null
     def remove(k: AnyRef) = this
     def updated[V1 >: Null <: AnyRef](k: AnyRef, v: V1) = new Map1(k, v)
-    def mapValues[V1 >: Null <: AnyRef](f: (AnyRef, V1) => V1) = this
+    def mapValuesNow[V1 >: Null <: AnyRef](f: (AnyRef, V1) => V1) = this
     def foreachBinding(f: (AnyRef, Null) => Unit) = ()
   }
 
@@ -49,7 +49,7 @@ object SimpleMap {
     def updated[V1 >: V <: AnyRef](k: K, v: V1) =
       if (k == k1) new Map1(k, v)
       else new Map2(k1, v1, k, v)
-    def mapValues[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
+    def mapValuesNow[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
       val w1 = f(k1, v1)
       if (v1 eq w1) this else new Map1(k1, w1)
     }
@@ -70,7 +70,7 @@ object SimpleMap {
       if (k == k1) new Map2(k, v, k2, v2)
       else if (k == k2) new Map2(k1, v1, k, v)
       else new Map3(k1, v1, k2, v2, k, v)
-    def mapValues[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
+    def mapValuesNow[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
       val w1 = f(k1, v1); val w2 = f(k2, v2)
       if ((v1 eq w1) && (v2 eq w2)) this
       else new Map2(k1, w1, k2, w2)
@@ -95,7 +95,7 @@ object SimpleMap {
       else if (k == k2) new Map3(k1, v1, k, v, k3, v3)
       else if (k == k3) new Map3(k1, v1, k2, v2, k, v)
       else new Map4(k1, v1, k2, v2, k3, v3, k, v)
-    def mapValues[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
+    def mapValuesNow[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
       val w1 = f(k1, v1); val w2 = f(k2, v2); val w3 = f(k3, v3)
       if ((v1 eq w1) && (v2 eq w2) && (v3 eq w3)) this
       else new Map3(k1, w1, k2, w2, k3, w3)
@@ -123,7 +123,7 @@ object SimpleMap {
       else if (k == k3) new Map4(k1, v1, k2, v2, k, v, k4, v4)
       else if (k == k4) new Map4(k1, v1, k2, v2, k3, v3, k, v)
       else new MapMore(Array[AnyRef](k1, v1, k2, v2, k3, v3, k4, v4, k, v))
-    def mapValues[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
+    def mapValuesNow[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
       val w1 = f(k1, v1); val w2 = f(k2, v2); val w3 = f(k3, v3); val w4 = f(k4, v4)
       if ((v1 eq w1) && (v2 eq w2) && (v3 eq w3) && (v4 eq w4)) this
       else new Map4(k1, w1, k2, w2, k3, w3, k4, w4)
@@ -197,7 +197,7 @@ object SimpleMap {
       false
     }
 
-    def mapValues[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
+    def mapValuesNow[V1 >: V <: AnyRef](f: (K, V1) => V1) = {
       var bindings1: Array[AnyRef] = bindings
       var i = 0
       while (i < bindings.length) {
diff --git a/tests/pending/pos/t8230a.scala b/tests/pos/t8230a.scala
index 405aa86f5..dfbae51ee 100644
--- a/tests/pending/pos/t8230a.scala
+++ b/tests/pos/t8230a.scala
@@ -15,9 +15,9 @@ 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