Skip to content

Commit 5491e59

Browse files
committed
Fix #1757: Be more careful about positions of type variable binders
We interpolate a type variable if the current tree contains the type variables binding tree. Previously, this was the application owning the variable. However, sometimes this tree is transformed so that the containment test fails, and type variables are instantiated too late (in the case of #1757 this was never). We fix this by - setting the binding tree to the type tree that first contains the type variable - making sure that tree is never copied literally anywhere else. It's a tricky dance, but I believe we got it right now.
1 parent 47d2084 commit 5491e59

File tree

5 files changed

+24
-14
lines changed

5 files changed

+24
-14
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2862,14 +2862,14 @@ object Types {
28622862
*
28632863
* @param origin The parameter that's tracked by the type variable.
28642864
* @param creatorState The typer state in which the variable was created.
2865-
* @param owningTree The function part of the TypeApply tree tree that introduces
2866-
* the type variable.
2865+
* @param bindingTree The TypeTree which introduces the type variable, or EmptyTree
2866+
* if the type variable does not correspond to a source term.
28672867
* @paran owner The current owner if the context where the variable was created.
28682868
*
28692869
* `owningTree` and `owner` are used to determine whether a type-variable can be instantiated
28702870
* at some given point. See `Inferencing#interpolateUndetVars`.
28712871
*/
2872-
final class TypeVar(val origin: PolyParam, creatorState: TyperState, val owningTree: untpd.Tree, val owner: Symbol) extends CachedProxyType with ValueType {
2872+
final class TypeVar(val origin: PolyParam, creatorState: TyperState, val bindingTree: untpd.Tree, val owner: Symbol) extends CachedProxyType with ValueType {
28732873

28742874
/** The permanent instance type of the variable, or NoType is none is given yet */
28752875
private[core] var inst: Type = NoType

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,15 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
250250

251251
/** Splice new method reference into existing application */
252252
def spliceMeth(meth: Tree, app: Tree): Tree = app match {
253-
case Apply(fn, args) => Apply(spliceMeth(meth, fn), args)
254-
case TypeApply(fn, targs) => TypeApply(spliceMeth(meth, fn), targs)
253+
case Apply(fn, args) =>
254+
spliceMeth(meth, fn).appliedToArgs(args)
255+
case TypeApply(fn, targs) =>
256+
// Note: It is important that the type arguments `targs` are passed in new trees
257+
// instead of being spliced in literally. Otherwise, a type argument to a default
258+
// method could be constructed as the definition site of the type variable for
259+
// that default constructor. This would interpolate type variables too early,
260+
// causing lots of tests (among them tasty_unpickleScala2) to fail.
261+
spliceMeth(meth, fn).appliedToTypes(targs.tpes)
255262
case _ => meth
256263
}
257264

@@ -333,7 +340,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
333340
val getter = findDefaultGetter(n + numArgs(normalizedFun))
334341
if (getter.isEmpty) missingArg(n)
335342
else {
336-
addTyped(treeToArg(spliceMeth(getter withPos appPos, normalizedFun)), formal)
343+
addTyped(treeToArg(spliceMeth(getter withPos normalizedFun.pos, normalizedFun)), formal)
337344
matchArgs(args1, formals1, n + 1)
338345
}
339346
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,10 @@ object Inferencing {
216216
def interpolateUndetVars(tree: Tree, ownedBy: Symbol)(implicit ctx: Context): Unit = {
217217
val constraint = ctx.typerState.constraint
218218
val qualifies = (tvar: TypeVar) =>
219-
(tree contains tvar.owningTree) || ownedBy.exists && tvar.owner == ownedBy
219+
(tree contains tvar.bindingTree) || ownedBy.exists && tvar.owner == ownedBy
220220
def interpolate() = Stats.track("interpolateUndetVars") {
221221
val tp = tree.tpe.widen
222-
constr.println(s"interpolate undet vars in ${tp.show}, pos = ${tree.pos}, mode = ${ctx.mode}, undets = ${constraint.uninstVars map (tvar => s"${tvar.show}@${tvar.owningTree.pos}")}")
222+
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}")}")
223223
constr.println(s"qualifying undet vars: ${constraint.uninstVars filter qualifies map (tvar => s"$tvar / ${tvar.show}")}, constraint: ${constraint.show}")
224224

225225
val vs = variances(tp, qualifies)

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -353,20 +353,23 @@ object ProtoTypes {
353353
* Also, if `owningTree` is non-empty, add a type variable for each parameter.
354354
* @return The added polytype, and the list of created type variables.
355355
*/
356-
def constrained(pt: PolyType, owningTree: untpd.Tree)(implicit ctx: Context): (PolyType, List[TypeVar]) = {
356+
def constrained(pt: PolyType, owningTree: untpd.Tree)(implicit ctx: Context): (PolyType, List[TypeTree]) = {
357357
val state = ctx.typerState
358358
assert(!(ctx.typerState.isCommittable && owningTree.isEmpty),
359359
s"inconsistent: no typevars were added to committable constraint ${state.constraint}")
360360

361-
def newTypeVars(pt: PolyType): List[TypeVar] =
361+
def newTypeVars(pt: PolyType): List[TypeTree] =
362362
for (n <- (0 until pt.paramNames.length).toList)
363-
yield new TypeVar(PolyParam(pt, n), state, owningTree, ctx.owner)
363+
yield {
364+
val tt = new TypeTree().withPos(owningTree.pos)
365+
tt.withType(new TypeVar(PolyParam(pt, n), state, tt, ctx.owner))
366+
}
364367

365368
val added =
366369
if (state.constraint contains pt) pt.newLikeThis(pt.paramNames, pt.paramBounds, pt.resultType)
367370
else pt
368371
val tvars = if (owningTree.isEmpty) Nil else newTypeVars(added)
369-
ctx.typeComparer.addToConstraint(added, tvars)
372+
ctx.typeComparer.addToConstraint(added, tvars.tpes.asInstanceOf[List[TypeVar]])
370373
(added, tvars)
371374
}
372375

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,12 +1962,12 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
19621962
if (pt.isInstanceOf[PolyProto]) tree
19631963
else {
19641964
var typeArgs = tree match {
1965-
case Select(qual, nme.CONSTRUCTOR) => qual.tpe.widenDealias.argTypesLo
1965+
case Select(qual, nme.CONSTRUCTOR) => qual.tpe.widenDealias.argTypesLo.map(TypeTree)
19661966
case _ => Nil
19671967
}
19681968
if (typeArgs.isEmpty) typeArgs = constrained(poly, tree)._2
19691969
convertNewGenericArray(
1970-
adaptInterpolated(tree.appliedToTypes(typeArgs), pt, original))
1970+
adaptInterpolated(tree.appliedToTypeTrees(typeArgs), pt, original))
19711971
}
19721972
case wtp =>
19731973
pt match {

0 commit comments

Comments
 (0)