Skip to content

Commit 57808b6

Browse files
authored
Two fixes to constraint solving (#16353)
## 1. Fix replace operation In OrderingConstraint#replace we moved the actual replacement of a parameter with a type from the start of replace to its end, since that was more convenient for dependency adjustments. It turns out that doing so causes infinite recursion in instantiations in some cases, specifically if a parameter contains itself as an indirect lower bound that goes through an alias. Here is a situation that arises in i16311.scala: ```scala type WithTag[T, U] = T & Tagged[U] T1 >: WithTag[T2, Int] T2 >: T1 & Tagged[Int] ``` The correct instantiation for T1 and T2 is Nothing. But we ran into a cycle instead. The fix is to move the parameter replacement back to the start of `replace`, and to account for that in the dependency adjustment logic. Fixes #16311 (with failing Ycheck) ## 2. See through aliases before decomposing And/Or in isSubType There seem to be two missing cases in TypeComparer where we have a TypeParamRef on one side and an And/Or type under an alias on the other. Examples: type AND = A & B type OR = A | B p <:< AND OR <:< p In this case we missed the decomposition into smaller types that would happen otherwise. This broke i16311.scala in Ycheck and broke i15365.scala with an infinite recursion in avoidance. I verified that having an AndType as super or subtype of an abstract type works as expected. So if in the example above type AND >: A & B or type AND <: A & B it worked before. It was just aliases that were the problem (I assume it's the same for OrTypes as lower bounds). This fixes #16311 completely and also Fixes #15365
2 parents e09175e + c13cab0 commit 57808b6

File tree

8 files changed

+124
-44
lines changed

8 files changed

+124
-44
lines changed

compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -297,27 +297,36 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
297297
*/
298298
private trait ConstraintAwareTraversal[T] extends TypeAccumulator[T]:
299299

300+
/** Does `param` have bounds in the current constraint? */
301+
protected def hasBounds(param: TypeParamRef): Boolean = entry(param).isInstanceOf[TypeBounds]
302+
300303
override def tyconTypeParams(tp: AppliedType)(using Context): List[ParamInfo] =
301304
def tparams(tycon: Type): List[ParamInfo] = tycon match
302305
case tycon: TypeVar if !tycon.inst.exists => tparams(tycon.origin)
303-
case tycon: TypeParamRef =>
304-
entry(tycon) match
305-
case _: TypeBounds => tp.tyconTypeParams
306-
case tycon1 if tycon1.typeParams.nonEmpty => tycon1.typeParams
307-
case _ => tp.tyconTypeParams
306+
case tycon: TypeParamRef if !hasBounds(tycon) =>
307+
val entryParams = entry(tycon).typeParams
308+
if entryParams.nonEmpty then entryParams
309+
else tp.tyconTypeParams
308310
case _ => tp.tyconTypeParams
309311
tparams(tp.tycon)
310312

311313
override def applyToPrefix(x: T, tp: NamedType): T =
312314
this(x, tp.prefix)
313315
end ConstraintAwareTraversal
314316

315-
private class Adjuster(srcParam: TypeParamRef)(using Context)
317+
/** A type traverser that adjust dependencies originating from a given type
318+
* @param ignoreBinding if not null, a parameter that is assumed to be still uninstantiated.
319+
* This is necessary to handle replacements.
320+
*/
321+
private class Adjuster(srcParam: TypeParamRef, ignoreBinding: TypeParamRef | Null)(using Context)
316322
extends TypeTraverser, ConstraintAwareTraversal[Unit]:
317323

318324
var add: Boolean = compiletime.uninitialized
319325
val seen = util.HashSet[LazyRef]()
320326

327+
override protected def hasBounds(param: TypeParamRef) =
328+
(param eq ignoreBinding) || super.hasBounds(param)
329+
321330
def update(deps: ReverseDeps, referenced: TypeParamRef): ReverseDeps =
322331
val prev = deps.at(referenced)
323332
val newSet = if add then prev + srcParam else prev - srcParam
@@ -326,12 +335,11 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
326335

327336
def traverse(t: Type) = t match
328337
case param: TypeParamRef =>
329-
entry(param) match
330-
case _: TypeBounds =>
331-
if variance >= 0 then coDeps = update(coDeps, param)
332-
if variance <= 0 then contraDeps = update(contraDeps, param)
333-
case tp =>
334-
traverse(tp)
338+
if hasBounds(param) then
339+
if variance >= 0 then coDeps = update(coDeps, param)
340+
if variance <= 0 then contraDeps = update(contraDeps, param)
341+
else
342+
traverse(entry(param))
335343
case tp: LazyRef =>
336344
if !seen.contains(tp) then
337345
seen += tp
@@ -342,8 +350,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
342350
/** Adjust dependencies to account for the delta of previous entry `prevEntry`
343351
* and the new bound `entry` for the type parameter `srcParam`.
344352
*/
345-
def adjustDeps(entry: Type | Null, prevEntry: Type | Null, srcParam: TypeParamRef)(using Context): this.type =
346-
val adjuster = new Adjuster(srcParam)
353+
def adjustDeps(entry: Type | Null, prevEntry: Type | Null, srcParam: TypeParamRef, ignoreBinding: TypeParamRef | Null = null)(using Context): this.type =
354+
val adjuster = new Adjuster(srcParam, ignoreBinding)
347355

348356
/** Adjust reverse dependencies of all type parameters referenced by `bound`
349357
* @param isLower `bound` is a lower bound
@@ -676,7 +684,14 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
676684
override def apply(t: Type): Type =
677685
if (t eq replacedTypeVar) && t.exists then to else mapOver(t)
678686

679-
var current = this
687+
val coDepsOfParam = coDeps.at(param)
688+
val contraDepsOfParam = contraDeps.at(param)
689+
690+
var current = updateEntry(this, param, replacement)
691+
// Need to update param early to avoid infinite recursion on instantiation.
692+
// See i16311.scala for a test case. On the other hand, for the purposes of
693+
// dependency adjustment, we need to pretend that `param` is still unbound.
694+
// We achieve that by passing a `ignoreBinding = param` to `adjustDeps` below.
680695

681696
def removeParamFrom(ps: List[TypeParamRef]) =
682697
ps.filterConserve(param ne _)
@@ -710,22 +725,15 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
710725
newDepEntry = mapReplacedTypeVarTo(replacement)(newDepEntry)
711726
case _ =>
712727
if oldDepEntry ne newDepEntry then
713-
if current eq this then
714-
// We can end up here if oldEntry eq newEntry, so posssibly no new constraint
715-
// was created, but oldDepEntry ne newDepEntry. In that case we must make
716-
// sure we have a new constraint before updating dependencies.
717-
current = newConstraint()
718-
current.adjustDeps(newDepEntry, oldDepEntry, other)
728+
current.adjustDeps(newDepEntry, oldDepEntry, other, ignoreBinding = param)
719729
end replaceParamIn
720730

721731
if optimizeReplace then
722-
val co = current.coDeps.at(param)
723-
val contra = current.contraDeps.at(param)
724732
current.foreachParam { (p, i) =>
725733
val other = p.paramRefs(i)
726734
entry(other) match
727735
case _: TypeBounds =>
728-
if co.contains(other) || contra.contains(other) then
736+
if coDepsOfParam.contains(other) || contraDepsOfParam.contains(other) then
729737
replaceParamIn(other)
730738
case _ => replaceParamIn(other)
731739
}
@@ -734,10 +742,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
734742
val other = p.paramRefs(i)
735743
if other != param then replaceParamIn(other)
736744
}
737-
738-
current =
739-
if isRemovable(param.binder) then current.remove(param.binder)
740-
else updateEntry(current, param, replacement)
745+
if isRemovable(param.binder) then current = current.remove(param.binder)
741746
current.dropDeps(param)
742747
current.checkWellFormed()
743748
end replace

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

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -418,16 +418,16 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
418418
true
419419
}
420420
def compareTypeParamRef =
421-
assumedTrue(tp1) ||
422-
tp2.match {
423-
case tp2: TypeParamRef => constraint.isLess(tp1, tp2)
424-
case _ => false
425-
} ||
426-
isSubTypeWhenFrozen(bounds(tp1).hi.boxed, tp2) || {
427-
if (canConstrain(tp1) && !approx.high)
428-
addConstraint(tp1, tp2, fromBelow = false) && flagNothingBound
429-
else thirdTry
430-
}
421+
assumedTrue(tp1)
422+
|| tp2.dealias.match
423+
case tp2a: TypeParamRef => constraint.isLess(tp1, tp2a)
424+
case tp2a: AndType => recur(tp1, tp2a)
425+
case _ => false
426+
|| isSubTypeWhenFrozen(bounds(tp1).hi.boxed, tp2)
427+
|| (if canConstrain(tp1) && !approx.high then
428+
addConstraint(tp1, tp2, fromBelow = false) && flagNothingBound
429+
else thirdTry)
430+
431431
compareTypeParamRef
432432
case tp1: ThisType =>
433433
val cls1 = tp1.cls
@@ -585,7 +585,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
585585
}
586586

587587
def compareTypeParamRef(tp2: TypeParamRef): Boolean =
588-
assumedTrue(tp2) || {
588+
assumedTrue(tp2)
589+
|| {
589590
val alwaysTrue =
590591
// The following condition is carefully formulated to catch all cases
591592
// where the subtype relation is true without needing to add a constraint
@@ -596,11 +597,13 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
596597
// widening in `fourthTry` before adding to the constraint.
597598
if (frozenConstraint) recur(tp1, bounds(tp2).lo.boxed)
598599
else isSubTypeWhenFrozen(tp1, tp2)
599-
alwaysTrue || {
600-
if (canConstrain(tp2) && !approx.low)
601-
addConstraint(tp2, tp1.widenExpr, fromBelow = true)
602-
else fourthTry
603-
}
600+
alwaysTrue
601+
|| tp1.dealias.match
602+
case tp1a: OrType => recur(tp1a, tp2)
603+
case _ => false
604+
|| (if canConstrain(tp2) && !approx.low then
605+
addConstraint(tp2, tp1.widenExpr, fromBelow = true)
606+
else fourthTry)
604607
}
605608

606609
def thirdTry: Boolean = tp2 match {
File renamed without changes.
File renamed without changes.

tests/pos/i16311a.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
object test:
2+
3+
trait Tagged[U]
4+
type WithTag[+T, U] = T & Tagged[U]
5+
6+
trait FromInput[Val]
7+
implicit def coercedScalaInput[T]: FromInput[WithTag[T, Int]] = ???
8+
implicit def optionInput[T](implicit ev: FromInput[T]): FromInput[Option[T]] = ???
9+
10+
trait WithoutInputTypeTags[T]
11+
implicit def coercedOptArgTpe[T]: WithoutInputTypeTags[Option[T & Tagged[Int]]] = ???
12+
13+
trait InputType[+T]
14+
class OptionInputType[T](ofType: InputType[T]) extends InputType[Option[T]]
15+
16+
type Argument[T]
17+
def argument[T](argumentType: InputType[T])(implicit fromInput: FromInput[T], res: WithoutInputTypeTags[T]): Argument[Option[T]] = ???
18+
19+
def test = argument(OptionInputType(??? : InputType[Boolean & Tagged[Int]]))

tests/pos/i16311b.scala

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
trait Tagged[U]
2+
type WithTag[+T, U] = T & Tagged[U]
3+
4+
trait FromInput[Val]
5+
implicit def coercedScalaInput[T]: FromInput[T & Tagged[Int]] = ???
6+
implicit def optionInput[T](implicit ev: FromInput[T]): FromInput[Option[T]] = ???
7+
8+
trait WithoutInputTypeTags[T]
9+
implicit def coercedOptArgTpe[T]: WithoutInputTypeTags[Option[T & Tagged[Int]]] = ???
10+
11+
trait InputType[+T]
12+
class OptionInputType[T](ofType: InputType[T]) extends InputType[Option[T]]
13+
14+
type Argument[T]
15+
def argument[T](argumentType: InputType[T])(implicit fromInput: FromInput[T], res: WithoutInputTypeTags[T]): Argument[Option[T]] = ???
16+
17+
def test = argument(OptionInputType(??? : InputType[Boolean & Tagged[Int]]))

tests/pos/i16311c.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
class C:
2+
trait Tagged[U]
3+
type WithTag[+T, U] >: T & Tagged[U]
4+
5+
trait FromInput[Val]
6+
implicit def coercedScalaInput[T]: FromInput[WithTag[T, Int]] = ???
7+
implicit def optionInput[T](implicit ev: FromInput[T]): FromInput[Option[T]] = ???
8+
9+
trait WithoutInputTypeTags[T]
10+
implicit def coercedOptArgTpe[T]: WithoutInputTypeTags[Option[WithTag[T, Int]]] = ???
11+
12+
trait InputType[+T]
13+
class OptionInputType[T](ofType: InputType[T]) extends InputType[Option[T]]
14+
15+
type Argument[T]
16+
def argument[T](argumentType: InputType[T])(implicit fromInput: FromInput[T], res: WithoutInputTypeTags[T]): Argument[Option[T]] = ???
17+
18+
def test = argument(OptionInputType(??? : InputType[WithTag[Boolean, Int]]))

tests/pos/i16311d.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
class C:
2+
trait Tagged[U]
3+
type WithTag[+T, U] <: T & Tagged[U]
4+
5+
trait FromInput[Val]
6+
implicit def coercedScalaInput[T]: FromInput[WithTag[T, Int]] = ???
7+
implicit def optionInput[T](implicit ev: FromInput[T]): FromInput[Option[T]] = ???
8+
9+
trait WithoutInputTypeTags[T]
10+
implicit def coercedOptArgTpe[T]: WithoutInputTypeTags[Option[WithTag[T, Int]]] = ???
11+
12+
trait InputType[+T]
13+
class OptionInputType[T](ofType: InputType[T]) extends InputType[Option[T]]
14+
15+
type Argument[T]
16+
def argument[T](argumentType: InputType[T])(implicit fromInput: FromInput[T], res: WithoutInputTypeTags[T]): Argument[Option[T]] = ???
17+
18+
def test = argument(OptionInputType(??? : InputType[WithTag[Boolean, Int]]))

0 commit comments

Comments
 (0)