Fix constraint merging in FunProto

After the previous commit a few tests started failing due to TyperStates
attempting to instantiate a type variable with an owningState pointing
to a different TyperState. The issue is that after
`mergeConstraintWith`, multiple TyperState can own the same type
variable. This is fine as long as only one of them is committable since
the other ones won't attempt to instantiate any type variable, but
that's not necessarily the case in FunProto#typedArgs where we merge the
constraints of the FunProto context into the passed context.

This commit fixes this by instantiating any type variable in the
constraints of the FunProto which do not exist in the passed TyperState
before calling `mergeConstraintWith`. This ensures that merging can be
done without changing the ownership of any type variable, thus keeping
both TyperState safely committable.

Author: smarter

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

@@ -162,6 +162,11 @@ abstract class Constraint extends Showable {
    */
   def & (other: Constraint, otherHasErrors: Boolean)(using Context): Constraint
 
+  /** Whether `tl` is present in both `this` and `that` but is associated with
+   *  different TypeVars there, meaning that the constraints cannot be merged.
+   */
+  def hasConflictingTypeVarsFor(tl: TypeLambda, that: Constraint): Boolean
+
   /** Check that no constrained parameter contains itself as a bound */
   def checkNonCyclic()(using Context): this.type
 

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

@@ -493,6 +493,12 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
         merge(this.upperMap, that.upperMap, mergeParams))
   }.showing(i"constraint merge $this with $other = $result", constr)
 
+  def hasConflictingTypeVarsFor(tl: TypeLambda, that: Constraint): Boolean =
+    contains(tl) && that.contains(tl) &&
+    // Since TypeVars are allocated in bulk for each type lambda, we only have
+    // to check the first one to find out if some of them are different.
+    (this.typeVarOfParam(tl.paramRefs(0)) ne that.typeVarOfParam(tl.paramRefs(0)))
+
   def subst(from: TypeLambda, to: TypeLambda)(using Context): OrderingConstraint =
     def swapKey[T](m: ArrayValuedMap[T]) = m.remove(from).updated(to, m(from))
     var current = newConstraint(swapKey(boundsMap), swapKey(lowerMap), swapKey(upperMap))

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

