Handle TypeVar instantiation

Since GADTMap is re-initialised each time it is used, it's not necessary
to care about avoiding instantiations for shared TypeVars.

Author: Aleksander Boruch-Gruszecki

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

@@ -713,16 +713,27 @@ object Contexts {
     def addBound(sym: Symbol, bound: Type, isUpper: Boolean)(implicit ctx: Context): Boolean
     def bounds(sym: Symbol)(implicit ctx: Context): TypeBounds
     def contains(sym: Symbol)(implicit ctx: Context): Boolean
+    def debugBoundsDescription(implicit ctx: Context): String
     def derived: GADTMap
   }
 
   final class SmartGADTMap(
     private[this] var myConstraint: Constraint = new OrderingConstraint(SimpleIdentityMap.Empty, SimpleIdentityMap.Empty, SimpleIdentityMap.Empty),
     private[this] var mapping: SimpleIdentityMap[Symbol, TypeVar] = SimpleIdentityMap.Empty,
-    private[this] var reverseMapping: SimpleIdentityMap[TypeVar, Symbol] = SimpleIdentityMap.Empty
+    private[this] var reverseMapping: SimpleIdentityMap[TypeParamRef, Symbol] = SimpleIdentityMap.Empty
   ) extends GADTMap with ConstraintHandling {
     import dotty.tools.dotc.config.Printers.gadts
 
+    override def debugBoundsDescription(implicit ctx: Context): String = {
+      val sb = new mutable.StringBuilder
+      sb ++= constraint.show
+      sb += '\n'
+      mapping.foreachBinding { case (sym, _) =>
+        sb ++= i"$sym: ${bounds(sym)}\n"
+      }
+      sb.result
+    }
+
     // TODO: dirty kludge - should this class be an inner class of TyperState instead?
     private[this] var myCtx: Context = null
     implicit override def ctx = myCtx
@@ -750,13 +761,12 @@ object Contexts {
             val poly = PolyType(DepParamName.fresh(sym.name.toTypeName) :: Nil)(
               pt => TypeBounds.empty :: Nil,
               pt => defn.AnyType)
-            // null out creatorState as a precaution
             new TypeVar(poly.paramRefs.head, creatorState = null)
           }
           gadts.println(i"GADTMap: created tvar $sym -> $res")
           constraint = constraint.add(res.origin.binder, res :: Nil)
           mapping = mapping.updated(sym, res)
-          reverseMapping = reverseMapping.updated(res, sym)
+          reverseMapping = reverseMapping.updated(res.origin, sym)
           res
       }
       res
