From 3d4275dcb1e339ccc7b01d639277b00104d836a6 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 24 Sep 2022 13:03:18 +0200 Subject: [PATCH 01/17] Streamline OrderingConstraint's newConstraint --- .../tools/dotc/core/OrderingConstraint.scala | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 1341fac7d735..d71c7f1f5a68 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -24,17 +24,6 @@ object OrderingConstraint { /** The type of `OrderingConstraint#lowerMap`, `OrderingConstraint#upperMap` */ type ParamOrdering = ArrayValuedMap[List[TypeParamRef]] - /** A new constraint with given maps and given set of hard typevars */ - private def newConstraint( - boundsMap: ParamBounds, lowerMap: ParamOrdering, upperMap: ParamOrdering, - hardVars: TypeVars)(using Context) : OrderingConstraint = - if boundsMap.isEmpty && lowerMap.isEmpty && upperMap.isEmpty then - empty - else - val result = new OrderingConstraint(boundsMap, lowerMap, upperMap, hardVars) - if ctx.run != null then ctx.run.nn.recordConstraintSize(result, result.boundsMap.size) - result - /** A lens for updating a single entry array in one of the three constraint maps */ abstract class ConstraintLens[T <: AnyRef: ClassTag] { def entries(c: OrderingConstraint, poly: TypeLambda): Array[T] | Null @@ -93,7 +82,7 @@ object OrderingConstraint { def entries(c: OrderingConstraint, poly: TypeLambda): Array[Type] | Null = c.boundsMap(poly) def updateEntries(c: OrderingConstraint, poly: TypeLambda, entries: Array[Type])(using Context): OrderingConstraint = - newConstraint(c.boundsMap.updated(poly, entries), c.lowerMap, c.upperMap, c.hardVars) + c.newConstraint(boundsMap = c.boundsMap.updated(poly, entries)) def initial = NoType } @@ -101,7 +90,7 @@ object OrderingConstraint { def entries(c: OrderingConstraint, poly: TypeLambda): Array[List[TypeParamRef]] | Null = c.lowerMap(poly) def updateEntries(c: OrderingConstraint, poly: TypeLambda, entries: Array[List[TypeParamRef]])(using Context): OrderingConstraint = - newConstraint(c.boundsMap, c.lowerMap.updated(poly, entries), c.upperMap, c.hardVars) + c.newConstraint(lowerMap = c.lowerMap.updated(poly, entries)) def initial = Nil } @@ -109,7 +98,7 @@ object OrderingConstraint { def entries(c: OrderingConstraint, poly: TypeLambda): Array[List[TypeParamRef]] | Null = c.upperMap(poly) def updateEntries(c: OrderingConstraint, poly: TypeLambda, entries: Array[List[TypeParamRef]])(using Context): OrderingConstraint = - newConstraint(c.boundsMap, c.lowerMap, c.upperMap.updated(poly, entries), c.hardVars) + c.newConstraint(upperMap = c.upperMap.updated(poly, entries)) def initial = Nil } @@ -148,6 +137,19 @@ class OrderingConstraint(private val boundsMap: ParamBounds, type This = OrderingConstraint + /** A new constraint with given maps and given set of hard typevars */ + def newConstraint( // !!! Dotty problem: Making newConstraint `private` causes -Ytest-pickler failure. + boundsMap: ParamBounds = this.boundsMap, + lowerMap: ParamOrdering = this.lowerMap, + upperMap: ParamOrdering = this.upperMap, + hardVars: TypeVars = this.hardVars)(using Context) : OrderingConstraint = + if boundsMap.isEmpty && lowerMap.isEmpty && upperMap.isEmpty then + empty + else + val result = new OrderingConstraint(boundsMap, lowerMap, upperMap, hardVars) + if ctx.run != null then ctx.run.nn.recordConstraintSize(result, result.boundsMap.size) + result + // ----------- Basic indices -------------------------------------------------- /** The number of type parameters in the given entry array */ @@ -282,7 +284,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds, val entries1 = new Array[Type](nparams * 2) poly.paramInfos.copyToArray(entries1, 0) tvars.copyToArray(entries1, nparams) - newConstraint(boundsMap.updated(poly, entries1), lowerMap, upperMap, hardVars).init(poly) + newConstraint(boundsMap = this.boundsMap.updated(poly, entries1)) + .init(poly) } /** Split dependent parameters off the bounds for parameters in `poly`. @@ -511,7 +514,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def swapKey[T](m: ArrayValuedMap[T]) = val info = m(from) if info == null then m else m.remove(from).updated(to, info) - var current = newConstraint(swapKey(boundsMap), swapKey(lowerMap), swapKey(upperMap), hardVars) + var current = newConstraint(swapKey(boundsMap), swapKey(lowerMap), swapKey(upperMap)) def subst[T <: Type](x: T): T = x.subst(from, to).asInstanceOf[T] current.foreachParam {(p, i) => current = boundsLens.map(this, current, p, i, subst) @@ -524,7 +527,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def isHard(tv: TypeVar) = hardVars.contains(tv) def withHard(tv: TypeVar)(using Context) = - newConstraint(boundsMap, lowerMap, upperMap, hardVars + tv) + newConstraint(hardVars = this.hardVars + tv) def instType(tvar: TypeVar): Type = entry(tvar.origin) match case _: TypeBounds => NoType From 614fe90b6d0bfef2336ef47dc5dd731c4e4a859c Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 24 Sep 2022 13:06:05 +0200 Subject: [PATCH 02/17] 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 #15864 --- .../src/dotty/tools/dotc/config/Config.scala | 3 + .../dotty/tools/dotc/core/Constraint.scala | 29 +++- .../tools/dotc/core/OrderingConstraint.scala | 142 +++++++++++++++++- .../tools/dotc/printing/PlainPrinter.scala | 3 +- .../dotty/tools/dotc/typer/Inferencing.scala | 41 +++-- tests/pos/i15864.scala | 22 +++ 6 files changed, 213 insertions(+), 27 deletions(-) create mode 100644 tests/pos/i15864.scala diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 17e3ec352e7c..bd9d0be5dda5 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/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. diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala index 07b6e71cdcc9..d1b690687318 100644 --- a/compiler/src/dotty/tools/dotc/core/Constraint.scala +++ b/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. diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index d71c7f1f5a68..55223fdb0f65 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/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 } } diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 3c83d681e716..6c4891ff2b18 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/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 diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala index 27b83e025cf9..c9a4f057f346 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/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, _) => diff --git a/tests/pos/i15864.scala b/tests/pos/i15864.scala new file mode 100644 index 000000000000..a097e690c8bd --- /dev/null +++ b/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]]] + + From 412f5f317e2c35e328703b78262fd5c1b9a017c3 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 24 Sep 2022 13:18:16 +0200 Subject: [PATCH 03/17] Don't interpolate type variables if doing so would narrow the constraint --- .../src/dotty/tools/dotc/config/Config.scala | 2 + .../dotty/tools/dotc/core/Constraint.scala | 2 +- .../tools/dotc/core/OrderingConstraint.scala | 24 ++++-- .../dotty/tools/dotc/typer/Inferencing.scala | 78 +++++++++++++------ 4 files changed, 74 insertions(+), 32 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index bd9d0be5dda5..723ab53a500d 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -22,6 +22,8 @@ object Config { */ inline val checkConstraintsNonCyclic = false + inline val checkConstraintDeps = false + /** Check that each constraint resulting from a subtype test * is satisfiable. Also check that a type variable instantiation * satisfies its constraints. diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala index d1b690687318..d4b54eb1e2c2 100644 --- a/compiler/src/dotty/tools/dotc/core/Constraint.scala +++ b/compiler/src/dotty/tools/dotc/core/Constraint.scala @@ -189,7 +189,7 @@ abstract class Constraint extends Showable { 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 + def checkWellFormed()(using Context): this.type /** Does `param` occur at the toplevel in `tp` ? * Toplevel means: the type itself or a factor in some diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 55223fdb0f65..ecf911382e2c 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -433,7 +433,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, todos.dropInPlace(1) i += 1 } - current.checkNonCyclic() + current.checkWellFormed() } // ---------- Updates ------------------------------------------------------------ @@ -572,10 +572,10 @@ class OrderingConstraint(private val boundsMap: ParamBounds, /** The public version of `updateEntry`. Guarantees that there are no cycles */ def updateEntry(param: TypeParamRef, tp: Type)(using Context): This = - updateEntry(this, param, ensureNonCyclic(param, tp)).checkNonCyclic() + updateEntry(this, param, ensureNonCyclic(param, tp)).checkWellFormed() def addLess(param1: TypeParamRef, param2: TypeParamRef, direction: UnificationDirection)(using Context): This = - order(this, param1, param2, direction).checkNonCyclic() + order(this, param1, param2, direction).checkWellFormed() // ---------- Replacements and Removals ------------------------------------- @@ -585,7 +585,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, */ def replace(param: TypeParamRef, tp: Type)(using Context): OrderingConstraint = val replacement = tp.dealiasKeepAnnots.stripTypeVar - if param == replacement then this.checkNonCyclic() + if param == replacement then this.checkWellFormed() else assert(replacement.isValueTypeOrLambda) var current = @@ -607,7 +607,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, current = upperLens.map(this, current, p, i, removeParam) } current.dropDeps(typeVarOfParam(param)) - current.checkNonCyclic() + current.checkWellFormed() end replace def remove(pt: TypeLambda)(using Context): This = { @@ -620,8 +620,8 @@ 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) + .checkWellFormed() } def isRemovable(pt: TypeLambda): Boolean = { @@ -655,7 +655,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, current = upperLens.map(this, current, p, i, _.map(subst)) } constr.println(i"renamed $this to $current") - current.checkNonCyclic() + current.checkWellFormed() def isHard(tv: TypeVar) = hardVars.contains(tv) @@ -739,7 +739,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, // ---------- Checking ----------------------------------------------- - def checkNonCyclic()(using Context): this.type = + def checkWellFormed()(using Context): this.type = if Config.checkConstraintsNonCyclic then domainParams.foreach { param => val inst = entry(param) @@ -748,6 +748,14 @@ class OrderingConstraint(private val boundsMap: ParamBounds, assert(!occursAtToplevel(param, inst), s"cyclic bound for $param: ${inst.show} in ${this.show}") } + if Config.checkConstraintDeps then + def checkDeps(deps: TypeVarDeps) = + deps.foreachBinding { (tv, tvs) => + for tv1 <- tvs do + assert(!tv1.instanceOpt.exists, i"$this") + } + checkDeps(coDeps) + checkDeps(contraDeps) this def occursAtToplevel(param: TypeParamRef, inst: Type)(using Context): Boolean = diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala index c9a4f057f346..a940bc0f207f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala @@ -640,6 +640,31 @@ trait Inferencing { this: Typer => typr.println(i"no interpolation for nonvariant $tvar in $state") ) + def typeVarsIn(xs: collection.Seq[(TypeVar, Int)]): TypeVars = + xs.foldLeft(SimpleIdentitySet.empty: TypeVars)((tvs, tvi) => tvs + tvi._1) + + def filterByDeps(tvs0: List[(TypeVar, Int)]): List[(TypeVar, Int)] = { + val excluded = typeVarsIn(tvs0) + def step(tvs: List[(TypeVar, Int)]): List[(TypeVar, Int)] = tvs match + case tvs @ (hd @ (tvar, v)) :: tvs1 => + def aboveOK = !constraint.dependsOn(tvar, excluded, co = true) + def belowOK = !constraint.dependsOn(tvar, excluded, co = false) + if v == 0 && !aboveOK then + step((tvar, 1) :: tvs1) + else if v == 0 && !belowOK then + step((tvar, -1) :: tvs1) + else if v == -1 && !aboveOK || v == 1 && !belowOK then + println(i"drop $tvar, $v in $tp, $pt, qualifying = ${qualifying.toList}, tvs0 = ${tvs0.toList}%, %, excluded = ${excluded.toList}, $constraint") + step(tvs1) + else + tvs.derivedCons(hd, step(tvs1)) + case Nil => + Nil + val tvs1 = step(tvs0) + if tvs1 eq tvs0 then tvs1 else filterByDeps(tvs1) + }//.showing(i"filter $tvs0 in $constraint = $result") + end filterByDeps + /** Instantiate all type variables in `buf` in the indicated directions. * If a type variable A is instantiated from below, and there is another * type variable B in `buf` that is known to be smaller than A, wait and @@ -669,38 +694,45 @@ 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, v) = buf.head + def doInstantiate(tvs: List[(TypeVar, Int)]): Unit = + def excluded = typeVarsIn(tvs) + def tryInstantiate(tvs: List[(TypeVar, Int)]): List[(TypeVar, Int)] = tvs match + case (hd @ (tvar, v)) :: tvs1 => val fromBelow = if v == 0 then - val aboveOK = !constraint.dependsOn(tvar, varsToInstantiate, co = true) - val belowOK = !constraint.dependsOn(tvar, varsToInstantiate, co = false) + val aboveOK = true // !constraint.dependsOn(tvar, excluded, co = true, track = true) + val belowOK = true // !constraint.dependsOn(tvar, excluded, co = false, track = true) + assert(aboveOK, i"$tvar, excluded = ${excluded.toList}, $constraint") + assert(belowOK, i"$tvar, excluded = ${excluded.toList}, $constraint") 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, _) => - if fromBelow then - constraint.isLess(following.origin, tvar.origin) - else - constraint.isLess(tvar.origin, following.origin) + i"interpolate${if v == 0 then " non-occurring" else ""} $tvar in $state in $tree: $tp, fromBelow = $fromBelow, $constraint") + if tvar.isInstantiated then + tryInstantiate(tvs1) + else + val suspend = tvs1.exists{ (following, _) => + if fromBelow + then constraint.isLess(following.origin, tvar.origin) + else constraint.isLess(tvar.origin, following.origin) } - if suspend then suspended += first else tvar.instantiate(fromBelow) - end if - end while - doInstantiate(suspended) + if suspend then + typr.println(i"suspended: $hd") + hd :: tryInstantiate(tvs1) + else + tvar.instantiate(fromBelow) + tryInstantiate(tvs1) + case Nil => Nil + if tvs.nonEmpty then doInstantiate(tryInstantiate(tvs)) end doInstantiate - doInstantiate(toInstantiate) + val toInst = toInstantiate.toList + if toInst.nonEmpty then + typr.println(i"interpolating $toInst for $tp/$pt in $constraint") + val filtered = filterByDeps(toInst) + typr.println(i"filtered $filtered") + doInstantiate(filtered) } } tree From 572dcfd2500bcd088643d96148dbb87d834fb549 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 24 Sep 2022 13:19:11 +0200 Subject: [PATCH 04/17] Change dependency tracking to relation on TypeParams --- .../dotty/tools/dotc/core/Constraint.scala | 21 +- .../tools/dotc/core/OrderingConstraint.scala | 192 +++++++++--------- .../dotty/tools/dotc/typer/Inferencing.scala | 37 ++-- 3 files changed, 124 insertions(+), 126 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala index d4b54eb1e2c2..de81770460c3 100644 --- a/compiler/src/dotty/tools/dotc/core/Constraint.scala +++ b/compiler/src/dotty/tools/dotc/core/Constraint.scala @@ -4,7 +4,7 @@ package core import Types._, Contexts._ import printing.Showable -import util.SimpleIdentityMap +import util.{SimpleIdentitySet, SimpleIdentityMap} /** Constraint over undetermined type parameters. Constraints are built * over values of the following types: @@ -166,17 +166,17 @@ 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. + /** A map that associates type parameters of this constraint with all other type + * parameters that refer to them in their bounds covariantly, such that, if the + * type parameter is instantiated to a larger type, the constraint would be narrowed. */ - def coDeps: Constraint.TypeVarDeps + def coDeps: Constraint.ReverseDeps - /** 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. + /** A map that associates type parameters of this constraint with all other type + * parameters that refer to them in their bounds covariantly, such that, if the + * type parameter is instantiated to a smaller type, the constraint would be narrowed. */ - def contraDeps: Constraint.TypeVarDeps + def contraDeps: Constraint.ReverseDeps /** A string showing the `coDeps` and `contraDeps` maps */ def depsToString(using Context): String @@ -207,8 +207,7 @@ abstract class Constraint extends Showable { } object Constraint: - type TypeVarDeps = SimpleIdentityMap[TypeVar, TypeVars] - + type ReverseDeps = SimpleIdentityMap[TypeParamRef, SimpleIdentitySet[TypeParamRef]] /** When calling `Constraint#addLess(p1, p2, ...)`, the caller might end up * unifying one parameter with the other, this enum lets `addLess` know which diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index ecf911382e2c..2d30b970c973 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -10,8 +10,7 @@ import printing.Texts._ import config.Config import config.Printers.constr import reflect.ClassTag -import Constraint.TypeVarDeps -import NameKinds.DepParamName +import Constraint.ReverseDeps import annotation.tailrec import annotation.internal.sharable import cc.{CapturingType, derivedCapturingType} @@ -223,124 +222,120 @@ class OrderingConstraint(private val boundsMap: ParamBounds, if tvar == null then NoType else tvar -// ------------- TypeVar dependencies ----------------------------------- +// ------------- Type parameter dependencies ---------------------------------------- - var coDeps, contraDeps: TypeVarDeps = SimpleIdentityMap.empty + var coDeps, contraDeps: ReverseDeps = 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))) + def origin(tv: TypeVar) = + assert(!tv.isInstantiated) + tv.origin + val param = origin(tv) + val excluded = except.map(origin) + val qualifies: TypeParamRef => Boolean = !excluded.contains(_) + def test(deps: ReverseDeps, lens: ConstraintLens[List[TypeParamRef]]) = + val depending = deps(param) + null != depending && depending.exists(qualifies) + || lens(this, tv.origin.binder, tv.origin.paramNum).exists(qualifies) //.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: + private class Adjuster(srcParam: TypeParamRef)(using Context) extends TypeTraverser: var add: Boolean = compiletime.uninitialized - def update(deps: TypeVarDeps, referenced: TypeVar): TypeVarDeps = + def update(deps: ReverseDeps, referenced: TypeParamRef): ReverseDeps = val entry = deps(referenced) val prev = if null == entry then SimpleIdentitySet.empty else entry - val now = if add then prev + tvar else prev - tvar + val now = if add then prev + srcParam else prev - srcParam 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)) + if contains(param) then + if variance >= 0 then coDeps = update(coDeps, param) + if variance <= 0 then contraDeps = update(contraDeps, param) case tp: LazyRef if !tp.completed => - case _ => - traverseChildren(t) + case _ => traverseChildren(t) + end Adjuster /** Adjust dependencies to account for the delta of previous entry `prevEntry` - * and new bound `entry` for the type variable `tvar`. + * and new bound `entry` for the type parameter `srcParam`. */ - 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) + def adjustDeps(entry: Type | Null, prevEntry: Type | Null, srcParam: TypeParamRef)(using Context): this.type = + val adjuster = new Adjuster(srcParam) + + /** Adjust reverse dependencies of all type parameters 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 _ => - prevEntry match - case TypeBounds(plo, phi) => - adjustReferenced(plo, isLower = true, add = false) - adjustReferenced(phi, isLower = false, add = false) - case _ => - dropDeps(tvar) + 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(srcParam) this end adjustDeps - /** Adjust dependencies to account for adding or dropping `entries` to the - * constraint. + /** Adjust dependencies to account for adding or dropping all `entries` associated + * with `poly`. * @param add if true, entries is added, otherwise it is dropped */ - def adjustDeps(entries: Array[Type], add: Boolean)(using Context): this.type = + def adjustDeps(poly: TypeLambda, 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)) + then adjustDeps(entries(n), NoType, poly.paramRefs(n)) + else adjustDeps(NoType, entries(n), poly.paramRefs(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 _ => + /** Remove all reverse dependencies of `param` */ + def dropDeps(param: TypeParamRef)(using Context): Unit = + coDeps = coDeps.remove(param) + contraDeps = contraDeps.remove(param) /** 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}%, %" + def depsStr(deps: ReverseDeps): String = + def depStr(param: TypeParamRef) = i"$param --> ${deps(param).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" @@ -411,7 +406,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, tvars.copyToArray(entries1, nparams) newConstraint(boundsMap = this.boundsMap.updated(poly, entries1)) .init(poly) - .adjustDeps(entries1, add = true) + .adjustDeps(poly, entries1, add = true) } /** Split dependent parameters off the bounds for parameters in `poly`. @@ -558,7 +553,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)) + current1.adjustDeps(tp, current.entry(param), param) tp match { case TypeBounds(lo, hi) => for p <- dependentParams(lo, isUpper = false) do @@ -594,19 +589,22 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def removeParam(ps: List[TypeParamRef]) = ps.filterConserve(param ne _) - def replaceParam(tp: Type, atPoly: TypeLambda, atIdx: Int): Type = - current.ensureNonCyclic(atPoly.paramRefs(atIdx), tp.substParam(param, replacement)) + def replaceParam(entry: Type, atPoly: TypeLambda, atIdx: Int): Type = + val pref = atPoly.paramRefs(atIdx) + val newEntry = current.ensureNonCyclic(pref, entry.substParam(param, replacement)) + adjustDeps(newEntry, entry, pref) + newEntry current.foreachParam { (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)) + adjustDeps(newEntry, entry, p.paramRefs(i)) newEntry) current = lowerLens.map(this, current, p, i, removeParam) current = upperLens.map(this, current, p, i, removeParam) } - current.dropDeps(typeVarOfParam(param)) + current.dropDeps(param) current.checkWellFormed() end replace @@ -620,7 +618,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) - .adjustDeps(boundsMap(pt).nn, add = false) + .adjustDeps(pt, boundsMap(pt).nn, add = false) .checkWellFormed() } @@ -749,11 +747,11 @@ class OrderingConstraint(private val boundsMap: ParamBounds, s"cyclic bound for $param: ${inst.show} in ${this.show}") } if Config.checkConstraintDeps then - def checkDeps(deps: TypeVarDeps) = + def checkDeps(deps: ReverseDeps) = ()/* deps.foreachBinding { (tv, tvs) => for tv1 <- tvs do assert(!tv1.instanceOpt.exists, i"$this") - } + }*/ checkDeps(coDeps) checkDeps(contraDeps) this diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala index a940bc0f207f..d7e23409329e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala @@ -623,24 +623,25 @@ trait Inferencing { this: Typer => for tvar <- qualifying do if !tvar.isInstantiated && constraint.contains(tvar) && tvar.nestingLevel >= ctx.nestingLevel then constrainIfDependentParamRef(tvar, tree) - // Needs to be checked again, since previous interpolations could already have - // instantiated `tvar` through unification. - val v = vs(tvar) - if v == null then - toInstantiate += ((tvar, 0)) - else if v.intValue != 0 then - 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 - // N only contains type variables of level <= N. - typr.println(i"instantiate nonvariant $tvar of level ${tvar.nestingLevel} to a type variable of level <= ${ctx.nestingLevel}, $constraint") - cmp.atLevel(ctx.nestingLevel, tvar.origin) - else - typr.println(i"no interpolation for nonvariant $tvar in $state") - ) + if !tvar.isInstantiated then + // Needs to be checked again, since previous interpolations could already have + // instantiated `tvar` through unification. + val v = vs(tvar) + if v == null then + toInstantiate += ((tvar, 0)) + else if v.intValue != 0 then + 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 + // N only contains type variables of level <= N. + typr.println(i"instantiate nonvariant $tvar of level ${tvar.nestingLevel} to a type variable of level <= ${ctx.nestingLevel}, $constraint") + cmp.atLevel(ctx.nestingLevel, tvar.origin) + else + typr.println(i"no interpolation for nonvariant $tvar in $state") + ) - def typeVarsIn(xs: collection.Seq[(TypeVar, Int)]): TypeVars = + def typeVarsIn(xs: List[(TypeVar, Int)]): TypeVars = xs.foldLeft(SimpleIdentitySet.empty: TypeVars)((tvs, tvi) => tvs + tvi._1) def filterByDeps(tvs0: List[(TypeVar, Int)]): List[(TypeVar, Int)] = { @@ -654,7 +655,7 @@ trait Inferencing { this: Typer => else if v == 0 && !belowOK then step((tvar, -1) :: tvs1) else if v == -1 && !aboveOK || v == 1 && !belowOK then - println(i"drop $tvar, $v in $tp, $pt, qualifying = ${qualifying.toList}, tvs0 = ${tvs0.toList}%, %, excluded = ${excluded.toList}, $constraint") + typr.println(i"drop $tvar, $v in $tp, $pt, qualifying = ${qualifying.toList}, tvs0 = ${tvs0.toList}%, %, excluded = ${excluded.toList}, $constraint") step(tvs1) else tvs.derivedCons(hd, step(tvs1)) From 7418f2699647d2037205e06441b24ba6c23044f1 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 18 Sep 2022 15:15:59 +0200 Subject: [PATCH 05/17] Optimize update of lower and upper maps in replace --- .../src/dotty/tools/dotc/core/OrderingConstraint.scala | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 2d30b970c973..10d47d3c384f 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -595,14 +595,17 @@ class OrderingConstraint(private val boundsMap: ParamBounds, adjustDeps(newEntry, entry, pref) newEntry + for lo <- lower(param) do + current = upperLens.map(this, current, lo, removeParam) + for hi <- upper(param) do + current = lowerLens.map(this, current, hi, removeParam) + current.foreachParam { (p, i) => current = boundsLens.map(this, current, p, i, entry => val newEntry = replaceParam(entry, p, i) adjustDeps(newEntry, entry, p.paramRefs(i)) newEntry) - current = lowerLens.map(this, current, p, i, removeParam) - current = upperLens.map(this, current, p, i, removeParam) } current.dropDeps(param) current.checkWellFormed() From 6101435b511ff9ecf50da60f40e04531e2cb65c7 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 19 Sep 2022 17:39:08 +0200 Subject: [PATCH 06/17] Tweaks to OrderingConstraint --- .../src/dotty/tools/dotc/config/Config.scala | 2 +- .../tools/dotc/core/OrderingConstraint.scala | 33 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 723ab53a500d..fc64d1b16376 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -22,7 +22,7 @@ object Config { */ inline val checkConstraintsNonCyclic = false - inline val checkConstraintDeps = false + inline val checkConstraintDeps = true /** Check that each constraint resulting from a subtype test * is satisfiable. Also check that a type variable instantiation diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 10d47d3c384f..4cbc23e08478 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -226,6 +226,10 @@ class OrderingConstraint(private val boundsMap: ParamBounds, var coDeps, contraDeps: ReverseDeps = SimpleIdentityMap.empty + extension (deps: ReverseDeps) def at (param: TypeParamRef): SimpleIdentitySet[TypeParamRef] = + val result = deps(param) + if null == result then SimpleIdentitySet.empty else result + def dependsOn(tv: TypeVar, except: TypeVars, co: Boolean)(using Context): Boolean = def origin(tv: TypeVar) = assert(!tv.isInstantiated) @@ -234,8 +238,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, val excluded = except.map(origin) val qualifies: TypeParamRef => Boolean = !excluded.contains(_) def test(deps: ReverseDeps, lens: ConstraintLens[List[TypeParamRef]]) = - val depending = deps(param) - null != depending && depending.exists(qualifies) + deps.at(param).exists(qualifies) || lens(this, tv.origin.binder, tv.origin.paramNum).exists(qualifies) //.showing(i"outer depends on $tv with ${tvdeps.toList}%, % = $result") if co then test(coDeps, upperLens) else test(contraDeps, lowerLens) @@ -244,10 +247,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds, var add: Boolean = compiletime.uninitialized def update(deps: ReverseDeps, referenced: TypeParamRef): ReverseDeps = - val entry = deps(referenced) - val prev = if null == entry then SimpleIdentitySet.empty else entry - val now = if add then prev + srcParam else prev - srcParam - deps.updated(referenced, now) + val prev = deps.at(referenced) + deps.updated(referenced, if add then prev + srcParam else prev - srcParam) def traverse(t: Type) = t match case param: TypeParamRef => @@ -335,9 +336,9 @@ class OrderingConstraint(private val boundsMap: ParamBounds, /** A string representing the two depenecy maps */ def depsToString(using Context): String = def depsStr(deps: ReverseDeps): String = - def depStr(param: TypeParamRef) = i"$param --> ${deps(param).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" + def depStr(param: TypeParamRef) = i"$param --> ${deps.at(param).toList}%, %" + if deps.isEmpty then "" else i"\n ${deps.toList.map((k, v) => depStr(k))}%\n %" + i" co-deps:${depsStr(coDeps)}\n contra-deps:${depsStr(contraDeps)}\n" // ---------- Adding TypeLambdas -------------------------------------------------- @@ -550,11 +551,14 @@ class OrderingConstraint(private val boundsMap: ParamBounds, case _ => Nil - 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), param) - tp match { + private def updateEntryNoOrdering(current: This, param: TypeParamRef, newEntry: Type, oldEntry: Type)(using Context): This = + boundsLens.update(this, current, param, newEntry).adjustDeps(newEntry, oldEntry, param) + + private def updateEntry(current: This, param: TypeParamRef, newEntry: Type)(using Context): This = { + //println(i"update $param to $tp in $current") + if Config.checkNoWildcardsInConstraint then assert(!newEntry.containsWildcardTypes) + var current1 = updateEntryNoOrdering(current, param, newEntry, current.entry(param)) + newEntry match { case TypeBounds(lo, hi) => for p <- dependentParams(lo, isUpper = false) do current1 = order(current1, p, param) @@ -758,6 +762,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, checkDeps(coDeps) checkDeps(contraDeps) this + end checkWellFormed def occursAtToplevel(param: TypeParamRef, inst: Type)(using Context): Boolean = From c0c75957fc881e4acaade70e82ef814285276c4a Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 19 Sep 2022 17:51:38 +0200 Subject: [PATCH 07/17] Look inside lazy refs for dependency tracking --- compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 4cbc23e08478..d155b65a8266 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -245,6 +245,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, private class Adjuster(srcParam: TypeParamRef)(using Context) extends TypeTraverser: var add: Boolean = compiletime.uninitialized + val seen = util.HashSet[LazyRef]() def update(deps: ReverseDeps, referenced: TypeParamRef): ReverseDeps = val prev = deps.at(referenced) @@ -255,7 +256,10 @@ class OrderingConstraint(private val boundsMap: ParamBounds, if contains(param) then if variance >= 0 then coDeps = update(coDeps, param) if variance <= 0 then contraDeps = update(contraDeps, param) - case tp: LazyRef if !tp.completed => + case tp: LazyRef => + if !seen.contains(tp) then + seen += tp + traverse(tp.ref) case _ => traverseChildren(t) end Adjuster From 040293753ccfa52d700fa1b63b6ec2c9ee681e99 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 19 Sep 2022 18:34:39 +0200 Subject: [PATCH 08/17] Avoid double dependency checking when creating constraint entries --- compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index d155b65a8266..9b94c5ab4c4f 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -427,7 +427,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, val param = poly.paramRefs(i) val bounds = dropWildcards(nonParamBounds(param)) val stripped = stripParams(bounds, todos, isUpper = true) - current = updateEntry(current, param, stripped) + current = boundsLens.update(this, current, param, stripped) while todos.nonEmpty do current = todos.head(current, param) todos.dropInPlace(1) From efdfe0efb6aae57253ddfc330b06dc3a56799200 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 24 Sep 2022 13:23:43 +0200 Subject: [PATCH 09/17] Fix dependency checking in OrderingConstraint's replace --- .../tools/dotc/core/OrderingConstraint.scala | 43 +++++++++++-------- .../dotty/tools/dotc/core/Substituters.scala | 2 +- .../src/dotty/tools/dotc/core/Types.scala | 19 ++++++-- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 9b94c5ab4c4f..7867765cff7b 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -11,6 +11,7 @@ import config.Config import config.Printers.constr import reflect.ClassTag import Constraint.ReverseDeps +import Substituters.SubstParamMap import annotation.tailrec import annotation.internal.sharable import cc.{CapturingType, derivedCapturingType} @@ -37,7 +38,7 @@ object OrderingConstraint { } /** The `current` constraint but with the entry for `param` updated to `entry`. - * `current` is used linearly. If it is different from `prev` it is + * `current` is used linearly. If it is different from `prev` then `current` is * known to be dead after the call. Hence it is OK to update destructively * parts of `current` which are not shared by `prev`. */ @@ -133,6 +134,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, private val lowerMap : ParamOrdering, private val upperMap : ParamOrdering, private val hardVars : TypeVars) extends Constraint { + thisConstraint => import UnificationDirection.* @@ -243,7 +245,9 @@ class OrderingConstraint(private val boundsMap: ParamBounds, //.showing(i"outer depends on $tv with ${tvdeps.toList}%, % = $result") if co then test(coDeps, upperLens) else test(contraDeps, lowerLens) - private class Adjuster(srcParam: TypeParamRef)(using Context) extends TypeTraverser: + private class Adjuster(srcParam: TypeParamRef)(using Context) + extends TypeTraverser, ConstraintAwareTraversal: + var add: Boolean = compiletime.uninitialized val seen = util.HashSet[LazyRef]() @@ -411,7 +415,6 @@ class OrderingConstraint(private val boundsMap: ParamBounds, tvars.copyToArray(entries1, nparams) newConstraint(boundsMap = this.boundsMap.updated(poly, entries1)) .init(poly) - .adjustDeps(poly, entries1, add = true) } /** Split dependent parameters off the bounds for parameters in `poly`. @@ -433,7 +436,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds, todos.dropInPlace(1) i += 1 } - current.checkWellFormed() + current.adjustDeps(poly, current.boundsMap(poly).nn, add = true) + .checkWellFormed() } // ---------- Updates ------------------------------------------------------------ @@ -591,30 +595,33 @@ class OrderingConstraint(private val boundsMap: ParamBounds, if param == replacement then this.checkWellFormed() else assert(replacement.isValueTypeOrLambda) - var current = - if isRemovable(param.binder) then remove(param.binder) - else updateEntry(this, param, replacement) - def removeParam(ps: List[TypeParamRef]) = ps.filterConserve(param ne _) + val droppedTypeVar = typeVarOfParam(param) - def replaceParam(entry: Type, atPoly: TypeLambda, atIdx: Int): Type = - val pref = atPoly.paramRefs(atIdx) - val newEntry = current.ensureNonCyclic(pref, entry.substParam(param, replacement)) - adjustDeps(newEntry, entry, pref) - newEntry + //println(i"replace $param, $droppedTypeVar with $replacement in $this") + val dropTypeVar = new TypeMap: + override def apply(t: Type): Type = + if t.exists && (t eq droppedTypeVar) then param else mapOver(t) + var current = this + + def removeParam(ps: List[TypeParamRef]) = ps.filterConserve(param ne _) for lo <- lower(param) do current = upperLens.map(this, current, lo, removeParam) for hi <- upper(param) do current = lowerLens.map(this, current, hi, removeParam) current.foreachParam { (p, i) => - current = boundsLens.map(this, current, p, i, - entry => - val newEntry = replaceParam(entry, p, i) - adjustDeps(newEntry, entry, p.paramRefs(i)) - newEntry) + val other = p.paramRefs(i) + if other != param then + val oldEntry = current.entry(other) + val newEntry = current.ensureNonCyclic(other, oldEntry.substParam(param, replacement)) + current = updateEntryNoOrdering(current, other, newEntry, dropTypeVar(oldEntry)) } + + current = + if isRemovable(param.binder) then current.remove(param.binder) + else updateEntry(current, param, replacement) current.dropDeps(param) current.checkWellFormed() end replace diff --git a/compiler/src/dotty/tools/dotc/core/Substituters.scala b/compiler/src/dotty/tools/dotc/core/Substituters.scala index 3e32340b21bd..25cdb5d057f7 100644 --- a/compiler/src/dotty/tools/dotc/core/Substituters.scala +++ b/compiler/src/dotty/tools/dotc/core/Substituters.scala @@ -193,7 +193,7 @@ object Substituters: def apply(tp: Type): Type = substRecThis(tp, from, to, this)(using mapCtx) } - final class SubstParamMap(from: ParamRef, to: Type)(using Context) extends DeepTypeMap, IdempotentCaptRefMap { + class SubstParamMap(from: ParamRef, to: Type)(using Context) extends DeepTypeMap, IdempotentCaptRefMap { def apply(tp: Type): Type = substParam(tp, from, to, this)(using mapCtx) } diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 3243bb242a56..5cc5b6ca3821 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -5497,8 +5497,21 @@ object Types { stop == StopAt.Static && tp.currentSymbol.isStatic && isStaticPrefix(tp.prefix) || stop == StopAt.Package && tp.currentSymbol.is(Package) } + + protected def tyconTypeParams(tp: AppliedType)(using Context): List[ParamInfo] = + tp.tyconTypeParams end VariantTraversal + trait ConstraintAwareTraversal extends VariantTraversal: + override def tyconTypeParams(tp: AppliedType)(using Context): List[ParamInfo] = + tp.tycon match + case tycon: TypeParamRef => + ctx.typerState.constraint.entry(tycon) match + case _: TypeBounds => + case tp1 => if tp1.typeParams.nonEmpty then return tp1.typeParams + case _ => + tp.tyconTypeParams + /** A supertrait for some typemaps that are bijections. Used for capture checking. * BiTypeMaps should map capture references to capture references. */ @@ -5614,7 +5627,7 @@ object Types { derivedSelect(tp, prefix1) case tp: AppliedType => - derivedAppliedType(tp, this(tp.tycon), mapArgs(tp.args, tp.tyconTypeParams)) + derivedAppliedType(tp, this(tp.tycon), mapArgs(tp.args, tyconTypeParams(tp))) case tp: LambdaType => mapOverLambda(tp) @@ -5941,7 +5954,7 @@ object Types { case nil => true } - if (distributeArgs(args, tp.tyconTypeParams)) + if (distributeArgs(args, tyconTypeParams(tp))) range(tp.derivedAppliedType(tycon, loBuf.toList), tp.derivedAppliedType(tycon, hiBuf.toList)) else if tycon.isLambdaSub || args.exists(isRangeOfNonTermTypes) then @@ -6087,7 +6100,7 @@ object Types { } foldArgs(acc, tparams.tail, args.tail) } - foldArgs(this(x, tycon), tp.tyconTypeParams, args) + foldArgs(this(x, tycon), tyconTypeParams(tp), args) case _: BoundType | _: ThisType => x From e7c01e21fded3d9e7e250049ffad68a9a11ea045 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 22 Sep 2022 16:54:59 +0200 Subject: [PATCH 10/17] Avoid hygiene violations in stripImplicit --- .../src/dotty/tools/dotc/typer/Applications.scala | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 15493e6805dc..7bcceaed1112 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1502,11 +1502,17 @@ trait Applications extends Compatibility { } /** Drop any leading implicit parameter sections */ - def stripImplicit(tp: Type)(using Context): Type = tp match { + def stripImplicit(tp: Type, wildcardOnly: Boolean = false)(using Context): Type = tp match { case mt: MethodType if mt.isImplicitMethod => - stripImplicit(resultTypeApprox(mt)) + stripImplicit(resultTypeApprox(mt, wildcardOnly)) case pt: PolyType => - pt.derivedLambdaType(pt.paramNames, pt.paramInfos, stripImplicit(pt.resultType)).asInstanceOf[PolyType].flatten + pt.derivedLambdaType(pt.paramNames, pt.paramInfos, + stripImplicit(pt.resultType, wildcardOnly = true)) + // can't use TypeParamRefs for parameter references in `resultTypeApprox` + // since their bounds can refer to type parameters in `pt` that are not + // bound by the constraint. This can lead to hygiene violations if subsequently + // `pt` itself is added to the constraint. Test case is run/enrich-gentraversable.scala. + .asInstanceOf[PolyType].flatten case _ => tp } From e143ab77e14238cb32bd184e0e8ceef0989b07b1 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 24 Sep 2022 13:36:56 +0200 Subject: [PATCH 11/17] Check reverse dependencies Can be enabled globally with Config option (set to false by default) or locally through option -Ycheck-constraint-deps. The option is currently turned on for pos, run, and patmatExhaustivity tests. --- .../src/dotty/tools/dotc/config/Config.scala | 2 +- .../tools/dotc/config/ScalaSettings.scala | 1 + .../dotty/tools/dotc/core/Constraint.scala | 12 +- .../tools/dotc/core/OrderingConstraint.scala | 233 +++++++++++++----- .../src/dotty/tools/dotc/core/Types.scala | 29 +-- .../dotty/tools/dotc/CompilationTests.scala | 4 +- .../transform/PatmatExhaustivityTest.scala | 2 +- 7 files changed, 197 insertions(+), 86 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index fc64d1b16376..723ab53a500d 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -22,7 +22,7 @@ object Config { */ inline val checkConstraintsNonCyclic = false - inline val checkConstraintDeps = true + inline val checkConstraintDeps = false /** Check that each constraint resulting from a subtype test * is satisfiable. Also check that a type variable instantiation diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 09bedd3e8b35..6e1af4591cf3 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -309,6 +309,7 @@ private sealed trait YSettings: val YforceSbtPhases: Setting[Boolean] = BooleanSetting("-Yforce-sbt-phases", "Run the phases used by sbt for incremental compilation (ExtractDependencies and ExtractAPI) even if the compiler is ran outside of sbt, for debugging.") val YdumpSbtInc: Setting[Boolean] = BooleanSetting("-Ydump-sbt-inc", "For every compiled foo.scala, output the API representation and dependencies used for sbt incremental compilation in foo.inc, implies -Yforce-sbt-phases.") val YcheckAllPatmat: Setting[Boolean] = BooleanSetting("-Ycheck-all-patmat", "Check exhaustivity and redundancy of all pattern matching (used for testing the algorithm).") + val YcheckConstraintDeps: Setting[Boolean] = BooleanSetting("-Ycheck-constraint-deps", "Check dependency tracking in constraints (used for testing the algorithm).") val YretainTrees: Setting[Boolean] = BooleanSetting("-Yretain-trees", "Retain trees for top-level classes, accessible from ClassSymbol#tree") val YshowTreeIds: Setting[Boolean] = BooleanSetting("-Yshow-tree-ids", "Uniquely tag all tree nodes in debugging output.") val YfromTastyIgnoreList: Setting[List[String]] = MultiStringSetting("-Yfrom-tasty-ignore-list", "file", "List of `tasty` files in jar files that will not be loaded when using -from-tasty") diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala index de81770460c3..ee5130c6ead5 100644 --- a/compiler/src/dotty/tools/dotc/core/Constraint.scala +++ b/compiler/src/dotty/tools/dotc/core/Constraint.scala @@ -166,6 +166,12 @@ abstract class Constraint extends Showable { */ def hasConflictingTypeVarsFor(tl: TypeLambda, that: Constraint): Boolean + /** Does `param` occur at the toplevel in `tp` ? + * Toplevel means: the type itself or a factor in some + * combination of `&` or `|` types. + */ + def occursAtToplevel(param: TypeParamRef, tp: Type)(using Context): Boolean + /** A map that associates type parameters of this constraint with all other type * parameters that refer to them in their bounds covariantly, such that, if the * type parameter is instantiated to a larger type, the constraint would be narrowed. @@ -191,12 +197,6 @@ abstract class Constraint extends Showable { /** Check that no constrained parameter contains itself as a bound */ def checkWellFormed()(using Context): this.type - /** Does `param` occur at the toplevel in `tp` ? - * Toplevel means: the type itself or a factor in some - * combination of `&` or `|` types. - */ - def occursAtToplevel(param: TypeParamRef, tp: Type)(using Context): Boolean - /** Check that constraint only refers to TypeParamRefs bound by itself */ def checkClosed()(using Context): Unit diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 7867765cff7b..5acb6ea4ec23 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -18,6 +18,11 @@ import cc.{CapturingType, derivedCapturingType} object OrderingConstraint { + @sharable private var id = 0 + private def nextId = + id += 1 + id + type ArrayValuedMap[T] = SimpleIdentityMap[TypeLambda, Array[T]] /** The type of `OrderingConstraint#boundsMap` */ @@ -140,6 +145,10 @@ class OrderingConstraint(private val boundsMap: ParamBounds, type This = OrderingConstraint + var id = nextId + //if id == 118 then + // new Error(s"at $id").printStackTrace() + /** A new constraint with given maps and given set of hard typevars */ def newConstraint( // !!! Dotty problem: Making newConstraint `private` causes -Ytest-pickler failure. boundsMap: ParamBounds = this.boundsMap, @@ -245,8 +254,40 @@ class OrderingConstraint(private val boundsMap: ParamBounds, //.showing(i"outer depends on $tv with ${tvdeps.toList}%, % = $result") if co then test(coDeps, upperLens) else test(contraDeps, lowerLens) + /** Modify traversals in two respects: + * - when encountering an application C[Ts], where C is a type variable or parameter + * that has an instantiation in this constraint, assume the type parameters of + * the instantiation instead of the type parameters of C when traversing the + * arguments Ts. That can make a difference for the variance in which an argument + * is traversed. Example constraint: + * + * constrainded types: C[X], A + * A >: C[B] + * C := Option + * + * Here, B is traversed with variance +1 instead of 0. Test case: pos/t3152.scala + * + * - When typing a prefx, don't avoid negative variances. This matters only for the + * corner case where a parameter is instantiated to Nothing (see comment in + * TypeAccumulator#applyToPrefix). When determining instantiation directions + * (which is what dependency variances are for), it can be ignored. + */ + private trait ConstraintAwareTraversal[T] extends TypeAccumulator[T]: + override def tyconTypeParams(tp: AppliedType)(using Context): List[ParamInfo] = + def tparams(tycon: Type): List[ParamInfo] = tycon match + case tycon: TypeVar if !tycon.isInstantiated => tparams(tycon.origin) + case tycon: TypeParamRef => + entry(tycon) match + case _: TypeBounds => tp.tyconTypeParams + case tycon1 if tycon1.typeParams.nonEmpty => tycon1.typeParams + case _ => tp.tyconTypeParams + case _ => tp.tyconTypeParams + tparams(tp.tycon) + override def applyToPrefix(x: T, tp: NamedType): T = + this(x, tp.prefix) + private class Adjuster(srcParam: TypeParamRef)(using Context) - extends TypeTraverser, ConstraintAwareTraversal: + extends TypeTraverser, ConstraintAwareTraversal[Unit]: var add: Boolean = compiletime.uninitialized val seen = util.HashSet[LazyRef]() @@ -257,9 +298,12 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def traverse(t: Type) = t match case param: TypeParamRef => - if contains(param) then - if variance >= 0 then coDeps = update(coDeps, param) - if variance <= 0 then contraDeps = update(contraDeps, param) + entry(param) match + case _: TypeBounds => + if variance >= 0 then coDeps = update(coDeps, param) + if variance <= 0 then contraDeps = update(contraDeps, param) + case tp => + traverse(tp) case tp: LazyRef => if !seen.contains(tp) then seen += tp @@ -287,39 +331,40 @@ class OrderingConstraint(private val boundsMap: ParamBounds, * 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 + def adjustDelta(bound: Type, prevBound: Type, isLower: Boolean, baseCase: => Boolean): Boolean = + if bound eq prevBound then + baseCase else bound match case bound: AndOrType => - adjustDelta(bound.tp1, prevBound, isLower) && { + adjustDelta(bound.tp1, prevBound, isLower, baseCase) && { 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 + /** Adjust dependencies to account for the delta of previous bounds `prevBounds` + * and new bounds `bounds`. + * @param add true if the bounds are added, false if they are removed */ - def adjustBounds(bound: Type, prevBound: Type, isLower: Boolean) = - if !adjustDelta(bound, prevBound, isLower) then - adjustReferenced(prevBound, isLower, add = false) - adjustReferenced(bound, isLower, add = true) + def adjustBounds(bounds: TypeBounds, add: Boolean) = + adjustReferenced(bounds.lo, isLower = true, add) + adjustReferenced(bounds.hi, isLower = false, add) entry match - case TypeBounds(lo, hi) => + case entry @ TypeBounds(lo, hi) => prevEntry match - case TypeBounds(plo, phi) => - adjustBounds(lo, plo, isLower = true) - adjustBounds(hi, phi, isLower = false) + case prevEntry @ TypeBounds(plo, phi) => + if !adjustDelta(lo, plo, isLower = true, + adjustDelta(hi, phi, isLower = false, true)) + then + adjustBounds(prevEntry, add = false) + adjustBounds(entry, add = true) case _ => - adjustReferenced(lo, isLower = true, add = true) - adjustReferenced(hi, isLower = false, add = true) + adjustBounds(entry, add = true) case _ => prevEntry match - case TypeBounds(plo, phi) => - adjustReferenced(plo, isLower = true, add = false) - adjustReferenced(phi, isLower = false, add = false) + case prevEntry: TypeBounds => + adjustBounds(prevEntry, add = false) case _ => dropDeps(srcParam) this @@ -408,6 +453,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, } def add(poly: TypeLambda, tvars: List[TypeVar])(using Context): This = { + checkWellFormed() // TODO: drop assert(!contains(poly)) val nparams = poly.paramNames.length val entries1 = new Array[Type](nparams * 2) @@ -563,7 +609,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, boundsLens.update(this, current, param, newEntry).adjustDeps(newEntry, oldEntry, param) private def updateEntry(current: This, param: TypeParamRef, newEntry: Type)(using Context): This = { - //println(i"update $param to $tp in $current") + //println(i"update $param to $newEntry in $current") if Config.checkNoWildcardsInConstraint then assert(!newEntry.containsWildcardTypes) var current1 = updateEntryNoOrdering(current, param, newEntry, current.entry(param)) newEntry match { @@ -596,12 +642,12 @@ class OrderingConstraint(private val boundsMap: ParamBounds, else assert(replacement.isValueTypeOrLambda) - val droppedTypeVar = typeVarOfParam(param) + val replacedTypeVar = typeVarOfParam(param) + //println(i"replace $param, $replacedTypeVar with $replacement in $this") - //println(i"replace $param, $droppedTypeVar with $replacement in $this") - val dropTypeVar = new TypeMap: + def mapReplacedTypeVarTo(to: Type) = new TypeMap: override def apply(t: Type): Type = - if t.exists && (t eq droppedTypeVar) then param else mapOver(t) + if (t eq replacedTypeVar) && t.exists then to else mapOver(t) var current = this @@ -616,7 +662,29 @@ class OrderingConstraint(private val boundsMap: ParamBounds, if other != param then val oldEntry = current.entry(other) val newEntry = current.ensureNonCyclic(other, oldEntry.substParam(param, replacement)) - current = updateEntryNoOrdering(current, other, newEntry, dropTypeVar(oldEntry)) + current = boundsLens.update(this, current, other, newEntry) + var oldDepEntry = oldEntry + var newDepEntry = newEntry + replacedTypeVar match + case tvar: TypeVar => + if tvar.isInstantiated + then + // replace is called from TypeVar's instantiateWith, + // forget about instantiation for old dependencies + oldDepEntry = mapReplacedTypeVarTo(param)(oldDepEntry) + else + // replace is called from unify, + // assume parameter has been replaced for new dependencies + // (the actual replacement is done below) + newDepEntry = mapReplacedTypeVarTo(replacement)(newDepEntry) + case _ => + if oldDepEntry ne newDepEntry then + if current eq this then + // We can end up here if oldEntry eq newEntry, so posssibly no new constraint + // was created, but oldDepEntry ne newDepEntry. In that case we must make + // sure we have a new constraint before updating dependencies. + current = newConstraint() + current.adjustDeps(newDepEntry, oldDepEntry, other) } current = @@ -703,6 +771,26 @@ class OrderingConstraint(private val boundsMap: ParamBounds, assert(tvar.origin == param, i"mismatch $tvar, $param") case _ => + def occursAtToplevel(param: TypeParamRef, inst: Type)(using Context): Boolean = + def occurs(tp: Type)(using Context): Boolean = tp match + case tp: AndOrType => + occurs(tp.tp1) || occurs(tp.tp2) + case tp: TypeParamRef => + (tp eq param) || entry(tp).match + case NoType => false + case TypeBounds(lo, hi) => (lo eq hi) && occurs(lo) + case inst => occurs(inst) + case tp: TypeVar => + occurs(tp.underlying) + case TypeBounds(lo, hi) => + occurs(lo) || occurs(hi) + case _ => + val tp1 = tp.dealias + (tp1 ne tp) && occurs(tp1) + + occurs(inst) + end occursAtToplevel + // ---------- Exploration -------------------------------------------------------- def domainLambdas: List[TypeLambda] = boundsMap.keys @@ -755,7 +843,60 @@ class OrderingConstraint(private val boundsMap: ParamBounds, // ---------- Checking ----------------------------------------------- + /** Depending on Config settngs, check that there are no cycles and that + * reverse depenecies are correct. + */ def checkWellFormed()(using Context): this.type = + + /** Check that each dependency A -> B in coDeps and contraDeps corresponds to + * a reference to A at the right variance in the entry of B. + */ + def checkBackward(deps: ReverseDeps, depsName: String, v: Int)(using Context): Unit = + deps.foreachBinding { (param, params) => + for srcParam <- params do + assert(contains(srcParam) && occursAtVariance(param, v, in = entry(srcParam)), + i"wrong $depsName backwards reference $param -> $srcParam in $thisConstraint") + } + + /** A type traverser that checks that all references bound in the constraint + * are accounted for in coDeps and/or contraDeps. + */ + def checkForward(srcParam: TypeParamRef)(using Context) = + new TypeTraverser with ConstraintAwareTraversal[Unit]: + val seen = util.HashSet[LazyRef]() + def traverse(t: Type): Unit = t match + case param: TypeParamRef if param ne srcParam => + def check(deps: ReverseDeps, directDeps: List[TypeParamRef], depsName: String) = + assert(deps.at(param).contains(srcParam) || directDeps.contains(srcParam), + i"missing $depsName backwards reference $param -> $srcParam in $thisConstraint") + entry(param) match + case _: TypeBounds => + if variance >= 0 then check(contraDeps, upper(param), "contra") + if variance <= 0 then check(coDeps, lower(param), "co") + case tp => + traverse(tp) + case tp: LazyRef => + if !seen.contains(tp) then + seen += tp + traverse(tp.ref) + case _ => traverseChildren(t) + + /** Does `param` occur at variance `v` or else at variance 0 in entry `in`? */ + def occursAtVariance(param: TypeParamRef, v: Int, in: Type)(using Context): Boolean = + val test = new TypeAccumulator[Boolean] with ConstraintAwareTraversal[Boolean]: + def apply(x: Boolean, t: Type): Boolean = + if x then true + else t match + case t: TypeParamRef => + entry(t) match + case _: TypeBounds => + t == param && (variance == 0 || variance == v) + case e => + apply(x, e) + case _ => + foldOver(x, t) + test(false, in) + if Config.checkConstraintsNonCyclic then domainParams.foreach { param => val inst = entry(param) @@ -764,38 +905,14 @@ class OrderingConstraint(private val boundsMap: ParamBounds, assert(!occursAtToplevel(param, inst), s"cyclic bound for $param: ${inst.show} in ${this.show}") } - if Config.checkConstraintDeps then - def checkDeps(deps: ReverseDeps) = ()/* - deps.foreachBinding { (tv, tvs) => - for tv1 <- tvs do - assert(!tv1.instanceOpt.exists, i"$this") - }*/ - checkDeps(coDeps) - checkDeps(contraDeps) + if Config.checkConstraintDeps || ctx.settings.YcheckConstraintDeps.value then + checkBackward(coDeps, "co", -1) + checkBackward(contraDeps, "contra", +1) + domainParams.foreach(p => if contains(p) then checkForward(p).traverse(entry(p))) + this end checkWellFormed - def occursAtToplevel(param: TypeParamRef, inst: Type)(using Context): Boolean = - - def occurs(tp: Type)(using Context): Boolean = tp match - case tp: AndOrType => - occurs(tp.tp1) || occurs(tp.tp2) - case tp: TypeParamRef => - (tp eq param) || entry(tp).match - case NoType => false - case TypeBounds(lo, hi) => (lo eq hi) && occurs(lo) - case inst => occurs(inst) - case tp: TypeVar => - occurs(tp.underlying) - case TypeBounds(lo, hi) => - occurs(lo) || occurs(hi) - case _ => - val tp1 = tp.dealias - (tp1 ne tp) && occurs(tp1) - - occurs(inst) - end occursAtToplevel - override def checkClosed()(using Context): Unit = def isFreeTypeParamRef(tp: Type) = tp match diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index 5cc5b6ca3821..b79093e6bca4 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -5502,16 +5502,6 @@ object Types { tp.tyconTypeParams end VariantTraversal - trait ConstraintAwareTraversal extends VariantTraversal: - override def tyconTypeParams(tp: AppliedType)(using Context): List[ParamInfo] = - tp.tycon match - case tycon: TypeParamRef => - ctx.typerState.constraint.entry(tycon) match - case _: TypeBounds => - case tp1 => if tp1.typeParams.nonEmpty then return tp1.typeParams - case _ => - tp.tyconTypeParams - /** A supertrait for some typemaps that are bijections. Used for capture checking. * BiTypeMaps should map capture references to capture references. */ @@ -5617,13 +5607,7 @@ object Types { case tp: NamedType => if stopBecauseStaticOrLocal(tp) then tp else - val prefix1 = atVariance(variance max 0)(this(tp.prefix)) - // A prefix is never contravariant. Even if say `p.A` is used in a contravariant - // context, we cannot assume contravariance for `p` because `p`'s lower - // bound might not have a binding for `A` (e.g. the lower bound could be `Nothing`). - // By contrast, covariance does translate to the prefix, since we have that - // if `p <: q` then `p.A <: q.A`, and well-formedness requires that `A` is a member - // of `p`'s upper bound. + val prefix1 = atVariance(variance max 0)(this(tp.prefix)) // see comment of TypeAccumulator's applyToPrefix derivedSelect(tp, prefix1) case tp: AppliedType => @@ -6076,7 +6060,16 @@ object Types { protected def applyToAnnot(x: T, annot: Annotation): T = x // don't go into annotations - protected final def applyToPrefix(x: T, tp: NamedType): T = + /** A prefix is never contravariant. Even if say `p.A` is used in a contravariant + * context, we cannot assume contravariance for `p` because `p`'s lower + * bound might not have a binding for `A`, since the lower bound could be `Nothing`. + * By contrast, covariance does translate to the prefix, since we have that + * if `p <: q` then `p.A <: q.A`, and well-formedness requires that `A` is a member + * of `p`'s upper bound. + * Overridden in traversers that compute or check reverse dependencies in OrderingConstraint, + * where we use a more relaxed scheme. + */ + protected def applyToPrefix(x: T, tp: NamedType): T = atVariance(variance max 0)(this(x, tp.prefix)) // see remark on NamedType case in TypeMap def foldOver(x: T, tp: Type): T = { diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 261e6af21927..ff252932f763 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -44,7 +44,7 @@ class CompilationTests { compileFilesInDir("tests/pos-custom-args/erased", defaultOptions.and("-language:experimental.erasedDefinitions")), compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init")), // Run tests for experimental lightweight lazy vals - compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init", "-Ylightweight-lazy-vals"), FileFilter.include(TestSources.posLazyValsAllowlist)), + compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init", "-Ylightweight-lazy-vals", "-Ycheck-constraint-deps"), FileFilter.include(TestSources.posLazyValsAllowlist)), compileFilesInDir("tests/pos-deep-subtype", allowDeepSubtypes), compileFilesInDir("tests/pos-custom-args/no-experimental", defaultOptions.and("-Yno-experimental")), compileDir("tests/pos-special/java-param-names", defaultOptions.withJavacOnlyOptions("-parameters")), @@ -219,7 +219,7 @@ class CompilationTests { compileFilesInDir("tests/run-deep-subtype", allowDeepSubtypes), compileFilesInDir("tests/run", defaultOptions.and("-Ysafe-init"), FileFilter.exclude("serialization-new.scala")), // Run tests for experimental lightweight lazy vals and stable lazy vals. - compileFilesInDir("tests/run", defaultOptions.and("-Ysafe-init", "-Ylightweight-lazy-vals"), FileFilter.include(TestSources.runLazyValsAllowlist)), + compileFilesInDir("tests/run", defaultOptions.and("-Ysafe-init", "-Ylightweight-lazy-vals", "-Ycheck-constraint-deps"), FileFilter.include(TestSources.runLazyValsAllowlist)), ).checkRuns() } diff --git a/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala b/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala index eb6ab8e8fb5f..1e7d7ef2c708 100644 --- a/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala +++ b/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala @@ -20,7 +20,7 @@ class PatmatExhaustivityTest { val testsDir = "tests/patmat" // pagewidth/color: for a stable diff as the defaults are based on the terminal (e.g size) // stop-after: patmatexhaust-huge.scala crash compiler (but also hides other warnings..) - val options = List("-pagewidth", "80", "-color:never", "-Ystop-after:explicitSelf", "-classpath", TestConfiguration.basicClasspath) + val options = List("-pagewidth", "80", "-color:never", "-Ystop-after:explicitSelf", "-Ycheck-constraint-deps", "-classpath", TestConfiguration.basicClasspath) private def compile(files: List[JPath]): Seq[String] = { val opts = toolArgsFor(files).get(ToolName.Scalac).getOrElse(Nil) From d8cc74138c1dd013774358799ed7ccb784a1763e Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 24 Sep 2022 15:02:34 +0200 Subject: [PATCH 12/17] Optimize replace using reverse dependencies --- .../src/dotty/tools/dotc/config/Config.scala | 3 + .../tools/dotc/core/OrderingConstraint.scala | 105 +++++++++++------- 2 files changed, 66 insertions(+), 42 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 723ab53a500d..3b6d92b0235f 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -22,6 +22,9 @@ object Config { */ inline val checkConstraintsNonCyclic = false + /** Check that reverse dependencies in constraints are correct and complete. + * Can also be enabled using -Ycheck-constraint-deps. + */ inline val checkConstraintDeps = false /** Check that each constraint resulting from a subtype test diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 5acb6ea4ec23..8f4459a61029 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -18,10 +18,16 @@ import cc.{CapturingType, derivedCapturingType} object OrderingConstraint { - @sharable private var id = 0 - private def nextId = - id += 1 - id + /** If true, use reverse dependencies in `replace` to avoid checking the bounds + * of all parameters in the constraint. This can speed things up, but there are some + * rare corner cases where reverse dependencies miss a parameter. Specifically, + * if a constraint contains a free reference to TypeParam P and afterwards the + * same P is added as a bound variable to the constraint, a backwards link would + * then become necessary at this point but is missing. This causes two CB projects + * to fail when reverse dependencies are checked (parboiled2 and perspective). + * In these rare cases `replace` would behave differently when optimized. + */ + final val optimizeReplace = true type ArrayValuedMap[T] = SimpleIdentityMap[TypeLambda, Array[T]] @@ -145,10 +151,6 @@ class OrderingConstraint(private val boundsMap: ParamBounds, type This = OrderingConstraint - var id = nextId - //if id == 118 then - // new Error(s"at $id").printStackTrace() - /** A new constraint with given maps and given set of hard typevars */ def newConstraint( // !!! Dotty problem: Making newConstraint `private` causes -Ytest-pickler failure. boundsMap: ParamBounds = this.boundsMap, @@ -294,7 +296,9 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def update(deps: ReverseDeps, referenced: TypeParamRef): ReverseDeps = val prev = deps.at(referenced) - deps.updated(referenced, if add then prev + srcParam else prev - srcParam) + val newSet = if add then prev + srcParam else prev - srcParam + if newSet.isEmpty then deps.remove(referenced) + else deps.updated(referenced, newSet) def traverse(t: Type) = t match case param: TypeParamRef => @@ -651,41 +655,58 @@ class OrderingConstraint(private val boundsMap: ParamBounds, var current = this - def removeParam(ps: List[TypeParamRef]) = ps.filterConserve(param ne _) + def removeParamFrom(ps: List[TypeParamRef]) = + ps.filterConserve(param ne _) + for lo <- lower(param) do - current = upperLens.map(this, current, lo, removeParam) + current = upperLens.map(this, current, lo, removeParamFrom) for hi <- upper(param) do - current = lowerLens.map(this, current, hi, removeParam) - - current.foreachParam { (p, i) => - val other = p.paramRefs(i) - if other != param then - val oldEntry = current.entry(other) - val newEntry = current.ensureNonCyclic(other, oldEntry.substParam(param, replacement)) - current = boundsLens.update(this, current, other, newEntry) - var oldDepEntry = oldEntry - var newDepEntry = newEntry - replacedTypeVar match - case tvar: TypeVar => - if tvar.isInstantiated - then - // replace is called from TypeVar's instantiateWith, - // forget about instantiation for old dependencies - oldDepEntry = mapReplacedTypeVarTo(param)(oldDepEntry) - else - // replace is called from unify, - // assume parameter has been replaced for new dependencies - // (the actual replacement is done below) - newDepEntry = mapReplacedTypeVarTo(replacement)(newDepEntry) - case _ => - if oldDepEntry ne newDepEntry then - if current eq this then - // We can end up here if oldEntry eq newEntry, so posssibly no new constraint - // was created, but oldDepEntry ne newDepEntry. In that case we must make - // sure we have a new constraint before updating dependencies. - current = newConstraint() - current.adjustDeps(newDepEntry, oldDepEntry, other) - } + current = lowerLens.map(this, current, hi, removeParamFrom) + + def replaceParamIn(other: TypeParamRef) = + val oldEntry = current.entry(other) + val newEntry = current.ensureNonCyclic(other, oldEntry.substParam(param, replacement)) + current = boundsLens.update(this, current, other, newEntry) + var oldDepEntry = oldEntry + var newDepEntry = newEntry + replacedTypeVar match + case tvar: TypeVar => + if tvar.isInstantiated + then + // replace is called from TypeVar's instantiateWith, + // forget about instantiation for old dependencies + oldDepEntry = mapReplacedTypeVarTo(param)(oldDepEntry) + else + // replace is called from unify, + // assume parameter has been replaced for new dependencies + // (the actual replacement is done below) + newDepEntry = mapReplacedTypeVarTo(replacement)(newDepEntry) + case _ => + if oldDepEntry ne newDepEntry then + if current eq this then + // We can end up here if oldEntry eq newEntry, so posssibly no new constraint + // was created, but oldDepEntry ne newDepEntry. In that case we must make + // sure we have a new constraint before updating dependencies. + current = newConstraint() + current.adjustDeps(newDepEntry, oldDepEntry, other) + end replaceParamIn + + if optimizeReplace then + val co = current.coDeps.at(param) + val contra = current.contraDeps.at(param) + current.foreachParam { (p, i) => + val other = p.paramRefs(i) + entry(other) match + case _: TypeBounds => + if co.contains(other) || contra.contains(other) then + replaceParamIn(other) + case _ => replaceParamIn(other) + } + else + current.foreachParam { (p, i) => + val other = p.paramRefs(i) + if other != param then replaceParamIn(other) + } current = if isRemovable(param.binder) then current.remove(param.binder) From 06189efa63f995bd7e2ed548a4de2ad398f67f8a Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 24 Sep 2022 12:49:25 +0200 Subject: [PATCH 13/17] Add comments and some simplifications --- .../src/dotty/tools/dotc/config/Config.scala | 2 +- .../dotty/tools/dotc/core/Constraint.scala | 24 ++-- .../tools/dotc/core/OrderingConstraint.scala | 101 ++++++++++------- .../dotty/tools/dotc/core/Substituters.scala | 2 +- .../src/dotty/tools/dotc/core/Types.scala | 11 +- .../dotty/tools/dotc/typer/Inferencing.scala | 106 +++++++++--------- 6 files changed, 131 insertions(+), 115 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala index 3b6d92b0235f..cbd50429492e 100644 --- a/compiler/src/dotty/tools/dotc/config/Config.scala +++ b/compiler/src/dotty/tools/dotc/config/Config.scala @@ -189,7 +189,7 @@ 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. */ + /** If set, show variable/variable reverse dependencies when printing constraints. */ inline val showConstraintDeps = true /** If set, method results that are context functions are flattened by adding diff --git a/compiler/src/dotty/tools/dotc/core/Constraint.scala b/compiler/src/dotty/tools/dotc/core/Constraint.scala index ee5130c6ead5..fb87aed77c41 100644 --- a/compiler/src/dotty/tools/dotc/core/Constraint.scala +++ b/compiler/src/dotty/tools/dotc/core/Constraint.scala @@ -172,19 +172,9 @@ abstract class Constraint extends Showable { */ def occursAtToplevel(param: TypeParamRef, tp: Type)(using Context): Boolean - /** A map that associates type parameters of this constraint with all other type - * parameters that refer to them in their bounds covariantly, such that, if the - * type parameter is instantiated to a larger type, the constraint would be narrowed. + /** A string that shows the reverse dependencies maintained by this constraint + * (coDeps and contraDeps for OrderingConstraints). */ - def coDeps: Constraint.ReverseDeps - - /** A map that associates type parameters of this constraint with all other type - * parameters that refer to them in their bounds covariantly, such that, if the - * type parameter is instantiated to a smaller type, the constraint would be narrowed. - */ - def contraDeps: Constraint.ReverseDeps - - /** A string showing the `coDeps` and `contraDeps` maps */ def depsToString(using Context): String /** Does the constraint restricted to variables outside `except` depend on `tv` @@ -194,7 +184,12 @@ abstract class Constraint extends Showable { */ def dependsOn(tv: TypeVar, except: TypeVars, co: Boolean)(using Context): Boolean - /** Check that no constrained parameter contains itself as a bound */ + /** Depending on Config settngs: + * - Under `checkConstraintsNonCyclic`, check that no constrained + * parameter contains itself as a bound. + * - Under `checkConstraintDeps`, check hat reverse dependencies in + * constraints are correct and complete. + */ def checkWellFormed()(using Context): this.type /** Check that constraint only refers to TypeParamRefs bound by itself */ @@ -206,9 +201,6 @@ abstract class Constraint extends Showable { def checkConsistentVars()(using Context): Unit } -object Constraint: - type ReverseDeps = SimpleIdentityMap[TypeParamRef, SimpleIdentitySet[TypeParamRef]] - /** 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. diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 8f4459a61029..39b396b02f90 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -10,8 +10,6 @@ import printing.Texts._ import config.Config import config.Printers.constr import reflect.ClassTag -import Constraint.ReverseDeps -import Substituters.SubstParamMap import annotation.tailrec import annotation.internal.sharable import cc.{CapturingType, derivedCapturingType} @@ -25,20 +23,27 @@ object OrderingConstraint { * same P is added as a bound variable to the constraint, a backwards link would * then become necessary at this point but is missing. This causes two CB projects * to fail when reverse dependencies are checked (parboiled2 and perspective). - * In these rare cases `replace` would behave differently when optimized. + * In these rare cases `replace` could behave differently when optimized. However, + * no deviation was found in the two projects. It is not clear what the "right" + * behavior of `replace` should be in these cases. Normally, PolyTypes added + * to constraints are supposed to be fresh, so that would mean that the behavior + * with optimizeReplace = true would be correct. But the previous behavior without + * reverse dependency checking corresponds to `optimizeReplace = false`. This behavior + * makes sense if we assume that the added polytype was simply added too late, so we + * want to establish the link between newly bound variable and pre-existing reference. */ - final val optimizeReplace = true + private final val optimizeReplace = true - type ArrayValuedMap[T] = SimpleIdentityMap[TypeLambda, Array[T]] + private type ArrayValuedMap[T] = SimpleIdentityMap[TypeLambda, Array[T]] /** The type of `OrderingConstraint#boundsMap` */ - type ParamBounds = ArrayValuedMap[Type] + private type ParamBounds = ArrayValuedMap[Type] /** The type of `OrderingConstraint#lowerMap`, `OrderingConstraint#upperMap` */ - type ParamOrdering = ArrayValuedMap[List[TypeParamRef]] + private type ParamOrdering = ArrayValuedMap[List[TypeParamRef]] /** A lens for updating a single entry array in one of the three constraint maps */ - abstract class ConstraintLens[T <: AnyRef: ClassTag] { + private abstract class ConstraintLens[T <: AnyRef: ClassTag] { def entries(c: OrderingConstraint, poly: TypeLambda): Array[T] | Null def updateEntries(c: OrderingConstraint, poly: TypeLambda, entries: Array[T])(using Context): OrderingConstraint def initial: T @@ -91,7 +96,7 @@ object OrderingConstraint { map(prev, current, param.binder, param.paramNum, f) } - val boundsLens: ConstraintLens[Type] = new ConstraintLens[Type] { + private val boundsLens: ConstraintLens[Type] = new ConstraintLens[Type] { def entries(c: OrderingConstraint, poly: TypeLambda): Array[Type] | Null = c.boundsMap(poly) def updateEntries(c: OrderingConstraint, poly: TypeLambda, entries: Array[Type])(using Context): OrderingConstraint = @@ -99,7 +104,7 @@ object OrderingConstraint { def initial = NoType } - val lowerLens: ConstraintLens[List[TypeParamRef]] = new ConstraintLens[List[TypeParamRef]] { + private val lowerLens: ConstraintLens[List[TypeParamRef]] = new ConstraintLens[List[TypeParamRef]] { def entries(c: OrderingConstraint, poly: TypeLambda): Array[List[TypeParamRef]] | Null = c.lowerMap(poly) def updateEntries(c: OrderingConstraint, poly: TypeLambda, entries: Array[List[TypeParamRef]])(using Context): OrderingConstraint = @@ -107,7 +112,7 @@ object OrderingConstraint { def initial = Nil } - val upperLens: ConstraintLens[List[TypeParamRef]] = new ConstraintLens[List[TypeParamRef]] { + private val upperLens: ConstraintLens[List[TypeParamRef]] = new ConstraintLens[List[TypeParamRef]] { def entries(c: OrderingConstraint, poly: TypeLambda): Array[List[TypeParamRef]] | Null = c.upperMap(poly) def updateEntries(c: OrderingConstraint, poly: TypeLambda, entries: Array[List[TypeParamRef]])(using Context): OrderingConstraint = @@ -237,13 +242,30 @@ class OrderingConstraint(private val boundsMap: ParamBounds, // ------------- Type parameter dependencies ---------------------------------------- - var coDeps, contraDeps: ReverseDeps = SimpleIdentityMap.empty + private type ReverseDeps = SimpleIdentityMap[TypeParamRef, SimpleIdentitySet[TypeParamRef]] - extension (deps: ReverseDeps) def at (param: TypeParamRef): SimpleIdentitySet[TypeParamRef] = + /** A map that associates type parameters of this constraint with all other type + * parameters that refer to them in their bounds covariantly, such that, if the + * type parameter is instantiated to a larger type, the constraint would be narrowed + * (i.e. solution set changes other than simply being made larger). + */ + private var coDeps: ReverseDeps = SimpleIdentityMap.empty + + /** A map that associates type parameters of this constraint with all other type + * parameters that refer to them in their bounds covariantly, such that, if the + * type parameter is instantiated to a smaller type, the constraint would be narrowed. + * (i.e. solution set changes other than simply being made larger). + */ + private var contraDeps: ReverseDeps = SimpleIdentityMap.empty + + /** Null-safe indexing */ + extension (deps: ReverseDeps) def at(param: TypeParamRef): SimpleIdentitySet[TypeParamRef] = val result = deps(param) - if null == result then SimpleIdentitySet.empty else result + if null == result // swapped operand order important since `==` is overloaded in `SimpleIdentitySet` + then SimpleIdentitySet.empty + else result - def dependsOn(tv: TypeVar, except: TypeVars, co: Boolean)(using Context): Boolean = + override def dependsOn(tv: TypeVar, except: TypeVars, co: Boolean)(using Context): Boolean = def origin(tv: TypeVar) = assert(!tv.isInstantiated) tv.origin @@ -253,7 +275,6 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def test(deps: ReverseDeps, lens: ConstraintLens[List[TypeParamRef]]) = deps.at(param).exists(qualifies) || lens(this, tv.origin.binder, tv.origin.paramNum).exists(qualifies) - //.showing(i"outer depends on $tv with ${tvdeps.toList}%, % = $result") if co then test(coDeps, upperLens) else test(contraDeps, lowerLens) /** Modify traversals in two respects: @@ -271,10 +292,11 @@ class OrderingConstraint(private val boundsMap: ParamBounds, * * - When typing a prefx, don't avoid negative variances. This matters only for the * corner case where a parameter is instantiated to Nothing (see comment in - * TypeAccumulator#applyToPrefix). When determining instantiation directions - * (which is what dependency variances are for), it can be ignored. + * TypeAccumulator#applyToPrefix). When determining instantiation directions in + * interpolations (which is what dependency variances are for), it can be ignored. */ private trait ConstraintAwareTraversal[T] extends TypeAccumulator[T]: + override def tyconTypeParams(tp: AppliedType)(using Context): List[ParamInfo] = def tparams(tycon: Type): List[ParamInfo] = tycon match case tycon: TypeVar if !tycon.isInstantiated => tparams(tycon.origin) @@ -285,14 +307,16 @@ class OrderingConstraint(private val boundsMap: ParamBounds, case _ => tp.tyconTypeParams case _ => tp.tyconTypeParams tparams(tp.tycon) + override def applyToPrefix(x: T, tp: NamedType): T = this(x, tp.prefix) + end ConstraintAwareTraversal private class Adjuster(srcParam: TypeParamRef)(using Context) extends TypeTraverser, ConstraintAwareTraversal[Unit]: var add: Boolean = compiletime.uninitialized - val seen = util.HashSet[LazyRef]() + private val seen = util.HashSet[LazyRef]() def update(deps: ReverseDeps, referenced: TypeParamRef): ReverseDeps = val prev = deps.at(referenced) @@ -316,7 +340,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, end Adjuster /** Adjust dependencies to account for the delta of previous entry `prevEntry` - * and new bound `entry` for the type parameter `srcParam`. + * and the new bound `entry` for the type parameter `srcParam`. */ def adjustDeps(entry: Type | Null, prevEntry: Type | Null, srcParam: TypeParamRef)(using Context): this.type = val adjuster = new Adjuster(srcParam) @@ -332,8 +356,10 @@ class OrderingConstraint(private val boundsMap: ParamBounds, /** 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`. + * and/or prefix of `bound`, and `baseCase` is true, just add the new parts of `bound`. * @param isLower `bound` and `prevBound` are lower bounds + * @return true iff the delta strategy succeeded, false if it failed in which case + * the constraint is left unchanged. */ def adjustDelta(bound: Type, prevBound: Type, isLower: Boolean, baseCase: => Boolean): Boolean = if bound eq prevBound then @@ -346,9 +372,8 @@ class OrderingConstraint(private val boundsMap: ParamBounds, } case _ => false - /** Adjust dependencies to account for the delta of previous bounds `prevBounds` - * and new bounds `bounds`. - * @param add true if the bounds are added, false if they are removed + /** Add or remove depenencies referenced in `bounds`. + * @param add if true, dependecies are added, otherwise they are removed */ def adjustBounds(bounds: TypeBounds, add: Boolean) = adjustReferenced(bounds.lo, isLower = true, add) @@ -370,7 +395,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, case prevEntry: TypeBounds => adjustBounds(prevEntry, add = false) case _ => - dropDeps(srcParam) + dropDeps(srcParam) // srcParam is instantiated, so its dependencies can be dropped this end adjustDeps @@ -390,7 +415,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, coDeps = coDeps.remove(param) contraDeps = contraDeps.remove(param) - /** A string representing the two depenecy maps */ + /** A string representing the two dependency maps */ def depsToString(using Context): String = def depsStr(deps: ReverseDeps): String = def depStr(param: TypeParamRef) = i"$param --> ${deps.at(param).toList}%, %" @@ -457,7 +482,6 @@ class OrderingConstraint(private val boundsMap: ParamBounds, } def add(poly: TypeLambda, tvars: List[TypeVar])(using Context): This = { - checkWellFormed() // TODO: drop assert(!contains(poly)) val nparams = poly.paramNames.length val entries1 = new Array[Type](nparams * 2) @@ -609,13 +633,11 @@ class OrderingConstraint(private val boundsMap: ParamBounds, case _ => Nil - private def updateEntryNoOrdering(current: This, param: TypeParamRef, newEntry: Type, oldEntry: Type)(using Context): This = - boundsLens.update(this, current, param, newEntry).adjustDeps(newEntry, oldEntry, param) - private def updateEntry(current: This, param: TypeParamRef, newEntry: Type)(using Context): This = { - //println(i"update $param to $newEntry in $current") if Config.checkNoWildcardsInConstraint then assert(!newEntry.containsWildcardTypes) - var current1 = updateEntryNoOrdering(current, param, newEntry, current.entry(param)) + val oldEntry = current.entry(param) + var current1 = boundsLens.update(this, current, param, newEntry) + .adjustDeps(newEntry, oldEntry, param) newEntry match { case TypeBounds(lo, hi) => for p <- dependentParams(lo, isUpper = false) do @@ -647,7 +669,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, assert(replacement.isValueTypeOrLambda) val replacedTypeVar = typeVarOfParam(param) - //println(i"replace $param, $replacedTypeVar with $replacement in $this") + //println(i"replace $param with $replacement in $this") def mapReplacedTypeVarTo(to: Type) = new TypeMap: override def apply(t: Type): Type = @@ -673,13 +695,13 @@ class OrderingConstraint(private val boundsMap: ParamBounds, case tvar: TypeVar => if tvar.isInstantiated then - // replace is called from TypeVar's instantiateWith, - // forget about instantiation for old dependencies + // That's the case if replace is called from TypeVar's instantiateWith. + // Forget about instantiation for old dependencies. oldDepEntry = mapReplacedTypeVarTo(param)(oldDepEntry) else - // replace is called from unify, - // assume parameter has been replaced for new dependencies - // (the actual replacement is done below) + // That's the case if replace is called from unify. + // Assume parameter has been replaced for new dependencies + // (the actual replacement is done below). newDepEntry = mapReplacedTypeVarTo(replacement)(newDepEntry) case _ => if oldDepEntry ne newDepEntry then @@ -864,9 +886,6 @@ class OrderingConstraint(private val boundsMap: ParamBounds, // ---------- Checking ----------------------------------------------- - /** Depending on Config settngs, check that there are no cycles and that - * reverse depenecies are correct. - */ def checkWellFormed()(using Context): this.type = /** Check that each dependency A -> B in coDeps and contraDeps corresponds to diff --git a/compiler/src/dotty/tools/dotc/core/Substituters.scala b/compiler/src/dotty/tools/dotc/core/Substituters.scala index 25cdb5d057f7..3e32340b21bd 100644 --- a/compiler/src/dotty/tools/dotc/core/Substituters.scala +++ b/compiler/src/dotty/tools/dotc/core/Substituters.scala @@ -193,7 +193,7 @@ object Substituters: def apply(tp: Type): Type = substRecThis(tp, from, to, this)(using mapCtx) } - class SubstParamMap(from: ParamRef, to: Type)(using Context) extends DeepTypeMap, IdempotentCaptRefMap { + final class SubstParamMap(from: ParamRef, to: Type)(using Context) extends DeepTypeMap, IdempotentCaptRefMap { def apply(tp: Type): Type = substParam(tp, from, to, this)(using mapCtx) } diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala index b79093e6bca4..54bd0eafa064 100644 --- a/compiler/src/dotty/tools/dotc/core/Types.scala +++ b/compiler/src/dotty/tools/dotc/core/Types.scala @@ -5498,6 +5498,11 @@ object Types { || stop == StopAt.Package && tp.currentSymbol.is(Package) } + /** The type parameters of the constructor of this applied type. + * Overridden in OrderingConstraint's ConstraintAwareTraversal to take account + * of instantiations in the constraint that are not yet propagated to the + * instance types of type variables. + */ protected def tyconTypeParams(tp: AppliedType)(using Context): List[ParamInfo] = tp.tyconTypeParams end VariantTraversal @@ -6066,11 +6071,11 @@ object Types { * By contrast, covariance does translate to the prefix, since we have that * if `p <: q` then `p.A <: q.A`, and well-formedness requires that `A` is a member * of `p`'s upper bound. - * Overridden in traversers that compute or check reverse dependencies in OrderingConstraint, - * where we use a more relaxed scheme. + * Overridden in OrderingConstraint's ConstraintAwareTraversal, where a + * more relaxed scheme is used. */ protected def applyToPrefix(x: T, tp: NamedType): T = - atVariance(variance max 0)(this(x, tp.prefix)) // see remark on NamedType case in TypeMap + atVariance(variance max 0)(this(x, tp.prefix)) def foldOver(x: T, tp: Type): T = { record(s"foldOver $getClass") diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala index d7e23409329e..9d2db773c4d4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala @@ -573,7 +573,7 @@ trait Inferencing { this: Typer => * Then `Y` also occurs co-variantly in `T` because it needs to be minimized in order to constrain * `T` the least. See `variances` for more detail. */ - def interpolateTypeVars(tree: Tree, pt: Type, locked: TypeVars)(using Context): tree.type = { + def interpolateTypeVars(tree: Tree, pt: Type, locked: TypeVars)(using Context): tree.type = val state = ctx.typerState // Note that some variables in `locked` might not be in `state.ownedVars` @@ -582,7 +582,7 @@ trait Inferencing { this: Typer => // `qualifying`. val ownedVars = state.ownedVars - if ((ownedVars ne locked) && !ownedVars.isEmpty) { + if (ownedVars ne locked) && !ownedVars.isEmpty then val qualifying = ownedVars -- locked if (!qualifying.isEmpty) { typr.println(i"interpolate $tree: ${tree.tpe.widen} in $state, pt = $pt, owned vars = ${state.ownedVars.toList}%, %, qualifying = ${qualifying.toList}%, %, previous = ${locked.toList}%, % / ${state.constraint}") @@ -618,35 +618,46 @@ trait Inferencing { this: Typer => if state.reporter.hasUnreportedErrors then return tree def constraint = state.constraint - 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 - constrainIfDependentParamRef(tvar, tree) - if !tvar.isInstantiated then - // Needs to be checked again, since previous interpolations could already have - // instantiated `tvar` through unification. - val v = vs(tvar) - if v == null then - toInstantiate += ((tvar, 0)) - else if v.intValue != 0 then - 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 - // N only contains type variables of level <= N. - typr.println(i"instantiate nonvariant $tvar of level ${tvar.nestingLevel} to a type variable of level <= ${ctx.nestingLevel}, $constraint") - cmp.atLevel(ctx.nestingLevel, tvar.origin) - else - typr.println(i"no interpolation for nonvariant $tvar in $state") - ) - def typeVarsIn(xs: List[(TypeVar, Int)]): TypeVars = + /** Values of this type report type variables to instantiate with variance indication: + * +1 variable appears covariantly, can be instantiated from lower bound + * -1 variable appears contravariantly, can be instantiated from upper bound + * 0 variable does not appear at all, can be instantiated from either bound + */ + type ToInstantiate = List[(TypeVar, Int)] + + val toInstantiate: ToInstantiate = + val buf = new mutable.ListBuffer[(TypeVar, Int)] + for tvar <- qualifying do + if !tvar.isInstantiated && constraint.contains(tvar) && tvar.nestingLevel >= ctx.nestingLevel then + constrainIfDependentParamRef(tvar, tree) + if !tvar.isInstantiated then + // isInstantiated needs to be checked again, since previous interpolations could already have + // instantiated `tvar` through unification. + val v = vs(tvar) + if v == null then buf += ((tvar, 0)) + else if v.intValue != 0 then buf += ((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 + // N only contains type variables of level <= N. + typr.println(i"instantiate nonvariant $tvar of level ${tvar.nestingLevel} to a type variable of level <= ${ctx.nestingLevel}, $constraint") + cmp.atLevel(ctx.nestingLevel, tvar.origin) + else + typr.println(i"no interpolation for nonvariant $tvar in $state") + ) + buf.toList + + def typeVarsIn(xs: ToInstantiate): TypeVars = xs.foldLeft(SimpleIdentitySet.empty: TypeVars)((tvs, tvi) => tvs + tvi._1) - def filterByDeps(tvs0: List[(TypeVar, Int)]): List[(TypeVar, Int)] = { - val excluded = typeVarsIn(tvs0) - def step(tvs: List[(TypeVar, Int)]): List[(TypeVar, Int)] = tvs match + /** Filter list of proposed instantiations so that they don't constrain further + * the current constraint. + */ + def filterByDeps(tvs0: ToInstantiate): ToInstantiate = + val excluded = // ignore dependencies from other variables that are being instantiated + typeVarsIn(tvs0) + def step(tvs: ToInstantiate): ToInstantiate = tvs match case tvs @ (hd @ (tvar, v)) :: tvs1 => def aboveOK = !constraint.dependsOn(tvar, excluded, co = true) def belowOK = !constraint.dependsOn(tvar, excluded, co = false) @@ -657,16 +668,17 @@ trait Inferencing { this: Typer => else if v == -1 && !aboveOK || v == 1 && !belowOK then typr.println(i"drop $tvar, $v in $tp, $pt, qualifying = ${qualifying.toList}, tvs0 = ${tvs0.toList}%, %, excluded = ${excluded.toList}, $constraint") step(tvs1) - else + else // no conflict, keep the instantiation proposal tvs.derivedCons(hd, step(tvs1)) case Nil => Nil val tvs1 = step(tvs0) - if tvs1 eq tvs0 then tvs1 else filterByDeps(tvs1) - }//.showing(i"filter $tvs0 in $constraint = $result") + if tvs1 eq tvs0 then tvs1 + else filterByDeps(tvs1) // filter again with smaller excluded set end filterByDeps - /** Instantiate all type variables in `buf` in the indicated directions. + /** Instantiate all type variables in `tvs` in the indicated directions, + * as described in the doc comment of `ToInstantiate`. * If a type variable A is instantiated from below, and there is another * type variable B in `buf` that is known to be smaller than A, wait and * instantiate all other type variables before trying to instantiate A again. @@ -695,20 +707,12 @@ trait Inferencing { this: Typer => * * V2 := V3, O2 := O3 */ - def doInstantiate(tvs: List[(TypeVar, Int)]): Unit = - def excluded = typeVarsIn(tvs) - def tryInstantiate(tvs: List[(TypeVar, Int)]): List[(TypeVar, Int)] = tvs match + def doInstantiate(tvs: ToInstantiate): Unit = + + /** Try to instantiate `tvs`, return any suspended type variables */ + def tryInstantiate(tvs: ToInstantiate): ToInstantiate = tvs match case (hd @ (tvar, v)) :: tvs1 => - val fromBelow = - if v == 0 then - val aboveOK = true // !constraint.dependsOn(tvar, excluded, co = true, track = true) - val belowOK = true // !constraint.dependsOn(tvar, excluded, co = false, track = true) - assert(aboveOK, i"$tvar, excluded = ${excluded.toList}, $constraint") - assert(belowOK, i"$tvar, excluded = ${excluded.toList}, $constraint") - if aboveOK == belowOK then tvar.hasLowerBound - else belowOK - else - v == 1 + val fromBelow = v == 1 || (v == 0 && tvar.hasLowerBound) typr.println( i"interpolate${if v == 0 then " non-occurring" else ""} $tvar in $state in $tree: $tp, fromBelow = $fromBelow, $constraint") if tvar.isInstantiated then @@ -728,16 +732,12 @@ trait Inferencing { this: Typer => case Nil => Nil if tvs.nonEmpty then doInstantiate(tryInstantiate(tvs)) end doInstantiate - val toInst = toInstantiate.toList - if toInst.nonEmpty then - typr.println(i"interpolating $toInst for $tp/$pt in $constraint") - val filtered = filterByDeps(toInst) - typr.println(i"filtered $filtered") - doInstantiate(filtered) + + doInstantiate(filterByDeps(toInstantiate)) } - } + end if tree - } + end interpolateTypeVars /** If `tvar` represents a parameter of a dependent method type in the current `call` * approximate it from below with the type of the actual argument. Skolemize that From 45156b28e9908808333e9a131099fb64ee9684f7 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 9 Oct 2022 15:22:43 +0200 Subject: [PATCH 14/17] Address review comments --- .../tools/dotc/core/OrderingConstraint.scala | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 39b396b02f90..e8fe19e2fa89 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -316,7 +316,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, extends TypeTraverser, ConstraintAwareTraversal[Unit]: var add: Boolean = compiletime.uninitialized - private val seen = util.HashSet[LazyRef]() + val seen = util.HashSet[LazyRef]() def update(deps: ReverseDeps, referenced: TypeParamRef): ReverseDeps = val prev = deps.at(referenced) @@ -352,6 +352,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, def adjustReferenced(bound: Type, isLower: Boolean, add: Boolean) = adjuster.variance = if isLower then 1 else -1 adjuster.add = add + adjuster.seen.clear() adjuster.traverse(bound) /** Use an optimized strategy to adjust dependencies to account for the delta @@ -695,13 +696,17 @@ class OrderingConstraint(private val boundsMap: ParamBounds, case tvar: TypeVar => if tvar.isInstantiated then - // That's the case if replace is called from TypeVar's instantiateWith. - // Forget about instantiation for old dependencies. + // If the type variuable has been instantiated, we need to forget about + // the instantiation for old dependencies. + // I.e. to find out what the old entry was, we should not follow + // the newly instantiated type variable but assume the type variable's origin `param`. + // An example where this happens is if `replace` is called from TypeVar's `instantiateWith`. oldDepEntry = mapReplacedTypeVarTo(param)(oldDepEntry) else - // That's the case if replace is called from unify. - // Assume parameter has been replaced for new dependencies - // (the actual replacement is done below). + // If the type variuable has not been instantiated, we need to replace references to it + // in the new entry by `replacement`. Otherwise we would get stuck in an uninstantiated + // type variable. + // An example where this happens is if `replace` is called from unify. newDepEntry = mapReplacedTypeVarTo(replacement)(newDepEntry) case _ => if oldDepEntry ne newDepEntry then From 8491d2d96fd849d09b13a7e0bd0d95ba7706fe0f Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 30 Oct 2022 18:53:16 +0100 Subject: [PATCH 15/17] Fix typos --- compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index e8fe19e2fa89..018ab7fb1f4b 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -284,7 +284,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, * arguments Ts. That can make a difference for the variance in which an argument * is traversed. Example constraint: * - * constrainded types: C[X], A + * constrained types: C[X], A * A >: C[B] * C := Option * @@ -696,14 +696,14 @@ class OrderingConstraint(private val boundsMap: ParamBounds, case tvar: TypeVar => if tvar.isInstantiated then - // If the type variuable has been instantiated, we need to forget about + // If the type variable has been instantiated, we need to forget about // the instantiation for old dependencies. // I.e. to find out what the old entry was, we should not follow // the newly instantiated type variable but assume the type variable's origin `param`. // An example where this happens is if `replace` is called from TypeVar's `instantiateWith`. oldDepEntry = mapReplacedTypeVarTo(param)(oldDepEntry) else - // If the type variuable has not been instantiated, we need to replace references to it + // If the type variable has not been instantiated, we need to replace references to it // in the new entry by `replacement`. Otherwise we would get stuck in an uninstantiated // type variable. // An example where this happens is if `replace` is called from unify. From 854c35dd6fed56b25f52d3fb914930e8edb49d46 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 30 Oct 2022 19:06:38 +0100 Subject: [PATCH 16/17] Make tyconTypeParams only depend on current constraint --- compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index 018ab7fb1f4b..f77310c19fa7 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -299,7 +299,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.isInstantiated => tparams(tycon.origin) + case tycon: TypeVar if !tycon.inst.exists => tparams(tycon.origin) case tycon: TypeParamRef => entry(tycon) match case _: TypeBounds => tp.tyconTypeParams From 145d2ba41f351186f4b77010e5d295f79fa793b6 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 31 Oct 2022 07:57:06 +0100 Subject: [PATCH 17/17] Apply suggestions from code review Co-authored-by: Guillaume Martres --- compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala index f77310c19fa7..ac6cb78f9e91 100644 --- a/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala +++ b/compiler/src/dotty/tools/dotc/core/OrderingConstraint.scala @@ -267,7 +267,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, override def dependsOn(tv: TypeVar, except: TypeVars, co: Boolean)(using Context): Boolean = def origin(tv: TypeVar) = - assert(!tv.isInstantiated) + assert(!instType(tv).exists) tv.origin val param = origin(tv) val excluded = except.map(origin) @@ -694,7 +694,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds, var newDepEntry = newEntry replacedTypeVar match case tvar: TypeVar => - if tvar.isInstantiated + if tvar.inst.exists // `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.