Skip to content

Commit 535dd89

Browse files
authored
Unpickle arguments of parent constructors in Templates lazily (#16688)
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
2 parents ede74c4 + 67d6400 commit 535dd89

17 files changed

+140
-84
lines changed

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ package ast
44
import core.Contexts._
55
import core.Decorators._
66
import util.Spans._
7-
import Trees.{MemberDef, DefTree, WithLazyField}
7+
import Trees.{MemberDef, DefTree, WithLazyFields}
88
import dotty.tools.dotc.core.Types.AnnotatedType
99
import dotty.tools.dotc.core.Types.ImportType
1010
import dotty.tools.dotc.core.Types.Type
@@ -106,16 +106,14 @@ object NavigateAST {
106106
// FIXME: We shouldn't be manually forcing trees here, we should replace
107107
// our usage of `productIterator` by something in `Positioned` that takes
108108
// care of low-level details like this for us.
109-
p match {
110-
case p: WithLazyField[?] =>
111-
p.forceIfLazy
109+
p match
110+
case p: WithLazyFields => p.forceFields()
112111
case _ =>
113-
}
114112
val iterator = p match
115113
case defdef: DefTree[?] =>
116114
p.productIterator ++ defdef.mods.productIterator
117115
case _ =>
118-
p.productIterator
116+
p.productIterator
119117
childPath(iterator, p :: path)
120118
}
121119
else {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] =>
313313
*/
314314
def parentsKind(parents: List[Tree])(using Context): FlagSet = parents match {
315315
case Nil => NoInitsInterface
316-
case Apply(_, _ :: _) :: _ => EmptyFlags
316+
case Apply(_, _ :: _) :: _ | Block(_, _) :: _ => EmptyFlags
317317
case _ :: parents1 => parentsKind(parents1)
318318
}
319319

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ class TreeMapWithImplicits extends tpd.TreeMapWithPreciseStatContexts {
5555
transform(tree.tpt),
5656
transform(tree.rhs)(using nestedScopeCtx(tree.paramss.flatten)))
5757
}
58-
case impl @ Template(constr, parents, self, _) =>
58+
case impl @ Template(constr, _, self, _) =>
5959
cpy.Template(tree)(
6060
transformSub(constr),
61-
transform(parents)(using ctx.superCallContext),
61+
transform(impl.parents)(using ctx.superCallContext),
6262
Nil,
6363
transformSelf(self),
6464
transformStats(impl.body, tree.symbol))

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,11 @@ class TreeTypeMap(
9292
cpy.Inlined(tree)(call, bindings1, expanded1)
9393

9494
override def transform(tree: tpd.Tree)(using Context): tpd.Tree = treeMap(tree) match {
95-
case impl @ Template(constr, parents, self, _) =>
95+
case impl @ Template(constr, _, self, _) =>
9696
val tmap = withMappedSyms(localSyms(impl :: self :: Nil))
9797
cpy.Template(impl)(
9898
constr = tmap.transformSub(constr),
99-
parents = parents.mapconserve(transform),
99+
parents = impl.parents.mapconserve(transform),
100100
self = tmap.transformSub(self),
101101
body = impl.body mapconserve
102102
(tmap.transform(_)(using ctx.withOwner(mapOwner(impl.symbol.owner))))

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

Lines changed: 42 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -407,12 +407,12 @@ object Trees {
407407
}
408408

409409
/** A ValDef or DefDef tree */
410-
abstract class ValOrDefDef[+T <: Untyped](implicit @constructorOnly src: SourceFile) extends MemberDef[T] with WithLazyField[Tree[T]] {
410+
abstract class ValOrDefDef[+T <: Untyped](implicit @constructorOnly src: SourceFile) extends MemberDef[T], WithLazyFields {
411411
type ThisTree[+T <: Untyped] <: ValOrDefDef[T]
412412
def name: TermName
413413
def tpt: Tree[T]
414-
def unforcedRhs: LazyTree[T] = unforced
415-
def rhs(using Context): Tree[T] = forceIfLazy
414+
def unforcedRhs: LazyTree[T]
415+
def rhs(using Context): Tree[T]
416416
}
417417

418418
trait ValOrTypeDef[+T <: Untyped] extends MemberDef[T]:
@@ -808,8 +808,10 @@ object Trees {
808808
extends ValOrDefDef[T], ValOrTypeDef[T] {
809809
type ThisTree[+T <: Untyped] = ValDef[T]
810810
assert(isEmpty || (tpt ne genericEmptyTree))
811-
def unforced: LazyTree[T] = preRhs
812-
protected def force(x: Tree[T @uncheckedVariance]): Unit = preRhs = x
811+
812+
def unforcedRhs: LazyTree[T] = preRhs
813+
def forceFields()(using Context): Unit = preRhs = force(preRhs)
814+
def rhs(using Context): Tree[T] = { forceFields(); preRhs.asInstanceOf[Tree[T]] }
813815
}
814816

815817
/** mods def name[tparams](vparams_1)...(vparams_n): tpt = rhs */
@@ -818,8 +820,10 @@ object Trees {
818820
extends ValOrDefDef[T] {
819821
type ThisTree[+T <: Untyped] = DefDef[T]
820822
assert(tpt ne genericEmptyTree)
821-
def unforced: LazyTree[T] = preRhs
822-
protected def force(x: Tree[T @uncheckedVariance]): Unit = preRhs = x
823+
824+
def unforcedRhs: LazyTree[T] = preRhs
825+
def forceFields()(using Context): Unit = preRhs = force(preRhs)
826+
def rhs(using Context): Tree[T] = { forceFields(); preRhs.asInstanceOf[Tree[T]] }
823827

824828
def leadingTypeParams(using Context): List[TypeDef[T]] = paramss match
825829
case (tparams @ (tparam: TypeDef[_]) :: _) :: _ => tparams.asInstanceOf[List[TypeDef[T]]]
@@ -855,16 +859,20 @@ object Trees {
855859
* if this is of class untpd.DerivingTemplate.
856860
* Typed templates only have parents.
857861
*/
858-
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)
859-
extends DefTree[T] with WithLazyField[List[Tree[T]]] {
862+
case class Template[+T <: Untyped] private[ast] (constr: DefDef[T], private var preParentsOrDerived: LazyTreeList[T], self: ValDef[T], private var preBody: LazyTreeList[T])(implicit @constructorOnly src: SourceFile)
863+
extends DefTree[T] with WithLazyFields {
860864
type ThisTree[+T <: Untyped] = Template[T]
861-
def unforcedBody: LazyTreeList[T] = unforced
862-
def unforced: LazyTreeList[T] = preBody
863-
protected def force(x: List[Tree[T @uncheckedVariance]]): Unit = preBody = x
864-
def body(using Context): List[Tree[T]] = forceIfLazy
865865

866-
def parents: List[Tree[T]] = parentsOrDerived // overridden by DerivingTemplate
867-
def derived: List[untpd.Tree] = Nil // overridden by DerivingTemplate
866+
def forceFields()(using Context): Unit =
867+
preParentsOrDerived = force(preParentsOrDerived)
868+
preBody = force(preBody)
869+
870+
def unforcedBody: LazyTreeList[T] = preBody
871+
def body(using Context): List[Tree[T]] = { forceFields(); preBody.asInstanceOf[List[Tree[T]]] }
872+
def parentsOrDerived(using Context): List[Tree[T]] = { forceFields(); preParentsOrDerived.asInstanceOf[List[Tree[T]]] }
873+
874+
def parents(using Context): List[Tree[T]] = parentsOrDerived // overridden by DerivingTemplate
875+
def derived: List[untpd.Tree] = Nil // overridden by DerivingTemplate
868876
}
869877

870878

@@ -1008,30 +1016,27 @@ object Trees {
10081016

10091017
// ----- Lazy trees and tree sequences
10101018

1011-
/** A tree that can have a lazy field
1012-
* The field is represented by some private `var` which is
1013-
* accessed by `unforced` and `force`. Forcing the field will
1014-
* set the `var` to the underlying value.
1015-
*/
1016-
trait WithLazyField[+T <: AnyRef] {
1017-
def unforced: T | Lazy[T]
1018-
protected def force(x: T @uncheckedVariance): Unit
1019-
def forceIfLazy(using Context): T = unforced match {
1020-
case lzy: Lazy[T @unchecked] =>
1021-
val x = lzy.complete
1022-
force(x)
1023-
x
1024-
case x: T @ unchecked => x
1025-
}
1026-
}
1027-
10281019
/** A base trait for lazy tree fields.
10291020
* These can be instantiated with Lazy instances which
10301021
* can delay tree construction until the field is first demanded.
10311022
*/
1032-
trait Lazy[+T <: AnyRef] {
1023+
trait Lazy[+T <: AnyRef]:
10331024
def complete(using Context): T
1034-
}
1025+
1026+
/** A tree that can have a lazy fields.
1027+
* Such fields are variables of type `T | Lazy[T]`, for some tyope `T`.
1028+
*/
1029+
trait WithLazyFields:
1030+
1031+
/** If `x` is lazy, computes the underlying value */
1032+
protected def force[T <: AnyRef](x: T | Lazy[T])(using Context): T = x match
1033+
case x: Lazy[T] @unchecked => x.complete
1034+
case x: T @unchecked => x
1035+
1036+
/** Assigns all lazy fields their underlying non-lazy value. */
1037+
def forceFields()(using Context): Unit
1038+
1039+
end WithLazyFields
10351040

10361041
// ----- Generic Tree Instances, inherited from `tpt` and `untpd`.
10371042

@@ -1355,7 +1360,7 @@ object Trees {
13551360
DefDef(tree: Tree)(name, paramss, tpt, rhs)
13561361
def TypeDef(tree: TypeDef)(name: TypeName = tree.name, rhs: Tree = tree.rhs)(using Context): TypeDef =
13571362
TypeDef(tree: Tree)(name, rhs)
1358-
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 =
1363+
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 =
13591364
Template(tree: Tree)(constr, parents, derived, self, body)
13601365
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 =
13611366
Hole(tree: Tree)(isTerm, idx, args, content, tpt)
@@ -1618,8 +1623,8 @@ object Trees {
16181623
inContext(localCtx(tree)) {
16191624
this(x, rhs)
16201625
}
1621-
case tree @ Template(constr, parents, self, _) if tree.derived.isEmpty =>
1622-
this(this(this(this(x, constr), parents), self), tree.body)
1626+
case tree @ Template(constr, _, self, _) if tree.derived.isEmpty =>
1627+
this(this(this(this(x, constr), tree.parents), self), tree.body)
16231628
case Import(expr, _) =>
16241629
this(x, expr)
16251630
case Export(expr, _) =>

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
5454
*/
5555
class DerivingTemplate(constr: DefDef, parentsOrDerived: List[Tree], self: ValDef, preBody: LazyTreeList, derivedCount: Int)(implicit @constructorOnly src: SourceFile)
5656
extends Template(constr, parentsOrDerived, self, preBody) {
57-
override val parents = parentsOrDerived.dropRight(derivedCount)
57+
private val myParents = parentsOrDerived.dropRight(derivedCount)
58+
override def parents(using Context) = myParents
5859
override val derived = parentsOrDerived.takeRight(derivedCount)
5960
}
6061

@@ -415,6 +416,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
415416
def Template(constr: DefDef, parents: List[Tree], derived: List[Tree], self: ValDef, body: LazyTreeList)(implicit src: SourceFile): Template =
416417
if (derived.isEmpty) new Template(constr, parents, self, body)
417418
else new DerivingTemplate(constr, parents ++ derived, self, body, derived.length)
419+
def Template(constr: DefDef, parents: LazyTreeList, self: ValDef, body: LazyTreeList)(implicit src: SourceFile): Template =
420+
new Template(constr, parents, self, body)
418421
def Import(expr: Tree, selectors: List[ImportSelector])(implicit src: SourceFile): Import = new Import(expr, selectors)
419422
def Export(expr: Tree, selectors: List[ImportSelector])(implicit src: SourceFile): Export = new Export(expr, selectors)
420423
def PackageDef(pid: RefTree, stats: List[Tree])(implicit src: SourceFile): PackageDef = new PackageDef(pid, stats)

compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import dotty.tools.tasty.TastyBuffer
88
import TastyBuffer._
99

1010
import ast._
11-
import Trees.WithLazyField
11+
import Trees.WithLazyFields
1212
import util.{SourceFile, NoSource}
1313
import core._
1414
import Annotations._, Decorators._
@@ -80,7 +80,7 @@ class PositionPickler(
8080
def alwaysNeedsPos(x: Positioned) = x match {
8181
case
8282
// initialSpan is inaccurate for trees with lazy field
83-
_: WithLazyField[?]
83+
_: WithLazyFields
8484

8585
// A symbol is created before the corresponding tree is unpickled,
8686
// and its position cannot be changed afterwards.

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,51 @@ class TreeUnpickler(reader: TastyReader,
959959
tree.setDefTree
960960
}
961961

962+
/** Read enough of parent to determine its type, without reading arguments
963+
* of applications. This is necessary to make TreeUnpickler as lazy as Namer
964+
* in this regard. See i16673 for a test case.
965+
*/
966+
private def readParentType()(using Context): Type =
967+
readByte() match
968+
case TYPEAPPLY =>
969+
val end = readEnd()
970+
val tycon = readParentType()
971+
if tycon.typeParams.isEmpty then
972+
goto(end)
973+
tycon
974+
else
975+
val args = until(end)(readTpt())
976+
val cls = tycon.classSymbol
977+
assert(cls.typeParams.hasSameLengthAs(args))
978+
cls.typeRef.appliedTo(args.tpes)
979+
case APPLY | BLOCK =>
980+
val end = readEnd()
981+
try readParentType()
982+
finally goto(end)
983+
case SELECTin =>
984+
val end = readEnd()
985+
readName()
986+
readTerm() match
987+
case nu: New =>
988+
try nu.tpe
989+
finally goto(end)
990+
case SHAREDterm =>
991+
forkAt(readAddr()).readParentType()
992+
993+
/** Read template parents
994+
* @param withArgs if false, only read enough of parent trees to determine their type
995+
* but skip constructor arguments. Return any trees that were partially
996+
* parsed in this way as InferredTypeTrees.
997+
*/
998+
def readParents(withArgs: Boolean)(using Context): List[Tree] =
999+
collectWhile(nextByte != SELFDEF && nextByte != DEFDEF) {
1000+
nextUnsharedTag match
1001+
case APPLY | TYPEAPPLY | BLOCK =>
1002+
if withArgs then readTerm()
1003+
else InferredTypeTree().withType(readParentType())
1004+
case _ => readTpt()
1005+
}
1006+
9621007
private def readTemplate(using Context): Template = {
9631008
val start = currentAddr
9641009
assert(sourcePathAt(start).isEmpty)
@@ -981,12 +1026,8 @@ class TreeUnpickler(reader: TastyReader,
9811026
while (bodyIndexer.reader.nextByte != DEFDEF) bodyIndexer.skipTree()
9821027
bodyIndexer.indexStats(end)
9831028
}
984-
val parents = collectWhile(nextByte != SELFDEF && nextByte != DEFDEF) {
985-
nextUnsharedTag match {
986-
case APPLY | TYPEAPPLY | BLOCK => readTerm()(using parentCtx)
987-
case _ => readTpt()(using parentCtx)
988-
}
989-
}
1029+
val parentReader = fork
1030+
val parents = readParents(withArgs = false)(using parentCtx)
9901031
val parentTypes = parents.map(_.tpe.dealias)
9911032
val self =
9921033
if (nextByte == SELFDEF) {
@@ -1000,7 +1041,13 @@ class TreeUnpickler(reader: TastyReader,
10001041
selfInfo = if (self.isEmpty) NoType else self.tpt.tpe)
10011042
.integrateOpaqueMembers
10021043
val constr = readIndexedDef().asInstanceOf[DefDef]
1003-
val mappedParents = parents.map(_.changeOwner(localDummy, constr.symbol))
1044+
val mappedParents: LazyTreeList =
1045+
if parents.exists(_.isInstanceOf[InferredTypeTree]) then
1046+
// parents were not read fully, will need to be read again later on demand
1047+
new LazyReader(parentReader, localDummy, ctx.mode, ctx.source,
1048+
_.readParents(withArgs = true)
1049+
.map(_.changeOwner(localDummy, constr.symbol)))
1050+
else parents
10041051

10051052
val lazyStats = readLater(end, rdr => {
10061053
val stats = rdr.readIndexedStats(localDummy, end)
@@ -1009,7 +1056,7 @@ class TreeUnpickler(reader: TastyReader,
10091056
defn.patchStdLibClass(cls)
10101057
NamerOps.addConstructorProxies(cls)
10111058
setSpan(start,
1012-
untpd.Template(constr, mappedParents, Nil, self, lazyStats)
1059+
untpd.Template(constr, mappedParents, self, lazyStats)
10131060
.withType(localDummy.termRef))
10141061
}
10151062

compiler/src/dotty/tools/dotc/interactive/Interactive.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,8 @@ object Interactive {
313313
case _ =>
314314
}
315315
localCtx
316-
case tree @ Template(constr, parents, self, _) =>
317-
if ((constr :: self :: parents).contains(nested)) outer
316+
case tree @ Template(constr, _, self, _) =>
317+
if ((constr :: self :: tree.parentsOrDerived).contains(nested)) outer
318318
else contextOfStat(tree.body, nested, tree.symbol, outer.inClassContext(self.symbol))
319319
case _ =>
320320
outer

compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -737,8 +737,7 @@ private class ExtractAPICollector(using Context) extends ThunkHolder {
737737
var h = initHash
738738

739739
p match
740-
case p: WithLazyField[?] =>
741-
p.forceIfLazy
740+
case p: WithLazyFields => p.forceFields()
742741
case _ =>
743742

744743
if inlineOrigin.exists then

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ abstract class MacroTransform extends Phase {
3838
tree
3939
case _: PackageDef | _: MemberDef =>
4040
super.transform(tree)(using localCtx(tree))
41-
case impl @ Template(constr, parents, self, _) =>
41+
case impl @ Template(constr, _, self, _) =>
4242
cpy.Template(tree)(
4343
transformSub(constr),
44-
transform(parents)(using ctx.superCallContext),
44+
transform(impl.parents)(using ctx.superCallContext),
4545
Nil,
4646
transformSelf(self),
4747
transformStats(impl.body, tree.symbol))

0 commit comments

Comments
 (0)