Skip to content

Simplify level fixing scheme #15936

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
Aug 30, 2022
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
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
}