Skip to content

Commit

Permalink
Read argument of parent constructors in Templates lazily
Browse files Browse the repository at this point in the history
Avoid reading the arguments of parent constructors unless someone forces them.
We don't need them to determine the parent types of the class to which the
template belongs. This makes TreeUnpickler as lazy as Namer in this respect
and therefore avoids CyclicReferences during unpickling.

Fixes #16673
  • Loading branch information
odersky committed Jan 13, 2023
1 parent ed81385 commit dc41bfd
Show file tree
Hide file tree
Showing 14 changed files with 117 additions and 41 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] =>
*/
def parentsKind(parents: List[Tree])(using Context): FlagSet = parents match {
case Nil => NoInitsInterface
case Apply(_, _ :: _) :: _ => EmptyFlags
case Apply(_, _ :: _) :: _ | Block(_, _) :: _ => EmptyFlags
case _ :: parents1 => parentsKind(parents1)
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ class TreeMapWithImplicits extends tpd.TreeMapWithPreciseStatContexts {
transform(tree.tpt),
transform(tree.rhs)(using nestedScopeCtx(tree.paramss.flatten)))
}
case impl @ Template(constr, parents, self, _) =>
case impl @ Template(constr, _, self, _) =>
cpy.Template(tree)(
transformSub(constr),
transform(parents)(using ctx.superCallContext),
transform(impl.parents)(using ctx.superCallContext),
Nil,
transformSelf(self),
transformStats(impl.body, tree.symbol))
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ class TreeTypeMap(
cpy.Inlined(tree)(call, bindings1, expanded1)

override def transform(tree: tpd.Tree)(using Context): tpd.Tree = treeMap(tree) match {
case impl @ Template(constr, parents, self, _) =>
case impl @ Template(constr, _, self, _) =>
val tmap = withMappedSyms(localSyms(impl :: self :: Nil))
cpy.Template(impl)(
constr = tmap.transformSub(constr),
parents = parents.mapconserve(transform),
parents = impl.parents.mapconserve(transform),
self = tmap.transformSub(self),
body = impl.body mapconserve
(tmap.transform(_)(using ctx.withOwner(mapOwner(impl.symbol.owner))))
Expand Down
26 changes: 20 additions & 6 deletions compiler/src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -855,16 +855,30 @@ object Trees {
* if this is of class untpd.DerivingTemplate.
* Typed templates only have parents.
*/
case class Template[+T <: Untyped] private[ast] (constr: DefDef[T], parentsOrDerived: List[Tree[T]], self: ValDef[T], private var preBody: LazyTreeList[T])(implicit @constructorOnly src: SourceFile)
case class Template[+T <: Untyped] private[ast] (constr: DefDef[T], private var myParentsOrDerived: LazyTreeList[T], self: ValDef[T], private var preBody: LazyTreeList[T])(implicit @constructorOnly src: SourceFile)
extends DefTree[T] with WithLazyField[List[Tree[T]]] {
type ThisTree[+T <: Untyped] = Template[T]
def unforcedBody: LazyTreeList[T] = unforced
def unforced: LazyTreeList[T] = preBody

protected def force(x: List[Tree[T @uncheckedVariance]]): Unit = preBody = x

// The post-condition of forceIfLazy is that all lazy fields are trees, so
// we need to force parentsAndDerived as well as body.
override def forceIfLazy(using Context) =
parentsOrDerived
super.forceIfLazy

def body(using Context): List[Tree[T]] = forceIfLazy

def parents: List[Tree[T]] = parentsOrDerived // overridden by DerivingTemplate
def derived: List[untpd.Tree] = Nil // overridden by DerivingTemplate
def parentsOrDerived(using Context): List[Tree[T]] = myParentsOrDerived match
case latePs: Lazy[List[Tree[T]]] @unchecked =>
myParentsOrDerived = latePs.complete
parentsOrDerived
case ps: List[Tree[T]] @unchecked => ps

def parents(using Context): List[Tree[T]] = parentsOrDerived // overridden by DerivingTemplate
def derived: List[untpd.Tree] = Nil // overridden by DerivingTemplate
}


Expand Down Expand Up @@ -1355,7 +1369,7 @@ object Trees {
DefDef(tree: Tree)(name, paramss, tpt, rhs)
def TypeDef(tree: TypeDef)(name: TypeName = tree.name, rhs: Tree = tree.rhs)(using Context): TypeDef =
TypeDef(tree: Tree)(name, rhs)
def Template(tree: Template)(constr: DefDef = tree.constr, parents: List[Tree] = tree.parents, derived: List[untpd.Tree] = tree.derived, self: ValDef = tree.self, body: LazyTreeList = tree.unforcedBody)(using Context): Template =
def Template(tree: Template)(using Context)(constr: DefDef = tree.constr, parents: List[Tree] = tree.parents, derived: List[untpd.Tree] = tree.derived, self: ValDef = tree.self, body: LazyTreeList = tree.unforcedBody): Template =
Template(tree: Tree)(constr, parents, derived, self, body)
def Hole(tree: Hole)(isTerm: Boolean = tree.isTerm, idx: Int = tree.idx, args: List[Tree] = tree.args, content: Tree = tree.content, tpt: Tree = tree.tpt)(using Context): Hole =
Hole(tree: Tree)(isTerm, idx, args, content, tpt)
Expand Down Expand Up @@ -1618,8 +1632,8 @@ object Trees {
inContext(localCtx(tree)) {
this(x, rhs)
}
case tree @ Template(constr, parents, self, _) if tree.derived.isEmpty =>
this(this(this(this(x, constr), parents), self), tree.body)
case tree @ Template(constr, _, self, _) if tree.derived.isEmpty =>
this(this(this(this(x, constr), tree.parents), self), tree.body)
case Import(expr, _) =>
this(x, expr)
case Export(expr, _) =>
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/ast/untpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
*/
class DerivingTemplate(constr: DefDef, parentsOrDerived: List[Tree], self: ValDef, preBody: LazyTreeList, derivedCount: Int)(implicit @constructorOnly src: SourceFile)
extends Template(constr, parentsOrDerived, self, preBody) {
override val parents = parentsOrDerived.dropRight(derivedCount)
private val myParents = parentsOrDerived.dropRight(derivedCount)
override def parents(using Context) = myParents
override val derived = parentsOrDerived.takeRight(derivedCount)
}

Expand Down Expand Up @@ -415,6 +416,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
def Template(constr: DefDef, parents: List[Tree], derived: List[Tree], self: ValDef, body: LazyTreeList)(implicit src: SourceFile): Template =
if (derived.isEmpty) new Template(constr, parents, self, body)
else new DerivingTemplate(constr, parents ++ derived, self, body, derived.length)
def Template(constr: DefDef, parents: LazyTreeList, self: ValDef, body: LazyTreeList)(implicit src: SourceFile): Template =
new Template(constr, parents, self, body)
def Import(expr: Tree, selectors: List[ImportSelector])(implicit src: SourceFile): Import = new Import(expr, selectors)
def Export(expr: Tree, selectors: List[ImportSelector])(implicit src: SourceFile): Export = new Export(expr, selectors)
def PackageDef(pid: RefTree, stats: List[Tree])(implicit src: SourceFile): PackageDef = new PackageDef(pid, stats)
Expand Down
74 changes: 64 additions & 10 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,51 @@ class TreeUnpickler(reader: TastyReader,
tree.setDefTree
}

/** Read enough of parent to determine its type, without reading arguments
* of applications. This is necessary to make TreeUnpickler as lazy as Namer
* in this regard. See i16673 for a test case.
*/
private def readParentType()(using Context): Type =
readByte() match
case TYPEAPPLY =>
val end = readEnd()
val tycon = readParentType()
if tycon.typeParams.isEmpty then
goto(end)
tycon
else
val args = until(end)(readTpt())
val cls = tycon.classSymbol
assert(cls.typeParams.hasSameLengthAs(args))
cls.typeRef.appliedTo(args.tpes)
case APPLY | BLOCK =>
val end = readEnd()
try readParentType()
finally goto(end)
case SELECTin =>
val end = readEnd()
readName()
readTerm() match
case nu: New =>
try nu.tpe
finally goto(end)
case SHAREDterm =>
forkAt(readAddr()).readParentType()

/** Read template parents
* @param withArgs if false, only read enough of parent trees to determine their type
* but skip constructor arguments. Return any trees that were partially
* parsed in this way as InferredTypeTrees.
*/
def readParents(withArgs: Boolean)(using Context): List[Tree] =
collectWhile(nextByte != SELFDEF && nextByte != DEFDEF) {
nextUnsharedTag match
case APPLY | TYPEAPPLY | BLOCK =>
if withArgs then readTerm()
else InferredTypeTree().withType(readParentType())
case _ => readTpt()
}

private def readTemplate(using Context): Template = {
val start = currentAddr
assert(sourcePathAt(start).isEmpty)
Expand All @@ -979,12 +1024,8 @@ class TreeUnpickler(reader: TastyReader,
while (bodyIndexer.reader.nextByte != DEFDEF) bodyIndexer.skipTree()
bodyIndexer.indexStats(end)
}
val parents = collectWhile(nextByte != SELFDEF && nextByte != DEFDEF) {
nextUnsharedTag match {
case APPLY | TYPEAPPLY | BLOCK => readTerm()(using parentCtx)
case _ => readTpt()(using parentCtx)
}
}
val parentReader = fork
val parents = readParents(withArgs = false)(using parentCtx)
val parentTypes = parents.map(_.tpe.dealias)
val self =
if (nextByte == SELFDEF) {
Expand All @@ -998,7 +1039,13 @@ class TreeUnpickler(reader: TastyReader,
selfInfo = if (self.isEmpty) NoType else self.tpt.tpe)
.integrateOpaqueMembers
val constr = readIndexedDef().asInstanceOf[DefDef]
val mappedParents = parents.map(_.changeOwner(localDummy, constr.symbol))
val mappedParents: LazyTreeList =
if parents.exists(_.isInstanceOf[InferredTypeTree]) then
// parents were not read fully, will need to be read again later on demand
new LazyReader(parentReader, localDummy, ctx.mode, ctx.source,
_.readParents(withArgs = true)
.map(_.changeOwner(localDummy, constr.symbol)))
else parents

val lazyStats = readLater(end, rdr => {
val stats = rdr.readIndexedStats(localDummy, end)
Expand All @@ -1007,7 +1054,7 @@ class TreeUnpickler(reader: TastyReader,
defn.patchStdLibClass(cls)
NamerOps.addConstructorProxies(cls)
setSpan(start,
untpd.Template(constr, mappedParents, Nil, self, lazyStats)
untpd.Template(constr, mappedParents, self, lazyStats)
.withType(localDummy.termRef))
}

Expand Down Expand Up @@ -1325,8 +1372,15 @@ class TreeUnpickler(reader: TastyReader,
NoDenotation

val denot =
val d = ownerTpe.decl(name).atSignature(sig, target)
(if !d.exists then lookupInSuper else d).asSeenFrom(prefix)
if true then
val d = ownerTpe.decl(name).atSignature(sig, target)
(if !d.exists then lookupInSuper else d).asSeenFrom(prefix)
else
var d = ownerTpe.decl(name)
if d.isOverloaded then d = d.atSignature(sig, target)
if !d.exists then lookupInSuper
else if d.symbol.isConstructor then d
else d.asSeenFrom(prefix)

makeSelect(qual, name, denot)
case REPEATED =>
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/interactive/Interactive.scala
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,8 @@ object Interactive {
case _ =>
}
localCtx
case tree @ Template(constr, parents, self, _) =>
if ((constr :: self :: parents).contains(nested)) outer
case tree @ Template(constr, _, self, _) =>
if ((constr :: self :: tree.parentsOrDerived).contains(nested)) outer
else contextOfStat(tree.body, nested, tree.symbol, outer.inClassContext(self.symbol))
case _ =>
outer
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/MacroTransform.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ abstract class MacroTransform extends Phase {
tree
case _: PackageDef | _: MemberDef =>
super.transform(tree)(using localCtx(tree))
case impl @ Template(constr, parents, self, _) =>
case impl @ Template(constr, _, self, _) =>
cpy.Template(tree)(
transformSub(constr),
transform(parents)(using ctx.superCallContext),
transform(impl.parents)(using ctx.superCallContext),
Nil,
transformSelf(self),
transformStats(impl.body, tree.symbol))
Expand Down
22 changes: 11 additions & 11 deletions compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -710,16 +710,16 @@ object TreeChecker {
object TreeNodeChecker extends untpd.TreeTraverser:
import untpd._
def traverse(tree: Tree)(using Context) = tree match
case t: TypeTree => assert(assertion = false, i"TypeTree not expected: $t")
case t @ TypeApply(fun, _targs) => traverse(fun)
case t @ New(_tpt) =>
case t @ Typed(expr, _tpt) => traverse(expr)
case t @ Closure(env, meth, _tpt) => traverse(env); traverse(meth)
case t @ SeqLiteral(elems, _elemtpt) => traverse(elems)
case t @ ValDef(_, _tpt, _) => traverse(t.rhs)
case t @ DefDef(_, paramss, _tpt, _) => for params <- paramss do traverse(params); traverse(t.rhs)
case t @ TypeDef(_, _rhs) =>
case t @ Template(constr, parents, self, _) => traverse(constr); traverse(parents); traverse(self); traverse(t.body)
case t => traverseChildren(t)
case t: TypeTree => assert(assertion = false, i"TypeTree not expected: $t")
case t @ TypeApply(fun, _targs) => traverse(fun)
case t @ New(_tpt) =>
case t @ Typed(expr, _tpt) => traverse(expr)
case t @ Closure(env, meth, _tpt) => traverse(env); traverse(meth)
case t @ SeqLiteral(elems, _elemtpt) => traverse(elems)
case t @ ValDef(_, _tpt, _) => traverse(t.rhs)
case t @ DefDef(_, paramss, _tpt, _) => for params <- paramss do traverse(params); traverse(t.rhs)
case t @ TypeDef(_, _rhs) =>
case t @ Template(constr, _, self, _) => traverse(constr); traverse(t.parentsOrDerived); traverse(self); traverse(t.body)
case t => traverseChildren(t)
end traverse
}
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1390,9 +1390,9 @@ trait Checking {

if (stat.symbol.isAllOf(EnumCase))
stat match {
case TypeDef(_, Template(DefDef(_, paramss, _, _), parents, _, _)) =>
case TypeDef(_, impl @ Template(DefDef(_, paramss, _, _), _, _, _)) =>
paramss.foreach(_.foreach(check))
parents.foreach(check)
impl.parents.foreach(check)
case vdef: ValDef =>
vdef.rhs match {
case Block((clsDef @ TypeDef(_, impl: Template)) :: Nil, _)
Expand Down
4 changes: 2 additions & 2 deletions compiler/test/dotty/tools/dotc/parsing/DeSugarTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ class DeSugarTest extends ParserTest {
cpy.DefDef(tree1)(name, transformParamss(paramss), transform(tpt, Type), transform(tree1.rhs))
case tree1 @ TypeDef(name, rhs) =>
cpy.TypeDef(tree1)(name, transform(rhs, Type))
case impl @ Template(constr, parents, self, _) =>
cpy.Template(tree1)(transformSub(constr), transform(parents), Nil, transformSub(self), transform(impl.body, Expr))
case impl @ Template(constr, _, self, _) =>
cpy.Template(tree1)(transformSub(constr), transform(impl.parentsOrDerived), Nil, transformSub(self), transform(impl.body, Expr))
case Thicket(trees) =>
Thicket(flatten(trees mapConserve super.transform))
case tree1 =>
Expand Down
2 changes: 2 additions & 0 deletions tests/pos/i16673/FooStub_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
abstract class FooStub(val that: Foo):
val bar = 1337;
2 changes: 2 additions & 0 deletions tests/pos/i16673/FooStub_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
abstract class FooStub(val that: Foo):
val bar = 1337;
1 change: 1 addition & 0 deletions tests/pos/i16673/Foo_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
final class Foo(_that: Foo) extends FooStub(_that)

0 comments on commit dc41bfd

Please sign in to comment.