Avoid the TypeVar.inst trap

odersky · odersky · commit 2863a29f9e6c · 2024-04-11T12:31:17.000+02:00
`tvar.inst` gives the _permanent_ instance of a type variable `tvar`. Even if `tvar.isInstantiated` is true its `inst` can still be NoType. This is a trap that caused a regression in the code of glb. This commit fixes the regression and introduces different names that will hopefully avoid the trap in the future. Fixes #20154
diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala
@@ -315,7 +315,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
 
     override def tyconTypeParams(tp: AppliedType)(using Context): List[ParamInfo] =
       def tparams(tycon: Type): List[ParamInfo] = tycon match
-        case tycon: TypeVar if !tycon.inst.exists => tparams(tycon.origin)
+        case tycon: TypeVar if !tycon.isPermanentlyInstantiated => tparams(tycon.origin)
         case tycon: TypeParamRef if !hasBounds(tycon) =>
           val entryParams = entry(tycon).typeParams
           if entryParams.nonEmpty then entryParams
@@ -715,7 +715,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
         var newDepEntry = newEntry
         replacedTypeVar match
           case tvar: TypeVar =>
-            if tvar.inst.exists // `isInstantiated` would use ctx.typerState.constraint rather than the current constraint
+            if tvar.isPermanentlyInstantiated // `isInstantiated` would use ctx.typerState.constraint rather than the current constraint
             then
               // If the type variable has been instantiated, we need to forget about
               // the instantiation for old dependencies.
@@ -781,7 +781,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     @tailrec def allRemovable(last: Int): Boolean =
       if (last < 0) true
       else typeVar(entries, last) match {
-        case tv: TypeVar => tv.inst.exists && allRemovable(last - 1)
+        case tv: TypeVar => tv.isPermanentlyInstantiated && allRemovable(last - 1)
         case _ => false
       }
     allRemovable(paramCount(entries) - 1)
@@ -887,7 +887,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       val limit = paramCount(entries)
       while i < limit do
         typeVar(entries, i) match
-          case tv: TypeVar if !tv.inst.exists => op(tv)
+          case tv: TypeVar if !tv.isPermanentlyInstantiated => op(tv)
           case _ =>
         i += 1
     }
