Skip to content

Commit 44d6f10

Browse files
committed
Bring back recompilation of types after typer.
Previous cpy was almost unconditionally re-creating trees for Apply, TypeApply and Select, which forced huge portions of other trees to be recomputed. In particular, if forced to recompute types of selections with a complex prefix. Those selections would change _signature_ when recomputed. Unfortunately, some types that typer are initialised by "lastSymbol" set by hand. Those type don't correctly recompute their signatures. They survive with wrong signature and are pickled with wrong signature. I think this is a bug, but I'm re-creating the way this bug was hidden before by making FirstTransform copy the entire tree using previous logic.
1 parent 84c26ec commit 44d6f10

File tree

4 files changed

+26
-11
lines changed

4 files changed

+26
-11
lines changed

compiler/src/dotty/tools/dotc/ast/tpd.scala

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -481,8 +481,19 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
481481
val tree1 = untpd.cpy.Select(tree)(qualifier, name)
482482
lazy val oldMember = tree.asInstanceOf[Select].qualifier.tpe.member(name)
483483
tree match {
484-
case tree: Select if (qualifier.tpe eq tree.qualifier.tpe) && (oldMember.isOverloaded || oldMember.signature == qualifier.tpe.member(name).signature) =>
485-
tree1.withTypeUnchecked(tree.tpe)
484+
case tree: Select if (qualifier.tpe eq tree.qualifier.tpe) =>
485+
tree.tpe match {
486+
case tpe: NamedType =>
487+
val tp = tpe.derivedSelect(qualifier.tpe.widenIfUnstable)
488+
if (tp == tree.tpe)
489+
// Unfortunately if qualifiers are the same, the widened type may be different from the one that
490+
// was used to construct the tree. And even time travel won't help here.
491+
// By the time we call typedSelect not all type-variables may be instantiated
492+
// and the selection may succeed with a weaker signature than the one we'll get here.
493+
tree.withTypeUnchecked(tree.tpe)
494+
else tree1.withType(tpe.derivedSelect(qualifier.tpe.widenIfUnstable))
495+
case _ => tree1.withTypeUnchecked(tree.tpe)
496+
}
486497
case _ => tree.tpe match {
487498
case tpe: NamedType => tree1.withType(tpe.derivedSelect(qualifier.tpe.widenIfUnstable))
488499
case _ => tree1.withTypeUnchecked(tree.tpe)
@@ -492,15 +503,15 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
492503

493504
override def Apply(tree: Tree)(fun: Tree, args: List[Tree])(implicit ctx: Context): Apply = {
494505
val untyped = untpd.cpy.Apply(tree)(fun, args)
495-
if (untyped eq tree)
506+
if (false && (untyped eq tree))
496507
tree.asInstanceOf[Apply]
497508
else
498509
ta.assignType(untyped, fun, args)
499510
}
500511

501512
override def TypeApply(tree: Tree)(fun: Tree, args: List[Tree])(implicit ctx: Context): TypeApply = {
502513
val untyped = untpd.cpy.TypeApply(tree)(fun, args)
503-
if (untyped eq tree)
514+
if (false && (untyped eq tree))
504515
tree.asInstanceOf[TypeApply]
505516
else
506517
ta.assignType(untyped, fun, args)
@@ -613,6 +624,9 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
613624
// does not work here: The computed type depends on the widened function type, not
614625
// the function type itself. A treetransform may keep the function type the
615626
// same but its widened type might change.
627+
// It's not only tree transform unfortunatelly. Typer may also create types of Apply nodes that are less precise
628+
// than the one that we'would expect because he didn't yet instantiate all type-variables
629+
// see https://github.com/lampepfl/dotty/pull/2446 for a testcase
616630
override def Apply(tree: tpd.Tree)(fun: tpd.Tree, args: List[tpd.Tree])(implicit ctx: Context): tpd.Apply = {
617631
ta.assignType(untpd.cpy.Apply(tree)(fun, args), fun, args)
618632
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import Contexts._
99
import Symbols._
1010
import Flags.PackageVal
1111
import Decorators._
12+
import dotty.tools.dotc.ast.tpd
1213

1314
/** A base class for transforms.
1415
* A transform contains a compiler phase which applies a tree transformer.
@@ -30,7 +31,7 @@ abstract class MacroTransform extends Phase {
3031
*/
3132
protected def transformPhase(implicit ctx: Context): Phase = this
3233

33-
class Transformer extends TreeMap {
34+
class Transformer(cpy: TreeCopier = tpd.cpy) extends TreeMap(cpy) {
3435

3536
protected def localCtx(tree: Tree)(implicit ctx: Context) = {
3637
val sym = tree.symbol

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
8787
case _ => tree
8888
}
8989

90-
class PostTyperTransformer extends Transformer {
90+
class PostTyperTransformer extends Transformer(tpd.cpy_slow) {
9191

9292
private var inJavaAnnot: Boolean = false
9393

@@ -119,7 +119,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
119119
val qual = tree.qualifier
120120
qual.symbol.moduleClass.denot match {
121121
case pkg: PackageClassDenotation if !tree.symbol.maybeOwner.is(Package) =>
122-
transformSelect(cpy.Select(tree)(qual select pkg.packageObj.symbol, tree.name), targs)
122+
transformSelect(cpy_slow.Select(tree)(qual select pkg.packageObj.symbol, tree.name), targs)
123123
case _ =>
124124
val tree1 = super.transform(tree)
125125
constToLiteral(tree1) match {
@@ -189,7 +189,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
189189
case sel: Select =>
190190
val args1 = transform(args)
191191
val sel1 = transformSelect(sel, args1)
192-
if (superAcc.isProtectedAccessor(sel1)) sel1 else cpy.TypeApply(tree1)(sel1, args1)
192+
if (superAcc.isProtectedAccessor(sel1)) sel1 else cpy_slow.TypeApply(tree1)(sel1, args1)
193193
case _ =>
194194
super.transform(tree1)
195195
}
@@ -207,7 +207,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
207207
// 2. To enable correct pickling (calls can share symbols with the inlined code, which
208208
// would trigger an assertion when pickling).
209209
val callTrace = Ident(call.symbol.topLevelClass.typeRef).withPos(call.pos)
210-
cpy.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion))
210+
cpy_slow.Inlined(tree)(callTrace, transformSub(bindings), transform(expansion))
211211
case tree: Template =>
212212
val saved = parentNews
213213
parentNews ++= tree.parents.flatMap(newPart)
@@ -248,7 +248,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
248248
Checking.checkInstantiable(tree.tpe, tree.pos)
249249
super.transform(tree)
250250
case tree @ Annotated(annotated, annot) =>
251-
cpy.Annotated(tree)(transform(annotated), transformAnnot(annot))
251+
cpy_slow.Annotated(tree)(transform(annotated), transformAnnot(annot))
252252
case tree: AppliedTypeTree =>
253253
Checking.checkAppliedType(tree)
254254
super.transform(tree)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
357357

358358
private def typedSelect(tree: untpd.Select, pt: Type, qual: Tree)(implicit ctx: Context): Select =
359359
healNonvariant(
360-
checkValue(assignType(cpy.Select(tree)(qual, tree.name), qual), pt),
360+
checkValue(assignType(cpy.Select(tree)(qual, tree.name), qual), pt), // see note in TypedTreeCopier.select
361361
pt)
362362

363363
/** Let `tree = p.n` where `p: T`. If tree's type is an unsafe instantiation

0 commit comments

Comments
 (0)