@@ -765,26 +775,93 @@ object Contexts {
     override def addEmptyBounds(sym: Symbol)(implicit ctx: Context): Unit = tvar(sym)
 
     override def addBound(sym: Symbol, bound: Type, isUpper: Boolean)(implicit ctx: Context): Boolean = inCtx(ctx) {
+      @annotation.tailrec def stripInst(tp: Type): Type = tp match {
+        case tv: TypeVar =>
+          val inst = tv.inst orElse instType(tv)
+          if (inst.exists) stripInst(inst) else tv
+        case _ => tp
+      }
+
+      def cautiousSubtype(tp1: Type, tp2: Type, isSubtype: Boolean, allowNarrowing: Boolean = false): Boolean = {
+        val externalizedTp1 = (new TypeVarRemovingMap)(tp1)
+        val externalizedTp2 = (new TypeVarRemovingMap)(tp2)
+
+        def descr = {
+          def op = s"frozen_${if (isSubtype) "<:<" else ">:>"}"
+          def flex = s"GADTFlexible=$allowNarrowing"
+          i"$tp1 $op $tp2\n\t$externalizedTp1 $op $externalizedTp2 ($flex)"
+        }
+        // gadts.println(descr)
+
+        val outerCtx = ctx
+        val res =  {
+          implicit val ctx : Context =
+            if (allowNarrowing) outerCtx else outerCtx.fresh.retractMode(Mode.GADTflexible)
+
+          // TypeComparer.explain[Boolean](gadts.println) { implicit ctx =>
+          if (isSubtype) externalizedTp1 frozen_<:< externalizedTp2
+          else externalizedTp2 frozen_<:< externalizedTp1
+          // }
+        }
+        gadts.println(i"$descr = $res")
+        res
+      }
+
+      val symTvar: TypeVar = stripInst(tvar(sym)) match {
+        case tv: TypeVar => tv
+        case inst =>
+          gadts.println(i"instantiated: $sym -> $inst")
+          return cautiousSubtype(inst, bound, isSubtype = isUpper, allowNarrowing = true)
+      }
+
+      def doAddBound(bound: Type): Boolean = {
+        val res = stripInst(bound) match {
+          case boundTvar: TypeVar =>
+            if (boundTvar eq symTvar) true
+            else if (isUpper) addLess(symTvar.origin, boundTvar.origin)
+            else addLess(boundTvar.origin, symTvar.origin)
+          case bound =>
+            if (cautiousSubtype(symTvar, bound, isSubtype = !isUpper)) { instantiate(symTvar, bound); true }
+            else if (isUpper) addUpperBound(symTvar.origin, bound)
+            else addLowerBound(symTvar.origin, bound)
+        }
+
+        vacuum()
+        res
+      }
+
       def isEmptyBounds(tp: Type) = tp match {
         case TypeBounds(lo, hi) => (lo eq defn.NothingType) && (hi eq defn.AnyType)
         case _ => false
       }
 
-      val symTvar = tvar(sym)
-
-      def doAddOrdering(bound: TypeParamRef) =
-        if (isUpper) addLess(symTvar.origin, bound) else addLess(bound, symTvar.origin)
+      def instantiate(tv: TypeVar, tp: Type): Unit = {
+        // instantiating one TypeVar to another makes us actually lose information
+        // that is, we need to know that both TypeVars are equal to another
+        //   *and* be able to record further bounds on either one
+        if (tp.isInstanceOf[TypeVar]) return
+        val externalizedTp = (new TypeVarRemovingMap)(tp)
+        gadts.println(i"instantiating $tv to $externalizedTp    ( $tp )")
+        tv.inst = externalizedTp
+        constraint = constraint.replace(tv.origin, externalizedTp)
+      }
 
-      def doAddBound(bound: Type) =
-        if (isUpper) addUpperBound(symTvar.origin, bound) else addLowerBound(symTvar.origin, bound)
+      def vacuum(): Unit = {
+        constraint.foreachTypeVar { tv =>
+          if (!tv.inst.exists) {
+            val inst = instType(tv)
+            if (inst.exists) instantiate(tv, inst)
+          }
+        }
+      }
 
       val tvarBound = (new TypeVarInsertingMap)(bound)
       val res = tvarBound match {
         case boundTvar: TypeVar =>
-          if (boundTvar eq symTvar) true else doAddOrdering(boundTvar.origin)
+          doAddBound(boundTvar)
         // hack to normalize T and T[_]
         case AppliedType(boundTvar: TypeVar, args) if args forall isEmptyBounds =>
-          doAddOrdering(boundTvar.origin)
+          doAddBound(boundTvar)
         case tp => doAddBound(tp)
       }
 
@@ -800,12 +877,10 @@ object Contexts {
       mapping(sym) match {
         case null => null
         case tv =>
-          val tb = constraint.fullBounds(tv.origin)
-          val res = {
-            val tm = new TypeVarRemovingMap
-            tb.derivedTypeBounds(tm(tb.lo), tm(tb.hi))
-          }
-          gadts.println(i"gadt bounds $sym: $res\t( $tv: $tb )")
+          val tb =
+            if (tv.inst.exists) TypeAlias(tv.inst) else constraint.fullBounds(tv.origin)
+          val res = (new TypeVarRemovingMap)(tb).asInstanceOf[TypeBounds]
+          // gadts.println(i"gadt bounds $sym: $res\t( $tv: $tb )")
           res
       }
     }
@@ -831,17 +906,15 @@ object Contexts {
     private final class TypeVarRemovingMap extends TypeMap {
       override def apply(tp: Type): Type = tp match {
         case tpr: TypeParamRef =>
-          constraint.typeVarOfParam(tpr) match {
-            case tv: TypeVar =>
-              reverseMapping(tv).typeRef
-            case unexpected =>
-              // if we didn't get a TypeVar, it's likely to cause problems
-              gadts.println(i"GADTMap: unexpected typeVarOfParam($tpr) = `$unexpected` ${unexpected.getClass}")
-              tpr
+          reverseMapping(tpr) match {
+            case null => tpr
+            case sym => sym.typeRef
           }
         case tv: TypeVar =>
-          if (reverseMapping.contains(tv)) reverseMapping(tv).typeRef
-          else tv
+          reverseMapping(tv.origin) match {
+            case null => tv
+            case sym => sym.typeRef
+          }
         case _ =>
           mapOver(tp)
       }
@@ -853,6 +926,7 @@ object Contexts {
     override def addBound(sym: Symbol, bound: Type, isUpper: Boolean)(implicit ctx: Context): Boolean = unsupported("EmptyGADTMap.addBound")
     override def bounds(sym: Symbol)(implicit ctx: Context): TypeBounds = null
     override def contains(sym: Symbol)(implicit ctx: Context) = false
+    override def debugBoundsDescription(implicit ctx: Context): String = "EmptyGADTMap"
     override def derived = new SmartGADTMap
   }
 }

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

@@ -325,8 +325,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
   private def order(current: This, param1: TypeParamRef, param2: TypeParamRef)(implicit ctx: Context): This =
     if (param1 == param2 || current.isLess(param1, param2)) this
     else {
-      assert(contains(param1))
-      assert(contains(param2))
+      assert(contains(param1), i"$param1")
+      assert(contains(param2), i"$param2")
       val newUpper = param2 :: exclusiveUpper(param2, param1)
       val newLower = param1 :: exclusiveLower(param1, param2)
       val current1 = (current /: newLower)(upperLens.map(this, _, _, newUpper ::: _))

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

@@ -3529,7 +3529,7 @@ object Types {
     private[core] def inst: Type = myInst
     private[core] def inst_=(tp: Type): Unit = {
       myInst = tp
-      if (tp.exists) {
+      if (tp.exists && (owningState ne null)) {
         owningState.get.ownedVars -= this
         owningState = null // no longer needed; null out to avoid a memory leak
       }
@@ -3538,13 +3538,14 @@ object Types {
     /** The state owning the variable. This is at first `creatorState`, but it can
      *  be changed to an enclosing state on a commit.
      */
-    private[core] var owningState: WeakReference[TyperState] = new WeakReference(creatorState)
+    private[core] var owningState: WeakReference[TyperState] =
+      if (creatorState == null) null else new WeakReference(creatorState)
 
     /** The instance type of this variable, or NoType if the variable is currently
      *  uninstantiated
      */
     def instanceOpt(implicit ctx: Context): Type =
-      if (inst.exists) inst else ctx.typerState.instType(this)
+      if (inst.exists) inst else ctx.typeComparer.instType(this)
 
     /** Is the variable already instantiated? */
     def isInstantiated(implicit ctx: Context): Boolean = instanceOpt.exists

compiler/src/dotty/tools/dotc/typer/Inferencing.scala

@@ -206,7 +206,11 @@ object Inferencing {
     }
 
     val widePt = if (ctx.scala2Mode || refinementIsInvariant(tp)) pt else widenVariantParams(pt)
-    tp <:< widePt
+    import dotty.tools.dotc.config.Printers.gadts
+    gadts.println(i"constraining pattern type $tp <:< $widePt")
+    val res = tp <:< widePt
+    gadts.println(i"constrained pattern type $tp <:< $widePt = $res\n${ctx.gadt.debugBoundsDescription}")
+    res
   }
 
   /** The list of uninstantiated type variables bound by some prefix of type `T` which

tests/gadt/one.scala renamed to tests/gadt/i4075.scala

@@ -1,9 +1,12 @@
-object one {
+object i4075 {
   case class One[T](fst: T)
   def bad[T](e: One[T]) = e match {
     case foo: One[a] =>
       val t: T = e.fst
       // val nok: Nothing = t // should not compile
       val ok: a = t // does compile
+      One(ok)
   }
+
+  val one: One[Int] = bad(One(0))
 }

tests/gadt/i4471.scala

@@ -0,0 +1,31 @@
+object i4471 {
+  sealed trait Shuffle[A1, A2] {
+    def andThen[A3](that: Shuffle[A2, A3]): Shuffle[A1, A3] = AndThen(this, that)
+  }
+
+  case class Id[A]() extends Shuffle[A, A]
+  case class Swap[A, B]() extends Shuffle[(A, B), (B, A)]
+  case class AssocLR[A, B, C]() extends Shuffle[((A, B), C), (A, (B, C))]
+  case class AssocRL[A, B, C]() extends Shuffle[(A, (B, C)), ((A, B), C)]
+  case class Par[A1, B1, A2, B2](_1: Shuffle[A1, B1], _2: Shuffle[A2, B2]) extends Shuffle[(A1, A2), (B1, B2)]
+  case class AndThen[A1, A2, A3](_1: Shuffle[A1, A2], _2: Shuffle[A2, A3]) extends Shuffle[A1, A3]
+  
+  def rewrite3[A1, A2, A3, A4](
+    op1: Shuffle[A1, A2],
+    op2: Shuffle[A2, A3],
+    op3: Shuffle[A3, A4]
+  ): Option[Shuffle[A1, A4]] = (op1, op2, op3) match {
+    case (
+      _: Swap[x, y],
+      _: AssocRL[u, v, w],
+      op3_ : Par[p1, q1, p2, q2]
+    ) => op3_ match {
+      case Par(_: Swap[r, s], _: Id[p2_]) =>
+        Some(
+          AssocLR[v, w, u]() andThen Par(Id[v](), Swap[w, u]()) andThen AssocRL[v, u, w]()
+        )
+      case _ => None
+    }
+    case _ => None
+  }
+}