From 511e771b762253802c0ceb6153fc44a52dcead46 Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 12 Jan 2023 21:04:28 +0100 Subject: [PATCH 1/5] Read argument of parent constructors in Templates lazily 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 --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 2 +- .../tools/dotc/ast/TreeMapWithImplicits.scala | 4 +- .../dotty/tools/dotc/ast/TreeTypeMap.scala | 4 +- compiler/src/dotty/tools/dotc/ast/Trees.scala | 26 ++++++-- compiler/src/dotty/tools/dotc/ast/untpd.scala | 5 +- .../tools/dotc/core/tasty/TreeUnpickler.scala | 63 ++++++++++++++++--- .../tools/dotc/interactive/Interactive.scala | 4 +- .../tools/dotc/transform/MacroTransform.scala | 4 +- .../src/dotty/tools/dotc/typer/Checking.scala | 4 +- .../tools/dotc/parsing/DeSugarTest.scala | 4 +- tests/pos/i16673/FooStub_1.scala | 2 + tests/pos/i16673/FooStub_2.scala | 2 + tests/pos/i16673/Foo_1.scala | 1 + 13 files changed, 97 insertions(+), 28 deletions(-) create mode 100644 tests/pos/i16673/FooStub_1.scala create mode 100644 tests/pos/i16673/FooStub_2.scala create mode 100644 tests/pos/i16673/Foo_1.scala diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index ccc6e99737d4..c2cb65b90ec4 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -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) } diff --git a/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala b/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala index caf8d68442f6..e52bf1064e4c 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeMapWithImplicits.scala @@ -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)) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala b/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala index 71998aff9304..f5bf55802adf 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeTypeMap.scala @@ -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)))) diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index 1053e7c86780..0f56b02181b6 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -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 } @@ -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) @@ -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, _) => diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala index 8a6ba48d22c5..aeebb1f203e8 100644 --- a/compiler/src/dotty/tools/dotc/ast/untpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/untpd.scala @@ -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) } @@ -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) diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index 26a9d2c3aa51..91290b4ddd41 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -959,6 +959,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) @@ -981,12 +1026,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) { @@ -1000,7 +1041,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) @@ -1009,7 +1056,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)) } diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 6b2237a09b3f..fd6d426f39bb 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala b/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala index 27ccd622bc65..7bb7ed365ebe 100644 --- a/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala +++ b/compiler/src/dotty/tools/dotc/transform/MacroTransform.scala @@ -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)) diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 85ee76b8830e..d923b444a1e3 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -1394,9 +1394,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, _) diff --git a/compiler/test/dotty/tools/dotc/parsing/DeSugarTest.scala b/compiler/test/dotty/tools/dotc/parsing/DeSugarTest.scala index a54880326704..bb2797c5d034 100644 --- a/compiler/test/dotty/tools/dotc/parsing/DeSugarTest.scala +++ b/compiler/test/dotty/tools/dotc/parsing/DeSugarTest.scala @@ -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 => diff --git a/tests/pos/i16673/FooStub_1.scala b/tests/pos/i16673/FooStub_1.scala new file mode 100644 index 000000000000..ae784755f2ce --- /dev/null +++ b/tests/pos/i16673/FooStub_1.scala @@ -0,0 +1,2 @@ +abstract class FooStub(val that: Foo): + val bar = 1337; \ No newline at end of file diff --git a/tests/pos/i16673/FooStub_2.scala b/tests/pos/i16673/FooStub_2.scala new file mode 100644 index 000000000000..ae784755f2ce --- /dev/null +++ b/tests/pos/i16673/FooStub_2.scala @@ -0,0 +1,2 @@ +abstract class FooStub(val that: Foo): + val bar = 1337; \ No newline at end of file diff --git a/tests/pos/i16673/Foo_1.scala b/tests/pos/i16673/Foo_1.scala new file mode 100644 index 000000000000..1c47985780ef --- /dev/null +++ b/tests/pos/i16673/Foo_1.scala @@ -0,0 +1 @@ +final class Foo(_that: Foo) extends FooStub(_that) \ No newline at end of file From 0d35860ddf81d2d6c9cccd0e70d301366036f3db Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 14 Jan 2023 09:04:48 +0100 Subject: [PATCH 2/5] Clean up forcing logic for lazy tree fields --- .../dotty/tools/dotc/ast/NavigateAST.scala | 10 ++-- compiler/src/dotty/tools/dotc/ast/Trees.scala | 60 ++++++++----------- .../dotc/core/tasty/PositionPickler.scala | 4 +- .../src/dotty/tools/dotc/sbt/ExtractAPI.scala | 3 +- .../tools/dotc/transform/TreeChecker.scala | 6 +- 5 files changed, 35 insertions(+), 48 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/NavigateAST.scala b/compiler/src/dotty/tools/dotc/ast/NavigateAST.scala index 054ffe66f323..ace396d1e583 100644 --- a/compiler/src/dotty/tools/dotc/ast/NavigateAST.scala +++ b/compiler/src/dotty/tools/dotc/ast/NavigateAST.scala @@ -4,7 +4,7 @@ package ast import core.Contexts._ import core.Decorators._ import util.Spans._ -import Trees.{MemberDef, DefTree, WithLazyField} +import Trees.{MemberDef, DefTree, WithLazyFields} import dotty.tools.dotc.core.Types.AnnotatedType import dotty.tools.dotc.core.Types.ImportType import dotty.tools.dotc.core.Types.Type @@ -106,16 +106,14 @@ object NavigateAST { // FIXME: We shouldn't be manually forcing trees here, we should replace // our usage of `productIterator` by something in `Positioned` that takes // care of low-level details like this for us. - p match { - case p: WithLazyField[?] => - p.forceIfLazy + p match + case p: WithLazyFields => p.forceFields() case _ => - } val iterator = p match case defdef: DefTree[?] => p.productIterator ++ defdef.mods.productIterator case _ => - p.productIterator + p.productIterator childPath(iterator, p :: path) } else { diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index 0f56b02181b6..85b4e72be95d 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -407,12 +407,12 @@ object Trees { } /** A ValDef or DefDef tree */ - abstract class ValOrDefDef[+T <: Untyped](implicit @constructorOnly src: SourceFile) extends MemberDef[T] with WithLazyField[Tree[T]] { + abstract class ValOrDefDef[+T <: Untyped](implicit @constructorOnly src: SourceFile) extends MemberDef[T], WithLazyFields { type ThisTree[+T <: Untyped] <: ValOrDefDef[T] def name: TermName def tpt: Tree[T] - def unforcedRhs: LazyTree[T] = unforced - def rhs(using Context): Tree[T] = forceIfLazy + def unforcedRhs: LazyTree[T] + def rhs(using Context): Tree[T] } trait ValOrTypeDef[+T <: Untyped] extends MemberDef[T]: @@ -808,8 +808,10 @@ object Trees { extends ValOrDefDef[T], ValOrTypeDef[T] { type ThisTree[+T <: Untyped] = ValDef[T] assert(isEmpty || (tpt ne genericEmptyTree)) - def unforced: LazyTree[T] = preRhs - protected def force(x: Tree[T @uncheckedVariance]): Unit = preRhs = x + + def unforcedRhs: LazyTree[T] = preRhs + def forceFields()(using Context): Unit = preRhs = force(preRhs) + def rhs(using Context): Tree[T] = { forceFields(); preRhs.asInstanceOf[Tree[T]] } } /** mods def name[tparams](vparams_1)...(vparams_n): tpt = rhs */ @@ -818,8 +820,10 @@ object Trees { extends ValOrDefDef[T] { type ThisTree[+T <: Untyped] = DefDef[T] assert(tpt ne genericEmptyTree) - def unforced: LazyTree[T] = preRhs - protected def force(x: Tree[T @uncheckedVariance]): Unit = preRhs = x + + def unforcedRhs: LazyTree[T] = preRhs + def forceFields()(using Context): Unit = preRhs = force(preRhs) + def rhs(using Context): Tree[T] = { forceFields(); preRhs.asInstanceOf[Tree[T]] } def leadingTypeParams(using Context): List[TypeDef[T]] = paramss match case (tparams @ (tparam: TypeDef[_]) :: _) :: _ => tparams.asInstanceOf[List[TypeDef[T]]] @@ -855,27 +859,17 @@ object Trees { * if this is of class untpd.DerivingTemplate. * Typed templates only have parents. */ - 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]]] { + 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) + extends DefTree[T] with WithLazyFields { 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 forceFields()(using Context): Unit = + preParentsOrDerived = force(preParentsOrDerived) + preBody = force(preBody) - 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 unforcedBody: LazyTreeList[T] = preBody + def body(using Context): List[Tree[T]] = { forceFields(); preBody.asInstanceOf[List[Tree[T]]] } + def parentsOrDerived(using Context): List[Tree[T]] = { forceFields(); preParentsOrDerived.asInstanceOf[List[Tree[T]]] } def parents(using Context): List[Tree[T]] = parentsOrDerived // overridden by DerivingTemplate def derived: List[untpd.Tree] = Nil // overridden by DerivingTemplate @@ -1027,17 +1021,11 @@ object Trees { * accessed by `unforced` and `force`. Forcing the field will * set the `var` to the underlying value. */ - trait WithLazyField[+T <: AnyRef] { - def unforced: T | Lazy[T] - protected def force(x: T @uncheckedVariance): Unit - def forceIfLazy(using Context): T = unforced match { - case lzy: Lazy[T @unchecked] => - val x = lzy.complete - force(x) - x - case x: T @ unchecked => x - } - } + trait WithLazyFields: + protected def force[T <: AnyRef](x: T | Lazy[T])(using Context): T = x match + case x: Lazy[T] @unchecked => x.complete + case x: T @unchecked => x + def forceFields()(using Context): Unit /** A base trait for lazy tree fields. * These can be instantiated with Lazy instances which diff --git a/compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala index e8946c8b4037..906cea0453c9 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala @@ -8,7 +8,7 @@ import dotty.tools.tasty.TastyBuffer import TastyBuffer._ import ast._ -import Trees.WithLazyField +import Trees.WithLazyFields import util.{SourceFile, NoSource} import core._ import Annotations._, Decorators._ @@ -80,7 +80,7 @@ class PositionPickler( def alwaysNeedsPos(x: Positioned) = x match { case // initialSpan is inaccurate for trees with lazy field - _: WithLazyField[?] + _: WithLazyFields // A symbol is created before the corresponding tree is unpickled, // and its position cannot be changed afterwards. diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala index e561b26abf6d..f54baeb7256c 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala @@ -737,8 +737,7 @@ private class ExtractAPICollector(using Context) extends ThunkHolder { var h = initHash p match - case p: WithLazyField[?] => - p.forceIfLazy + case p: WithLazyFields => p.forceFields() case _ => if inlineOrigin.exists then diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 4041f620db56..7b42a1957a6c 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -303,9 +303,11 @@ object TreeChecker { // case tree: untpd.Select => // case tree: untpd.Bind => case vd : ValDef => - assertIdentNotJavaClass(vd.forceIfLazy) + vd.forceFields() + assertIdentNotJavaClass(vd) case dd : DefDef => - assertIdentNotJavaClass(dd.forceIfLazy) + dd.forceFields() + assertIdentNotJavaClass(dd) // case tree: untpd.TypeDef => case Apply(fun, args) => assertIdentNotJavaClass(fun) From ae841ebb6d87c6a0211b53dcc01dead130975c26 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 14 Jan 2023 09:22:25 +0100 Subject: [PATCH 3/5] Dedup refactoring --- .../src/dotty/tools/dotc/transform/TreeChecker.scala | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 7b42a1957a6c..26f6c3ca6878 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -302,12 +302,9 @@ object TreeChecker { // case tree: untpd.Ident => // case tree: untpd.Select => // case tree: untpd.Bind => - case vd : ValDef => - vd.forceFields() - assertIdentNotJavaClass(vd) - case dd : DefDef => - dd.forceFields() - assertIdentNotJavaClass(dd) + case md: ValOrDefDef => + md.forceFields() + assertIdentNotJavaClass(md) // case tree: untpd.TypeDef => case Apply(fun, args) => assertIdentNotJavaClass(fun) From 3d91233d23d271389e759ac65a932c3fa4a399d4 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 16 Jan 2023 10:55:01 +0100 Subject: [PATCH 4/5] Update docs of `WithLazyFields` --- compiler/src/dotty/tools/dotc/ast/Trees.scala | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index 85b4e72be95d..c0b5987c3875 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -1016,24 +1016,27 @@ object Trees { // ----- Lazy trees and tree sequences - /** A tree that can have a lazy field - * The field is represented by some private `var` which is - * accessed by `unforced` and `force`. Forcing the field will - * set the `var` to the underlying value. + /** A base trait for lazy tree fields. + * These can be instantiated with Lazy instances which + * can delay tree construction until the field is first demanded. + */ + trait Lazy[+T <: AnyRef]: + def complete(using Context): T + + /** A tree that can have a lazy fields. + * Such fields are variables of type `T | Lazy[T]`, for some tyope `T`. */ trait WithLazyFields: + + /** If `x` is lazy, computes the underlying value */ protected def force[T <: AnyRef](x: T | Lazy[T])(using Context): T = x match case x: Lazy[T] @unchecked => x.complete case x: T @unchecked => x + + /** Assigns all lazy fields their underlying non-lazy value. */ def forceFields()(using Context): Unit - /** A base trait for lazy tree fields. - * These can be instantiated with Lazy instances which - * can delay tree construction until the field is first demanded. - */ - trait Lazy[+T <: AnyRef] { - def complete(using Context): T - } + end WithLazyFields // ----- Generic Tree Instances, inherited from `tpt` and `untpd`. From 67d64005c2506e94fb989164abb5d78898d3351f Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 16 Jan 2023 11:54:32 +0100 Subject: [PATCH 5/5] Fix rebase breakage --- .../tools/dotc/transform/TreeChecker.scala | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 26f6c3ca6878..4573c40df78b 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -185,17 +185,17 @@ 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 private[TreeChecker] def isValidJVMName(name: Name): Boolean = name.toString.forall(isValidJVMChar)