Skip to content

Avoid junk produced by Constraint#replace. #678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 25, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/dotty/tools/dotc/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
3 changes: 3 additions & 0 deletions src/dotty/tools/dotc/core/Constraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
30 changes: 26 additions & 4 deletions src/dotty/tools/dotc/core/OrderingConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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 =
Expand All @@ -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)
Expand Down Expand Up @@ -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"))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use s.c.r.internal.util.Collections#mforeach and friends to cleanup code dealing with nested lists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that also work for mixed Array/List code like here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it is specialized to List[List[_]].

}

private var myUninstVars: mutable.ArrayBuffer[TypeVar] = _

/** The uninstantiated typevars of this constraint */
Expand Down
8 changes: 6 additions & 2 deletions src/dotty/tools/dotc/core/TyperState.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
14 changes: 7 additions & 7 deletions src/dotty/tools/dotc/util/SimpleMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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) = ()
}

Expand All @@ -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)
}
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions tests/pending/pos/t8230a.scala → tests/pos/t8230a.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down