Skip to content

Commit

Permalink
Simplify level fixing scheme
Browse files Browse the repository at this point in the history
Don't instantiate co- and contravariant inner type variables eagerly.
Lift them instead to the reference level, same as for invariant type variables.

Fixes scala#15934
  • Loading branch information
odersky committed Aug 30, 2022
1 parent 104e8df commit 79b284b
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 31 deletions.
48 changes: 17 additions & 31 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -482,14 +482,9 @@ trait ConstraintHandling {
* and computes on the side sets of nested type variables that need
* to be instantiated.
*/
class NeedsLeveling extends TypeAccumulator[Boolean]:
def needsLeveling = new TypeAccumulator[Boolean]:
if !fromBelow then variance = -1

/** Nested type variables that should be instiated to theor lower (respoctively
* upper) bounds.
*/
var nestedVarsLo, nestedVarsHi: SimpleIdentitySet[TypeVar] = SimpleIdentitySet.empty

def apply(need: Boolean, tp: Type) =
need || tp.match
case tp: NamedType =>
Expand All @@ -499,39 +494,30 @@ trait ConstraintHandling {
val inst = tp.instanceOpt
if inst.exists then apply(need, inst)
else if tp.nestingLevel > maxLevel then
if variance > 0 then nestedVarsLo += tp
else if variance < 0 then nestedVarsHi += tp
else
// For invariant type variables, we use a different strategy.
// Rather than instantiating to a bound and then propagating in an
// AvoidMap, change the nesting level of an invariant type
// variable to `maxLevel`. This means that the type variable will be
// instantiated later to a less nested type. If there are other references
// to the same type variable that do not come from the type undergoing
// `fixLevels`, this could lead to coarser types. But it has the potential
// to give a better approximation for the current type, since it avoids forming
// a Range in invariant position, which can lead to very coarse types further out.
constr.println(i"widening nesting level of type variable $tp from ${tp.nestingLevel} to $maxLevel")
ctx.typerState.setNestingLevel(tp, maxLevel)
// Change the nesting level of inner type variable to `maxLevel`.
// This means that the type variable will be instantiated later to a
// less nested type. If there are other references to the same type variable
// that do not come from the type undergoing `fixLevels`, this could lead
// to coarser types than intended. An alternative is to instantiate the
// type variable right away, but this also loses information. See
// i15934.scala for a test where the current strategey works but an early instantiation
// of `tp` would fail.
constr.println(i"widening nesting level of type variable $tp from ${tp.nestingLevel} to $maxLevel")
ctx.typerState.setNestingLevel(tp, maxLevel)
true
else false
case _ =>
foldOver(need, tp)
end NeedsLeveling
end needsLeveling

class LevelAvoidMap extends TypeOps.AvoidMap:
def levelAvoid = new TypeOps.AvoidMap:
if !fromBelow then variance = -1
def toAvoid(tp: NamedType) = needsFix(tp)

if !Config.checkLevelsOnInstantiation || ctx.isAfterTyper then tp
else
val needsLeveling = NeedsLeveling()
if needsLeveling(false, tp) then
typr.println(i"instance $tp for $param needs leveling to $maxLevel, nested = ${needsLeveling.nestedVarsLo.toList} | ${needsLeveling.nestedVarsHi.toList}")
needsLeveling.nestedVarsLo.foreach(_.instantiate(fromBelow = true))
needsLeveling.nestedVarsHi.foreach(_.instantiate(fromBelow = false))
LevelAvoidMap()(tp)
else tp
if Config.checkLevelsOnInstantiation && !ctx.isAfterTyper && needsLeveling(false, tp) then
typr.println(i"instance $tp for $param needs leveling to $maxLevel")
levelAvoid(tp)
else tp
end fixLevels

/** Solve constraint set for given type parameter `param`.
Expand Down
44 changes: 44 additions & 0 deletions tests/pos/i15934.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
trait ReplicatedData
trait ActorRef[-T] {
def tell(msg: T): Unit = ???
}

// shared in both domains
abstract class Key[+T1 <: ReplicatedData]

// domain 1
object dd {
sealed abstract class GetResponse[A1 <: ReplicatedData] {
def key: Key[A1]
}
case class GetSuccess[A2 <: ReplicatedData](key: Key[A2]) extends GetResponse[A2]
case class GetFailure[A3 <: ReplicatedData](key: Key[A3]) extends GetResponse[A3]
}

// domain 2
object JReplicator {
final case class Get[A4 <: ReplicatedData](
key: Key[A4],
replyTo: ActorRef[GetResponse[A4]]
)
sealed abstract class GetResponse[A5 <: ReplicatedData] {
def key: Key[A5]
}
case class GetSuccess[A6 <: ReplicatedData](key: Key[A6]) extends GetResponse[A6]
case class GetFailure[A7 <: ReplicatedData](key: Key[A7]) extends GetResponse[A7]
}

val _ = null.asInstanceOf[Any] match {
case cmd: JReplicator.Get[d] =>
val reply =
util
.Try[dd.GetResponse[d]](???)
.map/*[JReplicator.GetResponse[d]]*/ {
// Needs at least 2 cases to triger failure
case rsp: dd.GetSuccess[d1] => JReplicator.GetSuccess(rsp.key)
case rsp: dd.GetResponse[d2] => JReplicator.GetFailure(rsp.key)
}
// needs recover to trigger failure
.recover { case _ => new JReplicator.GetFailure(cmd.key) }
reply.foreach { cmd.replyTo tell _ } // error
}

0 comments on commit 79b284b

Please sign in to comment.