Skip to content

Avoid infinite loop in type variable instantiation #9030

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 1 commit into from
May 24, 2020
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
6 changes: 6 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Constraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ abstract class Constraint extends Showable {
/** Check that no constrained parameter contains itself as a bound */
def checkNonCyclic()(implicit ctx: Context): this.type

/** Does `param` occur at the toplevel in `tp` ?
* Toplevel means: the type itself or a factor in some
* combination of `&` or `|` types.
*/
def occursAtToplevel(param: TypeParamRef, tp: Type)(using Context): Boolean

/** Check that constraint only refers to TypeParamRefs bound by itself */
def checkClosed()(implicit ctx: Context): Unit

Expand Down
17 changes: 15 additions & 2 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,21 @@ trait ConstraintHandling[AbstractContext] {
* is also a singleton type.
*/
def instanceType(param: TypeParamRef, fromBelow: Boolean)(implicit actx: AbstractContext): Type = {
val inst = approximation(param, fromBelow).simplified
if (fromBelow) widenInferred(inst, param) else inst
val approx = approximation(param, fromBelow).simplified
if (fromBelow)
val widened = widenInferred(approx, param)
// Widening can add extra constraints, in particular the widened type might
// be a type variable which is now instantiated to `param`, and therefore
// cannot be used as an instantiation of `param` without creating a loop.
// If that happens, we run `instanceType` again to find a new instantation.
// (we do not check for non-toplevel occurences: those should never occur
// since `addOneBound` disallows recursive lower bounds).
if constraint.occursAtToplevel(param, widened) then
instanceType(param, fromBelow)
else
widened
else
approx
}

/** Constraint `c1` subsumes constraint `c2`, if under `c2` as constraint we have
Expand Down
41 changes: 22 additions & 19 deletions compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
// ---------- Updates ------------------------------------------------------------

/** If `inst` is a TypeBounds, make sure it does not contain toplevel
* references to `param`. Toplevel means: the term itself or a factor in some
* combination of `&` or `|` types.
* references to `param` (see `Constraint#occursAtToplevel` for a definition
* of "toplevel").
* Any such references are replace by `Nothing` in the lower bound and `Any`
* in the upper bound.
* References can be direct or indirect through instantiations of other
Expand Down Expand Up @@ -594,33 +594,36 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
// ---------- Checking -----------------------------------------------

def checkNonCyclic()(implicit ctx: Context): this.type =
if Config.checkConstraintsNonCyclic then domainParams.foreach(checkNonCyclic)
if Config.checkConstraintsNonCyclic then
domainParams.foreach { param =>
val inst = entry(param)
assert(!isLess(param, param),
s"cyclic ordering involving $param in ${this.show}, upper = $inst")
assert(!occursAtToplevel(param, inst),
s"cyclic bound for $param: ${inst.show} in ${this.show}")
}
this

private def checkNonCyclic(param: TypeParamRef)(implicit ctx: Context): Unit =
assert(!isLess(param, param), i"cyclic ordering involving $param in $this, upper = ${upper(param)}")
def occursAtToplevel(param: TypeParamRef, inst: Type)(implicit ctx: Context): Boolean =

def recur(tp: Type)(using Context): Unit = tp match
def occurs(tp: Type)(using Context): Boolean = tp match
case tp: AndOrType =>
recur(tp.tp1)
recur(tp.tp2)
occurs(tp.tp1) || occurs(tp.tp2)
case tp: TypeParamRef =>
assert(tp ne param, i"cyclic bound for $param: ${entry(param)} in $this")
entry(tp) match
case NoType =>
case TypeBounds(lo, hi) => if lo eq hi then recur(lo)
case inst => recur(inst)
(tp eq param) || entry(tp).match
case NoType => false
case TypeBounds(lo, hi) => (lo eq hi) && occurs(lo)
case inst => occurs(inst)
case tp: TypeVar =>
recur(tp.underlying)
occurs(tp.underlying)
case TypeBounds(lo, hi) =>
recur(lo)
recur(hi)
occurs(lo) || occurs(hi)
case _ =>
val tp1 = tp.dealias
if tp1 ne tp then recur(tp1)
(tp1 ne tp) && occurs(tp1)

recur(entry(param))
end checkNonCyclic
occurs(inst)
end occursAtToplevel

override def checkClosed()(using Context): Unit =

Expand Down
12 changes: 12 additions & 0 deletions tests/neg/widenInst-cycle.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import scala.reflect.ClassTag

class Test {
def foo[N >: C | D <: C, C, D](implicit ct: ClassTag[N]): Unit = {}
// This used to lead to an infinite loop, because:
// widenInferred(?C | ?D, ?N)
// returns ?C, with the following extra constraints:
// ?C := ?N
// ?D := ?N
// So we ended up trying to instantiate ?N with ?N.
foo // error
}