Skip to content

Commit 6e7bb3a

Browse files
committed
Merge pull request #1058 from dotty-staging/fix-#864-v2
Prune constraints that could turn into cycles
2 parents cb5935e + 7d3f6d0 commit 6e7bb3a

File tree

4 files changed

+94
-9
lines changed

4 files changed

+94
-9
lines changed

src/dotty/tools/dotc/config/Config.scala

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@ object Config {
1515
*/
1616
final val checkConstraintsNonCyclic = false
1717

18-
/** Like `checkConstraintsNonCyclic`, but all constrained parameters
19-
* are tested for direct or indirect dependencies, each time a
20-
* constraint is added in TypeComparer.
18+
/** Make sure none of the bounds of a parameter in an OrderingConstraint
19+
* contains this parameter at its toplevel (i.e. as an operand of a
20+
* combination of &'s and |'s.). The check is performed each time a new bound
21+
* is added to the constraint.
2122
*/
22-
final val checkConstraintsNonCyclicTrans = false
23+
final val checkConstraintsSeparated = false
2324

2425
/** Check that each constraint resulting from a subtype test
2526
* is satisfiable.

src/dotty/tools/dotc/core/ConstraintHandling.scala

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import Types._, Contexts._, Symbols._
66
import Decorators._
77
import config.Config
88
import config.Printers._
9+
import collection.mutable
910

1011
/** Methods for adding constraints and solving them.
1112
*
@@ -35,6 +36,18 @@ trait ConstraintHandling {
3536

3637
private def addOneBound(param: PolyParam, bound: Type, isUpper: Boolean): Boolean =
3738
!constraint.contains(param) || {
39+
def occursIn(bound: Type): Boolean = {
40+
val b = bound.dealias
41+
(b eq param) || {
42+
b match {
43+
case b: AndOrType => occursIn(b.tp1) || occursIn(b.tp2)
44+
case b: TypeVar => occursIn(b.origin)
45+
case _ => false
46+
}
47+
}
48+
}
49+
if (Config.checkConstraintsSeparated)
50+
assert(!occursIn(bound), s"$param occurs in $bound")
3851
val c1 = constraint.narrowBound(param, bound, isUpper)
3952
(c1 eq constraint) || {
4053
constraint = c1
@@ -210,7 +223,7 @@ trait ConstraintHandling {
210223
final def canConstrain(param: PolyParam): Boolean =
211224
!frozenConstraint && (constraint contains param)
212225

213-
/** Add constraint `param <: bond` if `fromBelow` is true, `param >: bound` otherwise.
226+
/** Add constraint `param <: bound` if `fromBelow` is false, `param >: bound` otherwise.
214227
* `bound` is assumed to be in normalized form, as specified in `firstTry` and
215228
* `secondTry` of `TypeComparer`. In particular, it should not be an alias type,
216229
* lazy ref, typevar, wildcard type, error type. In addition, upper bounds may
@@ -222,11 +235,67 @@ trait ConstraintHandling {
222235
//checkPropagated(s"adding $description")(true) // DEBUG in case following fails
223236
checkPropagated(s"added $description") {
224237
addConstraintInvocations += 1
238+
239+
def addParamBound(bound: PolyParam) =
240+
if (fromBelow) addLess(bound, param) else addLess(param, bound)
241+
242+
/** Drop all constrained parameters that occur at the toplevel in `bound` and
243+
* handle them by `addLess` calls.
244+
* The preconditions make sure that such parameters occur only
245+
* in one of two ways:
246+
*
247+
* 1.
248+
*
249+
* P <: Ts1 | ... | Tsm (m > 0)
250+
* Tsi = T1 & ... Tn (n >= 0)
251+
* Some of the Ti are constrained parameters
252+
*
253+
* 2.
254+
*
255+
* Ts1 & ... & Tsm <: P (m > 0)
256+
* Tsi = T1 | ... | Tn (n >= 0)
257+
* Some of the Ti are constrained parameters
258+
*
259+
* In each case we cannot leave the parameter in place,
260+
* because that would risk making a parameter later a subtype or supertype
261+
* of a bound where the parameter occurs again at toplevel, which leads to cycles
262+
* in the subtyping test. So we intentionally narrow the constraint by
263+
* recording an isLess relationship instead (even though this is not implied
264+
* by the bound).
265+
*
266+
* Narrowing a constraint is better than widening it, because narrowing leads
267+
* to incompleteness (which we face anyway, see for instance eitherIsSubType)
268+
* but widening leads to unsoundness.
269+
*
270+
* A test case that demonstrates the problem is i864.scala.
271+
* Turn Config.checkConstraintsSeparated on to get an accurate diagnostic
272+
* of the cycle when it is created.
273+
*
274+
* @return The pruned type if all `addLess` calls succeed, `NoType` otherwise.
275+
*/
276+
def prune(bound: Type): Type = bound match {
277+
case bound: AndOrType =>
278+
val p1 = prune(bound.tp1)
279+
val p2 = prune(bound.tp2)
280+
if (p1.exists && p2.exists) bound.derivedAndOrType(p1, p2)
281+
else NoType
282+
case bound: TypeVar if constraint contains bound.origin =>
283+
prune(bound.underlying)
284+
case bound: PolyParam if constraint contains bound =>
285+
if (!addParamBound(bound)) NoType
286+
else if (fromBelow) defn.NothingType
287+
else defn.AnyType
288+
case _ =>
289+
bound
290+
}
291+
225292
try bound match {
226293
case bound: PolyParam if constraint contains bound =>
227-
if (fromBelow) addLess(bound, param) else addLess(param, bound)
294+
addParamBound(bound)
228295
case _ =>
229-
if (fromBelow) addLowerBound(param, bound) else addUpperBound(param, bound)
296+
val pbound = prune(bound)
297+
pbound.exists && (
298+
if (fromBelow) addLowerBound(param, pbound) else addUpperBound(param, pbound))
230299
}
231300
finally addConstraintInvocations -= 1
232301
}