@@ -896,12 +896,12 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
 
   /** The uninstantiated typevars of this constraint */
   def uninstVars: collection.Seq[TypeVar] = {
-    if (myUninstVars == null || myUninstVars.uncheckedNN.exists(_.inst.exists)) {
+    if (myUninstVars == null || myUninstVars.uncheckedNN.exists(_.isPermanentlyInstantiated)) {
       myUninstVars = new mutable.ArrayBuffer[TypeVar]
       boundsMap.foreachBinding { (poly, entries) =>
         for (i <- 0 until paramCount(entries))
           typeVar(entries, i) match {
-            case tv: TypeVar if !tv.inst.exists && isBounds(entries(i)) => myUninstVars.uncheckedNN += tv
+            case tv: TypeVar if !tv.isPermanentlyInstantiated && isBounds(entries(i)) => myUninstVars.uncheckedNN += tv
             case _ =>
           }
       }
diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -1609,7 +1609,7 @@ object SymDenotations {
       case tp: RefinedType => hasSkolems(tp.parent) || hasSkolems(tp.refinedInfo)
       case tp: RecType => hasSkolems(tp.parent)
       case tp: TypeBounds => hasSkolems(tp.lo) || hasSkolems(tp.hi)
-      case tp: TypeVar => hasSkolems(tp.inst)
+      case tp: TypeVar => hasSkolems(tp.permanentInst)
       case tp: ExprType => hasSkolems(tp.resType)
       case tp: AppliedType => hasSkolems(tp.tycon) || tp.args.exists(hasSkolems)
       case tp: LambdaType => tp.paramInfos.exists(hasSkolems) || hasSkolems(tp.resType)
diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala
@@ -1600,7 +1600,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
         val tycon1 = liftToThis(tp.tycon)
         if (tycon1 ne tp.tycon) tp.derivedAppliedType(tycon1, tp.args) else tp
       case tp: TypeVar if tp.isInstantiated =>
-        liftToThis(tp.inst)
+        liftToThis(tp.instanceOpt)
       case tp: AnnotatedType =>
         val parent1 = liftToThis(tp.parent)
         if (parent1 ne tp.parent) tp.derivedAnnotatedType(parent1, tp.annot) else tp
@@ -2521,14 +2521,14 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
 
     def isSuperOf(sub: Type): Boolean = sub match
       case AndType(sub1, sub2) => isSuperOf(sub1) || isSuperOf(sub2)
-      case sub: TypeVar if sub.isInstantiated => isSuperOf(sub.inst)
+      case sub: TypeVar if sub.isInstantiated => isSuperOf(sub.instanceOpt)
       case _ => isSubTypeWhenFrozen(sub, tp)
 
     tp match
       case tp @ AndType(tp1, tp2) =>
         recombine(dropIfSuper(tp1, sub), dropIfSuper(tp2, sub), tp)
       case tp: TypeVar if tp.isInstantiated =>
-        dropIfSuper(tp.inst, sub)
+        dropIfSuper(tp.instanceOpt, sub)
       case _ =>
         if isSuperOf(sub) then NoType else tp
   end dropIfSuper
@@ -2538,14 +2538,14 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
 
     def isSubOf(sup: Type): Boolean = sup match
       case OrType(sup1, sup2) => isSubOf(sup1) || isSubOf(sup2)
-      case sup: TypeVar if sup.isInstantiated => isSubOf(sup.inst)
+      case sup: TypeVar if sup.isInstantiated => isSubOf(sup.instanceOpt)
       case _ => isSubType(tp, sup, whenFrozen = !canConstrain)
 
     tp match
       case tp @ OrType(tp1, tp2) =>
         recombine(dropIfSub(tp1, sup, canConstrain), dropIfSub(tp2, sup, canConstrain), tp)
       case tp: TypeVar if tp.isInstantiated =>
-        dropIfSub(tp.inst, sup, canConstrain)
+        dropIfSub(tp.instanceOpt, sup, canConstrain)
       case _ =>
         if isSubOf(sup) then NoType else tp
   end dropIfSub
diff --git a/compiler/src/dotty/tools/dotc/core/TyperState.scala b/compiler/src/dotty/tools/dotc/core/TyperState.scala
@@ -231,7 +231,7 @@ class TyperState() {
           val tvars = tl.paramRefs.map(other.typeVarOfParam(_)).collect { case tv: TypeVar => tv }
           if this.isCommittable then
             tvars.foreach(tvar =>
-              if !tvar.inst.exists && !isOwnedAnywhere(this, tvar) then includeVar(tvar))
+              if !tvar.isPermanentlyInstantiated && !isOwnedAnywhere(this, tvar) then includeVar(tvar))
           typeComparer.addToConstraint(tl, tvars)
         }) &&
         // Integrate the additional constraints on type variables from `other`
@@ -287,10 +287,10 @@ class TyperState() {
       for tvar <- ownedVars do
         val tvarState = tvar.owningState.nn.get
         assert(tvarState eqn this, s"Inconsistent state in $this: it owns $tvar whose owningState is ${tvarState}")
-        assert(!tvar.inst.exists, s"Inconsistent state in $this: it owns $tvar which is already instantiated")
+        assert(!tvar.isPermanentlyInstantiated, s"Inconsistent state in $this: it owns $tvar which is already instantiated")
         val inst = constraint.instType(tvar)
         if inst.exists then
-          tvar.setInst(inst)
+          tvar.setPermanentInst(inst)
           val tl = tvar.origin.binder
           if constraint.isRemovable(tl) then toCollect += tl
       for tl <- toCollect do
diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala
@@ -139,7 +139,7 @@ object Types extends TypeUtils {
             case t: AppliedType =>
               t.fold(false, (x, tp) => x || test(tp, theAcc))
             case t: TypeVar =>
-              !t.inst.exists || test(t.inst, theAcc)
+              !t.isPermanentlyInstantiated || test(t.permanentInst, theAcc)
             case t: LazyRef =>
               !t.completed || test(t.ref, theAcc)
             case _ =>
@@ -4934,20 +4934,24 @@ object Types extends TypeUtils {
     def setOrigin(p: TypeParamRef) = currentOrigin = p
 
     /** The permanent instance type of the variable, or NoType is none is given yet */
-    private var myInst: Type = NoType
+    private var inst: Type = NoType
 
-    private[core] def inst: Type = myInst
-    private[core] def setInst(tp: Type): Unit =
-      myInst = tp
+    /** The permanent instance type that's stored in the type variable, so it cannot be retracted
+     *  anymore, or NoType if the variable can still be further constrained or a provisional
+     *  instance type in the constraint can be retracted.
+     */
+    private[core] def permanentInst = inst
+    private[core] def setPermanentInst(tp: Type): Unit =
+      inst = tp
       if tp.exists && owningState != null then
         val owningState1 = owningState.uncheckedNN.get
         if owningState1 != null then
           owningState1.ownedVars -= this
           owningState = null // no longer needed; null out to avoid a memory leak
 
     private[core] def resetInst(ts: TyperState): Unit =
-      assert(myInst.exists)
-      myInst = NoType
+      assert(inst.exists)
+      inst = NoType
       owningState = new WeakReference(ts)
 
     /** The state owning the variable. This is at first `creatorState`, but it can
@@ -4985,18 +4989,23 @@ object Types extends TypeUtils {
     /** Is the variable already instantiated? */
     def isInstantiated(using Context): Boolean = instanceOpt.exists
 
+    /** Is the variable already instantiated so that the instance cannot be
+     *  retracted anymore?
+     */
+    def isPermanentlyInstantiated: Boolean = inst.exists
+
     /** Instantiate variable with given type */
     def instantiateWith(tp: Type)(using Context): Type = {
       assert(tp ne this, i"self instantiation of $origin, constraint = ${ctx.typerState.constraint}")
-      assert(!myInst.exists, i"$origin is already instantiated to $myInst but we attempted to instantiate it to $tp")
+      assert(!inst.exists, i"$origin is already instantiated to $inst but we attempted to instantiate it to $tp")
       typr.println(i"instantiating $this with $tp")
 
       if Config.checkConstraintsSatisfiable then
         assert(currentEntry.bounds.contains(tp),
           i"$origin is constrained to be $currentEntry but attempted to instantiate it to $tp")
 
       if ((ctx.typerState eq owningState.nn.get.uncheckedNN) && !TypeComparer.subtypeCheckInProgress)
-        setInst(tp)
+        setPermanentInst(tp)
       ctx.typerState.constraint = ctx.typerState.constraint.replace(origin, tp)
       tp
     }
@@ -5013,8 +5022,8 @@ object Types extends TypeUtils {
      */
     def instantiate(fromBelow: Boolean)(using Context): Type =
       val tp = typeToInstantiateWith(fromBelow)
-      if myInst.exists then // The line above might have triggered instantiation of the current type variable
-        myInst
+      if inst.exists then // The line above might have triggered instantiation of the current type variable
+        inst
       else
         instantiateWith(tp)
 
diff --git a/tests/pos/i20154.scala b/tests/pos/i20154.scala
@@ -0,0 +1,15 @@
+sealed abstract class Kyo[+T, -S]
+opaque type <[+T, -S] >: T = T | Kyo[T, S]
+
+abstract class Effect[+E]:
+    type Command[_]
+
+case class Recurse[Command[_], Result[_], E <: Effect[E], T, S, S2](
+    h: ResultHandler[Command, Result, E, S],
+    v: T < (E & S & S2)
+)
+
+abstract class ResultHandler[Command[_], Result[_], E <: Effect[E], S]:
+  opaque type Handle[T, S2] >: (Result[T] < (S & S2)) = Result[T] < (S & S2) | Recurse[Command, Result, E, T, S, S2]
+
+  def handle[T, S2](h: ResultHandler[Command, Result, E, S], v: T < (E & S & S2)): Handle[T, S2] = Recurse(h, v)
