Skip to content

Commit 1060d12

Browse files
committed
New interpolation scheme
Major changes from previous one: - We explicitly keep track in typer of which variables should and which should not be interpolated. This replaces searching trees for embedded variable definitions, which is fragile e.g. in the presence of eta expansion. - We compute variances starting with all variables found in the type, not just teh qualifying ones. The previous scheme caused some variance information to be missed, which caused some variables to be mis-classified as non-occurring. i4032.scala is a test case. Unfortunately, fixing this caused several other tricky inference failures because which were previously hidden because some variables were already instantiated prematurely. Examples were hamp.scala, hmap-covariant.scala, and i2300.scala. - We interpolate at the end of typedUnadapted instead of at the beginning of `adapt`. Managing instantiatable variables turned out easier this way.
1 parent 85c3d48 commit 1060d12

12 files changed

+249
-205
lines changed

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

+7-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ package core
55
import Types._
66
import Flags._
77
import Contexts._
8-
import util.{SimpleIdentityMap, DotClass}
8+
import util.{SimpleIdentityMap, SimpleIdentitySet, DotClass}
99
import reporting._
1010
import printing.{Showable, Printer}
1111
import printing.Texts._
@@ -76,6 +76,11 @@ class TyperState(previous: TyperState /* | Null */) {
7676
/** The uninstantiated variables */
7777
def uninstVars = constraint.uninstVars
7878

79+
/** The set of uninstantiated type varibles which have this state as their owning state */
80+
private[this] var myOwnedVars: TypeVars = SimpleIdentitySet.empty
81+
def ownedVars = myOwnedVars
82+
def ownedVars_=(vs: TypeVars): Unit = myOwnedVars = vs
83+
7984
/** Gives for each instantiated type var that does not yet have its `inst` field
8085
* set, the instance value stored in the constraint. Storing instances in constraints
8186
* is done only in a temporary way for contexts that may be retracted
@@ -154,6 +159,7 @@ class TyperState(previous: TyperState /* | Null */) {
154159
constraint foreachTypeVar { tvar =>
155160
if (tvar.owningState.get eq this) tvar.owningState = new WeakReference(targetState)
156161
}
162+
targetState.ownedVars ++= ownedVars
157163
targetState.gc()
158164
reporter.flush()
159165
isCommitted = true

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

+7-3
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import Denotations._
1818
import Periods._
1919
import util.Positions.{Position, NoPosition}
2020
import util.Stats._
21-
import util.DotClass
21+
import util.{DotClass, SimpleIdentitySet}
2222
import reporting.diagnostic.Message
2323
import reporting.diagnostic.messages.CyclicReferenceInvolving
2424
import ast.tpd._
@@ -2538,7 +2538,6 @@ object Types {
25382538
def join(implicit ctx: Context): Type = {
25392539
if (myJoinPeriod != ctx.period) {
25402540
myJoin = ctx.orDominator(this)
2541-
core.println(i"join of $this == $myJoin")
25422541
assert(myJoin != this)
25432542
myJoinPeriod = ctx.period
25442543
}
@@ -3377,7 +3376,10 @@ object Types {
33773376
private[core] def inst = myInst
33783377
private[core] def inst_=(tp: Type) = {
33793378
myInst = tp
3380-
if (tp.exists) owningState = null // no longer needed; null out to avoid a memory leak
3379+
if (tp.exists) {
3380+
owningState.get.ownedVars -= this
3381+
owningState = null // no longer needed; null out to avoid a memory leak
3382+
}
33813383
}
33823384

33833385
/** The state owning the variable. This is at first `creatorState`, but it can
@@ -3441,6 +3443,8 @@ object Types {
34413443
}
34423444
}
34433445

3446+
type TypeVars = SimpleIdentitySet[TypeVar]
3447+
34443448
// ------ ClassInfo, Type Bounds ------------------------------------------------------------
34453449

34463450
type TypeOrSymbol = AnyRef /* should be: Type | Symbol */

compiler/src/dotty/tools/dotc/transform/Erasure.scala

+5-3
Original file line numberDiff line numberDiff line change
@@ -675,14 +675,16 @@ object Erasure {
675675
super.typedStats(stats1, exprOwner).filter(!_.isEmpty)
676676
}
677677

678-
override def adapt(tree: Tree, pt: Type)(implicit ctx: Context): Tree =
678+
override def adapt(tree: Tree, pt: Type, locked: TypeVars)(implicit ctx: Context): Tree =
679679
trace(i"adapting ${tree.showSummary}: ${tree.tpe} to $pt", show = true) {
680-
assert(ctx.phase == ctx.erasurePhase.next, ctx.phase)
680+
assert(ctx.phase == ctx.erasurePhase || ctx.phase == ctx.erasurePhase.next, ctx.phase)
681681
if (tree.isEmpty) tree
682682
else if (ctx.mode is Mode.Pattern) tree // TODO: replace with assertion once pattern matcher is active
683683
else adaptToType(tree, pt)
684684
}
685-
}
685+
686+
override def simplify(tree: Tree, pt: Type, locked: TypeVars)(implicit ctx: Context): tree.type = tree
687+
}
686688

687689
def takesBridges(sym: Symbol)(implicit ctx: Context) =
688690
sym.isClass && !sym.is(Flags.Trait | Flags.Package)

compiler/src/dotty/tools/dotc/transform/TreeChecker.scala

+6-4
Original file line numberDiff line numberDiff line change
@@ -259,14 +259,14 @@ class TreeChecker extends Phase with SymTransformer {
259259
tpdTree
260260
}
261261

262-
override def typedUnadapted(tree: untpd.Tree, pt: Type)(implicit ctx: Context): tpd.Tree = {
262+
override def typedUnadapted(tree: untpd.Tree, pt: Type, locked: TypeVars)(implicit ctx: Context): tpd.Tree = {
263263
val res = tree match {
264264
case _: untpd.TypedSplice | _: untpd.Thicket | _: EmptyValDef[_] =>
265-
super.typedUnadapted(tree)
265+
super.typedUnadapted(tree, pt, locked)
266266
case _ if tree.isType =>
267267
promote(tree)
268268
case _ =>
269-
val tree1 = super.typedUnadapted(tree, pt)
269+
val tree1 = super.typedUnadapted(tree, pt, locked)
270270
def isSubType(tp1: Type, tp2: Type) =
271271
(tp1 eq tp2) || // accept NoType / NoType
272272
(tp1 <:< tp2)
@@ -435,7 +435,7 @@ class TreeChecker extends Phase with SymTransformer {
435435
override def ensureNoLocalRefs(tree: Tree, pt: Type, localSyms: => List[Symbol])(implicit ctx: Context): Tree =
436436
tree
437437

438-
override def adapt(tree: Tree, pt: Type)(implicit ctx: Context) = {
438+
override def adapt(tree: Tree, pt: Type, locked: TypeVars)(implicit ctx: Context) = {
439439
def isPrimaryConstructorReturn =
440440
ctx.owner.isPrimaryConstructor && pt.isRef(ctx.owner.owner) && tree.tpe.isRef(defn.UnitClass)
441441
if (ctx.mode.isExpr &&
@@ -449,6 +449,8 @@ class TreeChecker extends Phase with SymTransformer {
449449
})
450450
tree
451451
}
452+
453+
override def simplify(tree: Tree, pt: Type, locked: TypeVars)(implicit ctx: Context): tree.type = tree
452454
}
453455

454456
/**

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
546546
init()
547547

548548
def addArg(arg: Tree, formal: Type): Unit =
549-
typedArgBuf += adaptInterpolated(arg, formal.widenExpr)
549+
typedArgBuf += adapt(arg, formal.widenExpr)
550550

551551
def makeVarArg(n: Int, elemFormal: Type): Unit = {
552552
val args = typedArgBuf.takeRight(n).toList
@@ -711,7 +711,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
711711
* part. Return an optional value to indicate success.
712712
*/
713713
def tryWithImplicitOnQualifier(fun1: Tree, proto: FunProto)(implicit ctx: Context): Option[Tree] =
714-
tryInsertImplicitOnQualifier(fun1, proto) flatMap { fun2 =>
714+
tryInsertImplicitOnQualifier(fun1, proto, ctx.typerState.ownedVars) flatMap { fun2 =>
715715
tryEither {
716716
implicit ctx => Some(simpleApply(fun2, proto)): Option[Tree]
717717
} {
@@ -1519,11 +1519,11 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
15191519
* If the resulting trees all have the same type, return them instead of the original ones.
15201520
*/
15211521
def harmonize(trees: List[Tree])(implicit ctx: Context): List[Tree] = {
1522-
def adapt(tree: Tree, pt: Type): Tree = tree match {
1523-
case cdef: CaseDef => tpd.cpy.CaseDef(cdef)(body = adapt(cdef.body, pt))
1524-
case _ => adaptInterpolated(tree, pt)
1522+
def adaptDeep(tree: Tree, pt: Type): Tree = tree match {
1523+
case cdef: CaseDef => tpd.cpy.CaseDef(cdef)(body = adaptDeep(cdef.body, pt))
1524+
case _ => adapt(tree, pt)
15251525
}
1526-
if (ctx.isAfterTyper) trees else harmonizeWith(trees)(_.tpe, adapt)
1526+
if (ctx.isAfterTyper) trees else harmonizeWith(trees)(_.tpe, adaptDeep)
15271527
}
15281528

15291529
/** Apply a transformation `harmonize` on the results of operation `op`.

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,12 @@ object ErrorReporting {
120120
val found1 = dropJavaMethod(found)
121121
val expected1 = dropJavaMethod(expected)
122122
if ((found1 eq found) != (expected eq expected1) && (found1 <:< expected1))
123-
"\n(Note that Scala's and Java's representation of this type differs)"
123+
i"""
124+
|(Note that Scala's and Java's representation of this type differs)"""
124125
else if (ctx.settings.explainTypes.value)
125-
"\n" + ctx.typerState.show + "\n" + TypeComparer.explained((found <:< expected)(_))
126+
i"""
127+
|${ctx.typerState.constraint}
128+
|${TypeComparer.explained((found <:< expected)(_))}"""
126129
else
127130
""
128131
}

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

+4-3
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ trait Implicits { self: Typer =>
768768
case result: SearchSuccess =>
769769
result.tstate.commit()
770770
implicits.println(i"success: $result")
771-
implicits.println(i"committing ${result.tstate.constraint} yielding ${ctx.typerState.constraint} ${ctx.typerState.hashesStr}")
771+
implicits.println(i"committing ${result.tstate.constraint} yielding ${ctx.typerState.constraint} in ${ctx.typerState}")
772772
result
773773
case result: SearchFailure if result.isAmbiguous =>
774774
val deepPt = pt.deepenProto
@@ -828,11 +828,12 @@ trait Implicits { self: Typer =>
828828
def typedImplicit(cand: Candidate, contextual: Boolean)(implicit ctx: Context): SearchResult = track("typedImplicit") { trace(i"typed implicit ${cand.ref}, pt = $pt, implicitsEnabled == ${ctx.mode is ImplicitsEnabled}", implicits, show = true) {
829829
val ref = cand.ref
830830
var generated: Tree = tpd.ref(ref).withPos(pos.startPos)
831+
val locked = ctx.typerState.ownedVars
831832
if (!argument.isEmpty)
832833
generated = typedUnadapted(
833834
untpd.Apply(untpd.TypedSplice(generated), untpd.TypedSplice(argument) :: Nil),
834-
pt)
835-
val generated1 = adapt(generated, pt)
835+
pt, locked)
836+
val generated1 = adapt(generated, pt, locked)
836837
lazy val shadowing =
837838
typed(untpd.Ident(cand.implicitRef.implicitName) withPos pos.toSynthetic, funProto)(
838839
nestedContext().addMode(Mode.ImplicitShadowing).setExploreTyperState())

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

+62-68
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import config.Printers.{typr, constr}
2020
import annotation.tailrec
2121
import reporting._
2222
import collection.mutable
23+
import config.Config
2324

2425
object Inferencing {
2526

@@ -161,12 +162,16 @@ object Inferencing {
161162
* - The prefix `p` of a selection `p.f`.
162163
* - The result expression `e` of a block `{s1; .. sn; e}`.
163164
*/
164-
def tvarsInParams(tree: Tree)(implicit ctx: Context): List[TypeVar] = {
165+
def tvarsInParams(tree: Tree, locked: TypeVars)(implicit ctx: Context): List[TypeVar] = {
165166
@tailrec def boundVars(tree: Tree, acc: List[TypeVar]): List[TypeVar] = tree match {
166167
case Apply(fn, _) => boundVars(fn, acc)
167168
case TypeApply(fn, targs) =>
168169
val tvars = targs.tpes.collect {
169-
case tvar: TypeVar if !tvar.isInstantiated && targs.contains(tvar.bindingTree) => tvar
170+
case tvar: TypeVar
171+
if !tvar.isInstantiated &&
172+
targs.contains(tvar.bindingTree) &&
173+
ctx.typerState.ownedVars.contains(tvar) &&
174+
!locked.contains(tvar) => tvar
170175
}
171176
boundVars(fn, acc ::: tvars)
172177
case Select(pre, _) => boundVars(pre, acc)
@@ -228,7 +233,7 @@ object Inferencing {
228233
* to instantiate undetermined type variables that occur non-variantly
229234
*/
230235
def maximizeType(tp: Type, pos: Position, fromScala2x: Boolean)(implicit ctx: Context): List[Symbol] = Stats.track("maximizeType") {
231-
val vs = variances(tp, alwaysTrue)
236+
val vs = variances(tp)
232237
val patternBound = new mutable.ListBuffer[Symbol]
233238
vs foreachBinding { (tvar, v) =>
234239
if (v == 1) tvar.instantiate(fromBelow = false)
@@ -265,14 +270,14 @@ object Inferencing {
265270
*
266271
* we want to instantiate U to x.type right away. No need to wait further.
267272
*/
268-
private def variances(tp: Type, include: TypeVar => Boolean)(implicit ctx: Context): VarianceMap = Stats.track("variances") {
273+
private def variances(tp: Type)(implicit ctx: Context): VarianceMap = Stats.track("variances") {
269274
val constraint = ctx.typerState.constraint
270275

271276
object accu extends TypeAccumulator[VarianceMap] {
272277
def setVariance(v: Int) = variance = v
273278
def apply(vmap: VarianceMap, t: Type): VarianceMap = t match {
274279
case t: TypeVar
275-
if !t.isInstantiated && (ctx.typerState.constraint contains t) && include(t) =>
280+
if !t.isInstantiated && ctx.typerState.constraint.contains(t) =>
276281
val v = vmap(t)
277282
if (v == null) vmap.updated(t, variance)
278283
else if (v == variance || v == 0) vmap
@@ -319,42 +324,41 @@ trait Inferencing { this: Typer =>
319324
import Inferencing._
320325
import tpd._
321326

322-
/** Interpolate those undetermined type variables in the widened type of this tree
323-
* which are introduced by type application contained in the tree.
324-
* If such a variable appears covariantly in type `tp` or does not appear at all,
325-
* approximate it by its lower bound. Otherwise, if it appears contravariantly
326-
* in type `tp` approximate it by its upper bound.
327-
* @param ownedBy if it is different from NoSymbol, all type variables owned by
328-
* `ownedBy` qualify, independent of position.
329-
* Without that second condition, it can be that certain variables escape
330-
* interpolation, for instance when their tree was eta-lifted, so
331-
* the typechecked tree is no longer the tree in which the variable
332-
* was declared. A concrete example of this phenomenon can be
333-
* observed when compiling core.TypeOps#asSeenFrom.
327+
/** Interpolate undetermined type variables in the widened type of this tree.
328+
* @param tree the tree whose type is interpolated
329+
* @param pt the expected result type
330+
* @param locked the set of type variables of the current typer state that cannot be interpolated
331+
* at the present time
332+
* Eligible for interpolation are all type variables owned by the current typerstate
333+
* that are not in locked. Type variables occurring co- (respectively, contra-) variantly in the type
334+
* are minimized (respectvely, maximized). Non occurring type variables are minimized if they
335+
* have a lower bound different from Nothing, maximized otherwise. Type variables appearing
336+
* non-variantly in the type are left untouched.
337+
*
338+
* Note that even type variables that do not appear directly in a type, can occur with
339+
* some variance in the type, because of the constraints. E.g if `X` occurs co-variantly in `T`
340+
* and we have a constraint
341+
*
342+
* Y <: X
343+
*
344+
* Then `Y` also occurs co-variantly in `T` because it needs to be minimized in order to constrain
345+
* `T` teh least. See `variances` for more detail.
334346
*/
335-
def interpolateUndetVars(tree: Tree, ownedBy: Symbol, pt: Type)(implicit ctx: Context): Unit = {
336-
val constraint = ctx.typerState.constraint
337-
val qualifies = (tvar: TypeVar) =>
338-
(tree contains tvar.bindingTree) || ownedBy.exists && tvar.owner == ownedBy
339-
def interpolate() = Stats.track("interpolateUndetVars") {
340-
val tp = tree.tpe.widen
341-
constr.println(s"interpolate undet vars in ${tp.show}, pos = ${tree.pos}, mode = ${ctx.mode}, undets = ${constraint.uninstVars map (tvar => s"${tvar.show}@${tvar.bindingTree.pos}")}")
342-
constr.println(s"qualifying undet vars: ${constraint.uninstVars filter qualifies map (tvar => s"$tvar / ${tvar.show}")}, constraint: ${constraint.show}")
343-
344-
val vs = variances(tp, qualifies)
345-
val hasUnreportedErrors = ctx.typerState.reporter match {
346-
case r: StoreReporter if r.hasErrors => true
347-
case _ => false
348-
}
349-
350-
var isConstrained = tree.isInstanceOf[Apply] || tree.tpe.isInstanceOf[MethodOrPoly]
351-
352-
def ensureConstrained() = if (!isConstrained) {
353-
isConstrained = true
347+
def interpolateTypeVars(tree: Tree, pt: Type, locked: TypeVars)(implicit ctx: Context): tree.type = {
348+
val state = ctx.typerState
349+
if (state.ownedVars.size > locked.size) {
350+
val qualifying = state.ownedVars -- locked
351+
typr.println(i"interpolate $tree: ${tree.tpe.widen} in $state, owned vars = ${state.ownedVars.toList}%, %, previous = ${locked.toList}%, % / ${state.constraint}")
352+
val resultAlreadyConstrained =
353+
tree.isInstanceOf[Apply] || tree.tpe.isInstanceOf[MethodOrPoly]
354+
if (!resultAlreadyConstrained)
354355
constrainResult(tree.tpe, pt)
355-
}
356+
// This is needed because it could establish singleton type upper bounds. See i2998.scala.
356357

357-
// Avoid interpolating variables if typerstate has unreported errors.
358+
val tp = tree.tpe.widen
359+
val vs = variances(tp)
360+
361+
// Avoid interpolating variables occurring in tree's type if typerstate has unreported errors.
358362
// Reason: The errors might reflect unsatisfiable constraints. In that
359363
// case interpolating without taking account the constraints risks producing
360364
// nonsensical types that then in turn produce incomprehensible errors.
@@ -374,39 +378,29 @@ trait Inferencing { this: Typer =>
374378
// found : Int(1)
375379
// required: String
376380
// val y: List[List[String]] = List(List(1))
377-
if (!hasUnreportedErrors)
378-
vs foreachBinding { (tvar, v) =>
379-
if (v != 0 && ctx.typerState.constraint.contains(tvar)) {
380-
// previous interpolations could have already instantiated `tvar`
381-
// through unification, that's why we have to check again whether `tvar`
382-
// is contained in the current constraint.
383-
typr.println(s"interpolate ${if (v == 1) "co" else "contra"}variant ${tvar.show} in ${tp.show}")
384-
ensureConstrained()
385-
tvar.instantiate(fromBelow = v == 1)
381+
val hasUnreportedErrors = state.reporter match {
382+
case r: StoreReporter if r.hasErrors => true
383+
case _ => false
384+
}
385+
def constraint = state.constraint
386+
for (tvar <- qualifying)
387+
if (!tvar.isInstantiated && state.constraint.contains(tvar)) {
388+
// Needs to be checked again, since previous interpolations could already have
389+
// instantiated `tvar` through unification.
390+
val v = vs(tvar)
391+
if (v == null) {
392+
typr.println(i"interpolate non-occurring $tvar in $state in $tree: $tp, fromBelow = ${tvar.hasLowerBound}, $constraint")
393+
tvar.instantiate(fromBelow = tvar.hasLowerBound)
386394
}
395+
else if (!hasUnreportedErrors)
396+
if (v.intValue != 0) {
397+
typr.println(i"interpolate $tvar in $state in $tree: $tp, fromBelow = ${v.intValue == 1}, $constraint")
398+
tvar.instantiate(fromBelow = v.intValue == 1)
399+
}
400+
else typr.println(i"no interpolation for nonvariant $tvar in $state")
387401
}
388-
for (tvar <- constraint.uninstVars)
389-
if (!(vs contains tvar) && qualifies(tvar)) {
390-
typr.println(s"instantiating non-occurring ${tvar.show} in ${tp.show} / $tp")
391-
ensureConstrained()
392-
tvar.instantiate(fromBelow = tvar.hasLowerBound)
393-
}
394-
}
395-
if (constraint.uninstVars exists qualifies) interpolate()
396-
}
397-
398-
/** The uninstantiated type variables introduced somehwere in `tree` */
399-
def uninstBoundVars(tree: Tree)(implicit ctx: Context): List[TypeVar] = {
400-
val buf = new mutable.ListBuffer[TypeVar]
401-
tree.foreachSubTree {
402-
case TypeApply(_, args) =>
403-
args.tpes.foreach {
404-
case tv: TypeVar if !tv.isInstantiated && tree.contains(tv.bindingTree) => buf += tv
405-
case _ =>
406-
}
407-
case _ =>
408402
}
409-
buf.toList
403+
tree
410404
}
411405
}
412406

0 commit comments

Comments
 (0)