From 2863a29f9e6c7685bd0bf632fa02b322f59ce99f Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 11 Apr 2024 12:31:17 +0200 Subject: [PATCH] Avoid the TypeVar.inst trap `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 --- .../tools/dotc/core/OrderingConstraint.scala | 12 +++---- .../tools/dotc/core/SymDenotations.scala | 2 +- .../dotty/tools/dotc/core/TypeComparer.scala | 10 +++--- .../dotty/tools/dotc/core/TyperState.scala | 6 ++-- .../src/dotty/tools/dotc/core/Types.scala | 31 ++++++++++++------- tests/pos/i20154.scala | 15 +++++++++ 6 files changed, 50 insertions(+), 26 deletions(-) create mode 100644 tests/pos/i20154.scala diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index dd2319ed508b..8256a3cdbab1 100644 --- 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 index 7536e4bd76ef..bfaaf78883ae 100644 --- 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 index a9b5a39c2a62..cee1ec7fffa8 100644 --- 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 index ef7329c3698d..160d7749de61 100644 --- 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 index 3c9f7e05b6e2..ba48b6a0f2e6 100644 --- 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,11 +4934,15 @@ 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 @@ -4946,8 +4950,8 @@ object Types extends TypeUtils { 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,10 +4989,15 @@ 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 @@ -4996,7 +5005,7 @@ object Types extends TypeUtils { 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 new file mode 100644 index 000000000000..17dc41be7011 --- /dev/null +++ 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)