src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,10 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
124124
pendingSubTypes = new mutable.HashSet[(Type, Type)]
125125
ctx.log(s"!!! deep subtype recursion involving ${tp1.show} <:< ${tp2.show}, constraint = ${state.constraint.show}")
126126
ctx.log(s"!!! constraint = ${constraint.show}")
127-
assert(!ctx.settings.YnoDeepSubtypes.value) //throw new Error("deep subtype")
127+
//if (ctx.settings.YnoDeepSubtypes.value) {
128+
// new Error("deep subtype").printStackTrace()
129+
//}
130+
assert(!ctx.settings.YnoDeepSubtypes.value)
128131
if (Config.traceDeepSubTypeRecursions && !this.isInstanceOf[ExplainingTypeComparer])
129132
ctx.log(TypeComparer.explained(implicit ctx => ctx.typeComparer.isSubType(tp1, tp2)))
130133
}
@@ -1050,7 +1053,9 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
10501053
else hkCombine(tp1, tp2, tparams1, tparams2, op)
10511054
}
10521055

1053-
/** Try to distribute `&` inside type, detect and handle conflicts */
1056+
/** Try to distribute `&` inside type, detect and handle conflicts
1057+
* @pre !(tp1 <: tp2) && !(tp2 <:< tp1) -- these cases were handled before
1058+
*/
10541059
private def distributeAnd(tp1: Type, tp2: Type): Type = tp1 match {
10551060
// opportunistically merge same-named refinements
10561061
// this does not change anything semantically (i.e. merging or not merging

tests/pos/i864.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
object C {
2+
val a: Int = 1
3+
val b: Int = 2
4+
val c: Int = 2
5+
6+
trait X[T]
7+
implicit def u[A, B]: X[A | B] = new X[A | B] {}
8+
def y[T](implicit x: X[T]): T = ???
9+
val x: a.type & b.type | b.type & c.type = y
10+
}

0 commit comments

Comments
 (0)