@@ -82,7 +82,8 @@ class TyperState() {
   /** The set of uninstantiated type variables which have this state as their owning state.
    *
    *  Invariant:
-   *  `tstate.ownedVars.contains(tvar)` iff `tvar.owningState.get eq tstate`
+   *   if `tstate.isCommittable` then
+   *     `tstate.ownedVars.contains(tvar)` iff `tvar.owningState.get eq tstate`
    */
   private var myOwnedVars: TypeVars = _
   def ownedVars: TypeVars = myOwnedVars
@@ -138,6 +139,7 @@ class TyperState() {
   def commit()(using Context): Unit = {
     Stats.record("typerState.commit")
     assert(isCommittable)
+    setCommittable(false)
     val targetState = ctx.typerState
     if constraint ne targetState.constraint then
       Stats.record("typerState.commit.new constraint")
@@ -157,12 +159,7 @@ class TyperState() {
    *  in this constraint and its predecessors where necessary.
    */
   def ensureNotConflicting(other: Constraint)(using Context): Unit =
-    def hasConflictingTypeVarsFor(tl: TypeLambda) =
-      constraint.typeVarOfParam(tl.paramRefs(0)) ne other.typeVarOfParam(tl.paramRefs(0))
-        // Note: Since TypeVars are allocated in bulk for each type lambda, we only
-        // have to check the first one to find out if some of them are different.
-    val conflicting = constraint.domainLambdas.find(tl =>
-      other.contains(tl) && hasConflictingTypeVarsFor(tl))
+    val conflicting = constraint.domainLambdas.find(constraint.hasConflictingTypeVarsFor(_, other))
     for tl <- conflicting do
       val tl1 = constraint.ensureFresh(tl)
       for case (tvar: TypeVar, pref1) <- tl.paramRefs.map(constraint.typeVarOfParam).lazyZip(tl1.paramRefs) do
@@ -172,6 +169,12 @@ class TyperState() {
         ts.constraint = ts.constraint.subst(tl, tl1)
         ts = ts.previous
 
+  /** Integrate the constraints from `that` into this TyperState.
+   *
+   *  @pre If `that` is committable, it must not contain any type variable which
+   *       does not exist in `this` (in other words, all its type variables must
+   *       be owned by a common parent of `this` and `that`).
+   */
   def mergeConstraintWith(that: TyperState)(using Context): Unit =
     that.ensureNotConflicting(constraint)
     constraint = constraint & (that.constraint, otherHasErrors = that.reporter.errorsReported)
@@ -180,7 +183,15 @@ class TyperState() {
     for tl <- constraint.domainLambdas do
       if constraint.isRemovable(tl) then constraint = constraint.remove(tl)
 
+  /** Take ownership of `tvar`.
+   *
+   *  @pre `tvar` is not owned by a committable TyperState. This ensures
+   *       each tvar can only be instantiated by one TyperState.
+   */
   private def includeVar(tvar: TypeVar)(using Context): Unit =
+    val oldState = tvar.owningState.get
+    assert(oldState == null || !oldState.isCommittable,
+      i"$this attempted to take ownership of $tvar which is already owned by committable $oldState")
     tvar.owningState = new WeakReference(this)
     ownedVars += tvar
 

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

@@ -979,8 +979,10 @@ trait Implicits:
       val result =
         result0 match {
           case result: SearchSuccess =>
-            result.tstate.commit()
-            ctx.gadt.restore(result.gstate)
+            if result.tstate ne ctx.typerState then
+              result.tstate.commit()
+            if result.gstate ne ctx.gadt then
+              ctx.gadt.restore(result.gstate)
             if hasSkolem(false, result.tree) then
               report.error(SkolemInInferred(result.tree, pt, argument), ctx.source.atSpan(span))
             implicits.println(i"success: $result")

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

@@ -385,21 +385,46 @@ object ProtoTypes {
      *                before it is typed. The second Int parameter is the parameter index.
      */
     def typedArgs(norm: (untpd.Tree, Int) => untpd.Tree = sameTree)(using Context): List[Tree] =
-      if (state.typedArgs.size == args.length) state.typedArgs
-      else {
-        val prevConstraint = protoCtx.typerState.constraint
-
-        try
-          inContext(protoCtx) {
-            val args1 = args.mapWithIndexConserve((arg, idx) =>
-              cacheTypedArg(arg, arg => typer.typed(norm(arg, idx)), force = false))
-            if !args1.exists(arg => isUndefined(arg.tpe)) then state.typedArgs = args1
-            args1
-          }
-        finally
-          if (protoCtx.typerState.constraint ne prevConstraint)
-            ctx.typerState.mergeConstraintWith(protoCtx.typerState)
-      }
+      if state.typedArgs.size == args.length then state.typedArgs
+      else
+        val passedTyperState = ctx.typerState
+        inContext(protoCtx) {
+          val protoTyperState = protoCtx.typerState
+          val oldConstraint = protoTyperState.constraint
+          val args1 = args.mapWithIndexConserve((arg, idx) =>
+            cacheTypedArg(arg, arg => typer.typed(norm(arg, idx)), force = false))
+          val newConstraint = protoTyperState.constraint
+
+          if !args1.exists(arg => isUndefined(arg.tpe)) then state.typedArgs = args1
+
+          // We only need to propagate constraints if we typed the arguments in a different
+          // TyperState and if that created additional constraints.
+          if (passedTyperState ne protoTyperState) && (oldConstraint ne newConstraint) then
+            // To respect the pre-condition of `mergeConstraintWith` and keep
+            // `protoTyperState` committable we must ensure that it does not
+            // contain any type variable which don't already exist in the passed
+            // TyperState. This is achieved by instantiating any such type
+            // variable.
+            if protoTyperState.isCommittable then
+              val passedConstraint = passedTyperState.constraint
+              val newLambdas = newConstraint.domainLambdas.filter(tl =>
+                !passedConstraint.contains(tl) || passedConstraint.hasConflictingTypeVarsFor(tl, newConstraint))
+              val newTvars = newLambdas.flatMap(_.paramRefs).map(newConstraint.typeVarOfParam)
+
+              args1.foreach(arg => Inferencing.instantiateSelected(arg.tpe, newTvars))
+
+              // `instantiateSelected` can leave some type variables uninstantiated,
+              // so we maximize them in a second pass.
+              newTvars.foreach {
+                case tvar: TypeVar if !tvar.isInstantiated =>
+                  tvar.instantiate(fromBelow = false)
+                case _ =>
+              }
+
+            passedTyperState.mergeConstraintWith(protoTyperState)
+          end if
+          args1
+        }
 
     /** Type single argument and remember the unadapted result in `myTypedArg`.
      *  used to avoid repeated typings of trees when backtracking.