Track type variable dependencies to guide instantiation decisions

We now keep track of reverse type variable dependencies in constraints.
E.g. is a constraint contains a clause like

    A >: List[B]

We associate with `B` info that A depends co-variantly on it. Or, if

    A <: B => C

we associate with `B` that `A` depends co-variantly on it and with `C`
that `A` depends contra-variantly on it.

These dependencies are then used to guide type variable instantiation.
If an eligible type variable does not appear in the type of a typed expression,
we interpolate it to one of its bounds. Previously this was done in an ad-hoc
manner where we minimized the type variable if it had a lower bound and maximized
it otherwise. We now take reverse dependencies into account. If maximizing a type
variable would narrow the remaining constraint we minimize, and if minimizing
a type variable would narrow the remaining constraint we maximize. Only if
the type variable is not referred to from the remaining constraint we resort
to the old heuristic based on the lower bound.

Fixes scala#15864

Author: odersky

compiler/src/dotty/tools/dotc/config/Config.scala

@@ -184,6 +184,9 @@ object Config {
   /** If set, prints a trace of all symbol completions */
   inline val showCompletions = false
 
+  /** If set, show variable/variable reverse dependencoes when printing constraints. */
+  inline val showConstraintDeps = true
+
   /** If set, method results that are context functions are flattened by adding
    *  the parameters of the context function results to the methods themselves.
    *  This is an optimization that reduces closure allocations.

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

@@ -4,6 +4,7 @@ package core
 
 import Types._, Contexts._
 import printing.Showable
+import util.SimpleIdentityMap
 
 /** Constraint over undetermined type parameters. Constraints are built
  *  over values of the following types:
@@ -128,7 +129,7 @@ abstract class Constraint extends Showable {
 
   /** Is `tv` marked as hard in the constraint? */
   def isHard(tv: TypeVar): Boolean
-  
+
   /** The same as this constraint, but with `tv` marked as hard. */
   def withHard(tv: TypeVar)(using Context): This
 
@@ -165,6 +166,28 @@ abstract class Constraint extends Showable {
    */
   def hasConflictingTypeVarsFor(tl: TypeLambda, that: Constraint): Boolean
 
+  /** A map that associates type variables with all other type variables that
+   *  refer to them in their bounds covariantly, such that, if the type variable
+   *  is isntantiated to a larger type, the constraint would be narrowed.
+   */
+  def coDeps: Constraint.TypeVarDeps
+
+  /** A map that associates type variables with all other type variables that
+   *  refer to them in their bounds covariantly, such that, if the type variable
+   *  is isntantiated to a smaller type, the constraint would be narrowed.
+   */
+  def contraDeps: Constraint.TypeVarDeps
+
+  /** A string showing the `coDeps` and `contraDeps` maps */
+  def depsToString(using Context): String
+
+  /** Does the constraint restricted to variables outside `except` depend on `tv`
+   *  in the given direction `co`?
+   *  @param `co`  If true, test whether the constraint would change if the variable is made larger
+   *               otherwise, test whether the constraint would change if the variable is made smaller.
+   */
+  def dependsOn(tv: TypeVar, except: TypeVars, co: Boolean)(using Context): Boolean
+
   /** Check that no constrained parameter contains itself as a bound */
   def checkNonCyclic()(using Context): this.type
 
@@ -183,6 +206,10 @@ abstract class Constraint extends Showable {
   def checkConsistentVars()(using Context): Unit
 }
 
+object Constraint:
+  type TypeVarDeps = SimpleIdentityMap[TypeVar, TypeVars]
+
+
 /** When calling `Constraint#addLess(p1, p2, ...)`, the caller might end up
  *  unifying one parameter with the other, this enum lets `addLess` know which
  *  direction the unification will take.

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

@@ -10,6 +10,8 @@ import printing.Texts._
 import config.Config
 import config.Printers.constr
 import reflect.ClassTag
+import Constraint.TypeVarDeps
+import NameKinds.DepParamName
 import annotation.tailrec
 import annotation.internal.sharable
 import cc.{CapturingType, derivedCapturingType}
@@ -148,6 +150,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       else
         val result = new OrderingConstraint(boundsMap, lowerMap, upperMap, hardVars)
         if ctx.run != null then ctx.run.nn.recordConstraintSize(result, result.boundsMap.size)
+        result.coDeps = this.coDeps
+        result.contraDeps = this.contraDeps
         result
 
 // ----------- Basic indices --------------------------------------------------
@@ -219,6 +223,127 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
       if tvar == null then NoType
       else tvar
 
+// ------------- TypeVar dependencies -----------------------------------
+
+  var coDeps, contraDeps: TypeVarDeps = SimpleIdentityMap.empty
+
+  def dependsOn(tv: TypeVar, except: TypeVars, co: Boolean)(using Context): Boolean =
+    def test(deps: TypeVarDeps, lens: ConstraintLens[List[TypeParamRef]]) =
+      val tvdeps = deps(tv)
+      null != tvdeps && tvdeps.exists(!except.contains(_))
+      || lens(this, tv.origin.binder, tv.origin.paramNum).exists(
+            p => !except.contains(typeVarOfParam(p)))
+        //.showing(i"outer depends on $tv with ${tvdeps.toList}%, % = $result")
+    if co then test(coDeps, upperLens) else test(contraDeps, lowerLens)
+
+  private class Adjuster(tvar: TypeVar)(using Context) extends TypeTraverser:
+    var add: Boolean = compiletime.uninitialized
+
+    def update(deps: TypeVarDeps, referenced: TypeVar): TypeVarDeps =
+      val entry = deps(referenced)
+      val prev = if null == entry then SimpleIdentitySet.empty else entry
+      val now = if add then prev + tvar else prev - tvar
+      deps.updated(referenced, now)
+
+    def traverse(t: Type) = t match
+      case tv: TypeVar =>
+        val inst = tv.instanceOpt
+        if inst.exists then traverse(inst)
+        else
+          if variance >= 0 then coDeps = update(coDeps, tv)
+          if variance <= 0 then contraDeps = update(contraDeps, tv)
+      case param: TypeParamRef =>
+        traverse(typeVarOfParam(param))
+      case tp: LazyRef if !tp.completed =>
+      case _ =>
+        traverseChildren(t)
+
+  /** Adjust dependencies to account for the delta of previous entry `prevEntry`
+   *  and new bound `entry` for the type variable `tvar`.
+   */
+  def adjustDeps(entry: Type | Null, prevEntry: Type | Null, tvar: Type | Null)(using Context): this.type =
+    tvar match
+      case tvar: TypeVar =>
+        val adjuster = new Adjuster(tvar)
+
+        /** Adjust reverse depemdencies of all type variables referenced by `bound`
+         *  @param  isLower `bound` is a lower bound
+         *  @param  add     if true, add referenced variables to dependencoes, otherwise drop them.
+         */
+        def adjustReferenced(bound: Type, isLower: Boolean, add: Boolean) =
+          adjuster.variance = if isLower then 1 else -1
+          adjuster.add = add
+          adjuster.traverse(bound)
+
+        /** Use an optimized strategy to adjust dependencies to account for the delta
+         *  of previous bound `prevBound` and new bound `bound`: If `prevBound` is some
+         *  and/or prefix of `bound`, just add the new parts of `bound`.
+         *  @param  isLower `bound` and `prevBound` are lower bounds
+         */
+        def adjustDelta(bound: Type, prevBound: Type, isLower: Boolean): Boolean =
+          if bound eq prevBound then true
+          else bound match
+            case bound: AndOrType =>
+              adjustDelta(bound.tp1, prevBound, isLower) && {
+                adjustReferenced(bound.tp2, isLower, add = true)
+                true
+              }
+            case _ => false
+
+        /** Adjust dependencies to account for the delta of previous bound `prevBound`
+         *  and new bound `bound`.
+         *  @param  isLower `bound` and `prevBound` are lower bounds
+         */
+        def adjustBounds(bound: Type, prevBound: Type, isLower: Boolean) =
+          if !adjustDelta(bound, prevBound, isLower) then
+            adjustReferenced(prevBound, isLower, add = false)
+            adjustReferenced(bound, isLower, add = true)
+
+        entry match
+          case TypeBounds(lo, hi) =>
+            prevEntry match
+              case TypeBounds(plo, phi) =>
+                adjustBounds(lo, plo, isLower = true)
+                adjustBounds(hi, phi, isLower = false)
+              case _ =>
+                adjustReferenced(lo, isLower = true, add = true)
+                adjustReferenced(hi, isLower = false, add = true)
+          case _ =>
+            prevEntry match
+              case TypeBounds(plo, phi) =>
+                adjustReferenced(plo, isLower = true, add = false)
+                adjustReferenced(phi, isLower = false, add = false)
+              case _ =>
+            dropDeps(tvar)
+      case _ =>
+    this
+  end adjustDeps
+
+  /** Adjust dependencies to account for adding or dropping `entries` to the
+   *  constraint.
+   *  @param add   if true, entries is added, otherwise it is dropped
+   */
+  def adjustDeps(entries: Array[Type], add: Boolean)(using Context): this.type =
+    for n <- 0 until paramCount(entries) do
+      if add
+      then adjustDeps(entries(n), NoType, typeVar(entries, n))
+      else adjustDeps(NoType, entries(n), typeVar(entries, n))
+    this
+
+  /** If `tp` is a type variable, remove all its reverse dependencies */
+  def dropDeps(tp: Type)(using Context): Unit = tp match
+    case tv: TypeVar =>
+      coDeps = coDeps.remove(tv)
+      contraDeps = contraDeps.remove(tv)
+    case _ =>
+
+  /** A string representing the two depenecy maps */
+  def depsToString(using Context): String =
+    def depsStr(deps: SimpleIdentityMap[TypeVar, TypeVars]): String =
+      def depStr(tv: TypeVar) = i"$tv --> ${deps(tv).nn.toList}%, %"
+      if deps.isEmpty then "" else i"\n  ${deps.toList.map((k, v) => depStr(k))}%\n  %"
+    i"co-deps:${depsStr(coDeps)}\ncontra-deps:${depsStr(contraDeps)}\n"
+
 // ---------- Adding TypeLambdas --------------------------------------------------
 
   /** The bound type `tp` without constrained parameters which are clearly
@@ -286,6 +411,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     tvars.copyToArray(entries1, nparams)
     newConstraint(boundsMap = this.boundsMap.updated(poly, entries1))
       .init(poly)
+      .adjustDeps(entries1, add = true)
   }
 
   /** Split dependent parameters off the bounds for parameters in `poly`.
@@ -432,6 +558,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
   private def updateEntry(current: This, param: TypeParamRef, tp: Type)(using Context): This = {
     if Config.checkNoWildcardsInConstraint then assert(!tp.containsWildcardTypes)
     var current1 = boundsLens.update(this, current, param, tp)
+    current1.adjustDeps(tp, current.entry(param), typeVarOfParam(param))
     tp match {
       case TypeBounds(lo, hi) =>
         for p <- dependentParams(lo, isUpper = false) do
@@ -471,10 +598,15 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
         current.ensureNonCyclic(atPoly.paramRefs(atIdx), tp.substParam(param, replacement))
 
       current.foreachParam { (p, i) =>
-        current = boundsLens.map(this, current, p, i, replaceParam(_, p, i))
+        current = boundsLens.map(this, current, p, i,
+          entry =>
+            val newEntry = replaceParam(entry, p, i)
+            adjustDeps(newEntry, entry, typeVar(this.boundsMap(p).nn, i))
+            newEntry)
         current = lowerLens.map(this, current, p, i, removeParam)
         current = upperLens.map(this, current, p, i, removeParam)
       }
+      current.dropDeps(typeVarOfParam(param))
       current.checkNonCyclic()
   end replace
 
@@ -489,6 +621,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     val hardVars1 = pt.paramRefs.foldLeft(hardVars)((hvs, param) => hvs - typeVarOfParam(param))
     newConstraint(boundsMap.remove(pt), removeFromOrdering(lowerMap), removeFromOrdering(upperMap), hardVars1)
       .checkNonCyclic()
+      .adjustDeps(boundsMap(pt).nn, add = false)
   }
 
   def isRemovable(pt: TypeLambda): Boolean = {
@@ -666,13 +799,16 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     val constrainedText =
       " constrained types = " + domainLambdas.mkString("\n")
     val boundsText =
-      " bounds = " + {
+      "\n bounds = " + {
         val assocs =
           for (param <- domainParams)
           yield
             s"${param.binder.paramNames(param.paramNum)}: ${entryText(entry(param))}"
         assocs.mkString("\n")
     }
-    constrainedText + "\n" + boundsText
+    val depsText =
+      "\n coDeps = " + coDeps +
+      "\n contraDeps = " + contraDeps
+    constrainedText + boundsText + depsText
   }
 }

compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala

@@ -689,8 +689,9 @@ class PlainPrinter(_ctx: Context) extends Printer {
                 Text(ups.map(toText), ", ")
           Text(deps, "\n")
         }
+      val depsText = if Config.showConstraintDeps then c.depsToString else ""
       //Printer.debugPrintUnique = false
-      Text.lines(List(uninstVarsText, constrainedText, boundsText, orderingText))
+      Text.lines(List(uninstVarsText, constrainedText, boundsText, orderingText, depsText))
     finally
       ctx.typerState.constraint = savedConstraint
 

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

@@ -6,15 +6,14 @@ import core._
 import ast._
 import Contexts._, Types._, Flags._, Symbols._
 import ProtoTypes._
-import NameKinds.{AvoidNameKind, UniqueName}
+import NameKinds.UniqueName
 import util.Spans._
-import util.{Stats, SimpleIdentityMap, SrcPos}
+import util.{Stats, SimpleIdentityMap, SimpleIdentitySet, SrcPos}
 import Decorators._
 import config.Printers.{gadts, typr}
 import annotation.tailrec
 import reporting._
 import collection.mutable
-
 import scala.annotation.internal.sharable
 
 object Inferencing {
@@ -619,7 +618,7 @@ trait Inferencing { this: Typer =>
         if state.reporter.hasUnreportedErrors then return tree
 
         def constraint = state.constraint
-        type InstantiateQueue = mutable.ListBuffer[(TypeVar, Boolean)]
+        type InstantiateQueue = mutable.ListBuffer[(TypeVar, Int)]
         val toInstantiate = new InstantiateQueue
         for tvar <- qualifying do
           if !tvar.isInstantiated && constraint.contains(tvar) && tvar.nestingLevel >= ctx.nestingLevel then
@@ -628,24 +627,9 @@ trait Inferencing { this: Typer =>
             // instantiated `tvar` through unification.
             val v = vs(tvar)
             if v == null then
-              // Even though `tvar` is non-occurring in `v`, the specific
-              // instantiation we pick still matters because `tvar` might appear
-              // in the bounds of a non-`qualifying` type variable in the
-              // constraint.
-              // In particular, if `tvar` was created as the upper or lower
-              // bound of an existing variable by `LevelAvoidMap`, we
-              // instantiate it in the direction corresponding to the
-              // original variable which might be further constrained later.
-              // Otherwise, we simply rely on `hasLowerBound`.
-              val name = tvar.origin.paramName
-              val fromBelow =
-                name.is(AvoidNameKind.UpperBound) ||
-                !name.is(AvoidNameKind.LowerBound) && tvar.hasLowerBound
-              typr.println(i"interpolate non-occurring $tvar in $state in $tree: $tp, fromBelow = $fromBelow, $constraint")
-              toInstantiate += ((tvar, fromBelow))
+              toInstantiate += ((tvar, 0))
             else if v.intValue != 0 then
-              typr.println(i"interpolate $tvar in $state in $tree: $tp, fromBelow = ${v.intValue == 1}, $constraint")
-              toInstantiate += ((tvar, v.intValue == 1))
+              toInstantiate += ((tvar, v.intValue))
             else comparing(cmp =>
               if !cmp.levelOK(tvar.nestingLevel, ctx.nestingLevel) then
                 // Invariant: The type of a tree whose enclosing scope is level
@@ -686,10 +670,23 @@ trait Inferencing { this: Typer =>
          *      V2 := V3, O2 := O3
          */
         def doInstantiate(buf: InstantiateQueue): Unit =
+          val varsToInstantiate = buf.foldLeft(SimpleIdentitySet.empty: TypeVars) {
+            case (tvs, (tv, _)) => tvs + tv
+          }
           if buf.nonEmpty then
             val suspended = new InstantiateQueue
             while buf.nonEmpty do
-              val first @ (tvar, fromBelow) = buf.head
+              val first @ (tvar, v) = buf.head
+              val fromBelow =
+                if v == 0 then
+                  val aboveOK = !constraint.dependsOn(tvar, varsToInstantiate, co = true)
+                  val belowOK = !constraint.dependsOn(tvar, varsToInstantiate, co = false)
+                  if aboveOK == belowOK then tvar.hasLowerBound
+                  else belowOK
+                else
+                  v == 1
+              typr.println(
+                i"interpolate${if v == 0 then "non-occurring" else ""} $tvar in $state in $tree: $tp, fromBelow = $fromBelow, $constraint")
               buf.dropInPlace(1)
               if !tvar.isInstantiated then
                 val suspend = buf.exists{ (following, _) =>

tests/pos/i15864.scala

@@ -0,0 +1,22 @@
+object Test:
+  def op[O, P](ta: List[O], tb: List[P]): List[P] = ???
+
+  class Graph { class Node }
+
+  def outsQ(using g: Graph): List[List[g.Node]] = ???
+
+  object aGraph extends Graph
+  given implA: aGraph.type = aGraph
+
+  val q1: List[List[aGraph.Node]] = op(outsQ, op(outsQ, outsQ))
+  implicitly[q1.type <:< List[List[aGraph.Node]]]
+
+  val a1 = outsQ
+  val a2 = op(outsQ, outsQ)
+  val a3 = op(a1, a2)
+
+  val q2 = op(outsQ, op(outsQ, outsQ))
+  val q3: List[List[aGraph.Node]] = q2
+  implicitly[q2.type <:< List[List[aGraph.Node]]]
+
+