From c178b8c5b06b688a3b724baa2b2a5f04d119cd69 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 17 Oct 2018 13:48:06 +0200 Subject: [PATCH 1/6] Keep package structure in `rootTreeOrProvider` `rootTreeOrProvider` is a property of `ClassSymbol`, and contain the tree that defines this symbol (or a mean to get that tree). When the tree came from unpickled TASTY files, we were able to see the nested packages from which the tree comes, along with the imports. However, when the symbol has been loaded from source, we were filling the `rootTreeOrProvider` only with the `TypeDef` that introduced this symbol. This commit moves the assignment of `rootTreeOrProvider` out of the typer and into its own phase. Before assigning, we recreate the package structure so that the information that we get out of `rootTreeOrProvider` is the same regardless of whether the symbol has been loaded from source or unpickled from TASTY. --- compiler/src/dotty/tools/dotc/Compiler.scala | 1 + compiler/src/dotty/tools/dotc/Run.scala | 11 +++-- .../interactive/InteractiveCompiler.scala | 1 + .../tools/dotc/transform/SetRootTree.scala | 45 +++++++++++++++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 2 - 5 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 compiler/src/dotty/tools/dotc/transform/SetRootTree.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index d96f25ea9b67..5beecc5a6b71 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -52,6 +52,7 @@ class Compiler { List(new FirstTransform, // Some transformations to put trees into a canonical form new CheckReentrant, // Internal use only: Check that compiled program has no data races involving global vars new ElimPackagePrefixes, // Eliminate references to package prefixes in Select nodes + new SetRootTree, // Set the `rootTreeOrProvider` on class symbols new CookComments) :: // Cook the comments: expand variables, doc, etc. List(new CheckStatic, // Check restrictions that apply to @static members new ElimRepeated, // Rewrite vararg parameters and arguments diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index f7e88c124b57..82b420273497 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -200,7 +200,8 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint /** Enter top-level definitions of classes and objects contain in Scala source file `file`. * The newly added symbols replace any previously entered symbols. - * If `typeCheck = true`, also run typer on the compilation unit. + * If `typeCheck = true`, also run typer on the compilation unit, and set + * `rootTreeOrProvider`. */ def lateCompile(file: AbstractFile, typeCheck: Boolean)(implicit ctx: Context): Unit = if (!files.contains(file) && !lateFiles.contains(file)) { @@ -211,9 +212,13 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint if (unit.isJava) new JavaParser(unit.source).parse() else new Parser(unit.source).parse() ctx.typer.lateEnter(unit.untpdTree) - def typeCheckUnit() = unit.tpdTree = ctx.typer.typedExpr(unit.untpdTree) + def processUnit() = { + unit.tpdTree = ctx.typer.typedExpr(unit.untpdTree) + val phase = new transform.SetRootTree() + phase.runOn(unit) + } if (typeCheck) - if (compiling) finalizeActions += (() => typeCheckUnit()) else typeCheckUnit() + if (compiling) finalizeActions += (() => processUnit()) else processUnit() } process()(runContext.fresh.setCompilationUnit(unit)) } diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala index 2fdc2fc657b5..b3ef40dec8a3 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveCompiler.scala @@ -13,6 +13,7 @@ class InteractiveCompiler extends Compiler { // after each phase group instead of waiting for the pipeline to finish. override def phases: List[List[Phase]] = List( List(new FrontEnd), + List(new transform.SetRootTree), List(new transform.CookComments) ) } diff --git a/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala b/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala new file mode 100644 index 000000000000..714df1f2e205 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala @@ -0,0 +1,45 @@ +package dotty.tools.dotc.transform + +import dotty.tools.dotc.CompilationUnit +import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.transform.MegaPhase.MiniPhase + +/** Set the `rootTreeOrProvider` property of class symbols. */ +class SetRootTree extends MiniPhase { + + override val phaseName: String = SetRootTree.name + override def isRunnable(implicit ctx: Context) = + super.isRunnable && ctx.settings.YretainTrees.value + + def runOn(unit: CompilationUnit)(implicit ctx: Context) = { + val runCtx = ctx.fresh.setCompilationUnit(unit) + traverser.traverse(unit.tpdTree)(runCtx) + } + + override def transformTypeDef(tree: tpd.TypeDef)(implicit ctx: Context): tpd.Tree = { + if (tree.symbol.isClass) { + val sym = tree.symbol.asClass + tpd.sliceTopLevel(ctx.compilationUnit.tpdTree, sym) match { + case (pkg: tpd.PackageDef) :: Nil => + sym.rootTreeOrProvider = pkg + case _ => + sym.rootTreeOrProvider = tree + } + } + tree + } + + private def traverser = new tpd.TreeTraverser { + override def traverse(tree: tpd.Tree)(implicit ctx: Context): Unit = tree match { + case typeDef: tpd.TypeDef => transformTypeDef(typeDef) + case other => traverseChildren(other) + } + } + + +} + +object SetRootTree { + val name: String = "SetRootTree" +} diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index e11447c2a461..7424778a0a10 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1642,8 +1642,6 @@ class Typer extends Namer // check value class constraints checkDerivedValueClass(cls, body1) - if (ctx.settings.YretainTrees.value) cls.rootTreeOrProvider = cdef1 - cdef1 // todo later: check that From 619b4c587df32a7696d139287d1ce88191573f8d Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 19 Oct 2018 10:31:30 +0200 Subject: [PATCH 2/6] Move `SetRootTree` right before `Pickler` --- compiler/src/dotty/tools/dotc/Compiler.scala | 2 +- compiler/src/dotty/tools/dotc/Run.scala | 2 +- .../tools/dotc/transform/SetRootTree.scala | 39 ++++++++----------- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 5beecc5a6b71..5bc50b4b5d10 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -39,6 +39,7 @@ class Compiler { List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks List(new PostTyper) :: // Additional checks and cleanups after type checking List(new sbt.ExtractAPI) :: // Sends a representation of the API of classes to sbt via callbacks + List(new SetRootTree) :: // Set the `rootTreeOrProvider` on class symbols Nil /** Phases dealing with TASTY tree pickling and unpickling */ @@ -52,7 +53,6 @@ class Compiler { List(new FirstTransform, // Some transformations to put trees into a canonical form new CheckReentrant, // Internal use only: Check that compiled program has no data races involving global vars new ElimPackagePrefixes, // Eliminate references to package prefixes in Select nodes - new SetRootTree, // Set the `rootTreeOrProvider` on class symbols new CookComments) :: // Cook the comments: expand variables, doc, etc. List(new CheckStatic, // Check restrictions that apply to @static members new ElimRepeated, // Rewrite vararg parameters and arguments diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index 82b420273497..e295e68548c5 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -215,7 +215,7 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint def processUnit() = { unit.tpdTree = ctx.typer.typedExpr(unit.untpdTree) val phase = new transform.SetRootTree() - phase.runOn(unit) + phase.run } if (typeCheck) if (compiling) finalizeActions += (() => processUnit()) else processUnit() diff --git a/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala b/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala index 714df1f2e205..4e1e212cfa38 100644 --- a/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala +++ b/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala @@ -3,41 +3,34 @@ package dotty.tools.dotc.transform import dotty.tools.dotc.CompilationUnit import dotty.tools.dotc.ast.tpd import dotty.tools.dotc.core.Contexts.Context -import dotty.tools.dotc.transform.MegaPhase.MiniPhase +import dotty.tools.dotc.core.Phases.Phase /** Set the `rootTreeOrProvider` property of class symbols. */ -class SetRootTree extends MiniPhase { +class SetRootTree extends Phase { override val phaseName: String = SetRootTree.name override def isRunnable(implicit ctx: Context) = super.isRunnable && ctx.settings.YretainTrees.value - def runOn(unit: CompilationUnit)(implicit ctx: Context) = { - val runCtx = ctx.fresh.setCompilationUnit(unit) - traverser.traverse(unit.tpdTree)(runCtx) - } - - override def transformTypeDef(tree: tpd.TypeDef)(implicit ctx: Context): tpd.Tree = { - if (tree.symbol.isClass) { - val sym = tree.symbol.asClass - tpd.sliceTopLevel(ctx.compilationUnit.tpdTree, sym) match { - case (pkg: tpd.PackageDef) :: Nil => - sym.rootTreeOrProvider = pkg - case _ => - sym.rootTreeOrProvider = tree - } - } - tree + override def run(implicit ctx: Context): Unit = { + val tree = ctx.compilationUnit.tpdTree + traverser.traverse(tree) } private def traverser = new tpd.TreeTraverser { - override def traverse(tree: tpd.Tree)(implicit ctx: Context): Unit = tree match { - case typeDef: tpd.TypeDef => transformTypeDef(typeDef) - case other => traverseChildren(other) + override def traverse(tree: tpd.Tree)(implicit ctx: Context): Unit = { + if (tree.isInstanceOf[tpd.TypeDef] && tree.symbol.isClass) { + val sym = tree.symbol.asClass + tpd.sliceTopLevel(ctx.compilationUnit.tpdTree, sym) match { + case (pkg: tpd.PackageDef) :: Nil => + sym.rootTreeOrProvider = pkg + case _ => + sym.rootTreeOrProvider = tree + } + } + traverseChildren(tree) } } - - } object SetRootTree { From b7420530653fc345a657014d393ddd2ce6975953 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 29 Oct 2018 10:59:20 +0100 Subject: [PATCH 3/6] Traverse only non-local definitions in SetRootTree --- .../tools/dotc/transform/SetRootTree.scala | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala b/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala index 4e1e212cfa38..bf8eee8d6cea 100644 --- a/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala +++ b/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala @@ -19,16 +19,25 @@ class SetRootTree extends Phase { private def traverser = new tpd.TreeTraverser { override def traverse(tree: tpd.Tree)(implicit ctx: Context): Unit = { - if (tree.isInstanceOf[tpd.TypeDef] && tree.symbol.isClass) { - val sym = tree.symbol.asClass - tpd.sliceTopLevel(ctx.compilationUnit.tpdTree, sym) match { - case (pkg: tpd.PackageDef) :: Nil => - sym.rootTreeOrProvider = pkg - case _ => - sym.rootTreeOrProvider = tree - } + tree match { + case pkg: tpd.PackageDef => + traverseChildren(pkg) + case td: tpd.TypeDef => + if (td.symbol.isClass) { + val sym = td.symbol.asClass + tpd.sliceTopLevel(ctx.compilationUnit.tpdTree, sym) match { + case (pkg: tpd.PackageDef) :: Nil => + sym.rootTreeOrProvider = pkg + case _ => + sym.rootTreeOrProvider = td + } + } + traverseChildren(td) + case tpl: tpd.Template => + traverseChildren(tpl) + case _ => + () } - traverseChildren(tree) } } } From 45e4758cc8f4bfd1c33d40d3c0c53615b806d854 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 19 Nov 2018 07:05:13 +0100 Subject: [PATCH 4/6] Test for `rootTreeOrProvider` with late compile --- .../dotty/tools/languageserver/RenameTest.scala | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/language-server/test/dotty/tools/languageserver/RenameTest.scala b/language-server/test/dotty/tools/languageserver/RenameTest.scala index be1dc06fec2c..63c870de0f03 100644 --- a/language-server/test/dotty/tools/languageserver/RenameTest.scala +++ b/language-server/test/dotty/tools/languageserver/RenameTest.scala @@ -236,6 +236,19 @@ class RenameTest { } + @Test def renameImportFromTasty: Unit = { + // Note that everything here is in the empty package; this ensures that we will + // use the sourcefile loader to load `class Bar`. + def testRename(m: CodeMarker) = { + withSources( + code"""object O { class ${m1}Foo${m2} }""", + tasty"""import O.${m3}Foo${m4} + class Bar extends ${m5}Foo${m6}""" + ).rename(m, "NewName", Set(m1 to m2, m3 to m4, m5 to m6)) + } + testRename(m1) + testRename(m2) + } } From aa10ba78dc11077d651118cc2e5e80aa3cb93eda Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 28 Nov 2018 10:33:02 +0100 Subject: [PATCH 5/6] Shallower traversal in `SetRootTree` We only need to traverse the tree down to the top level classes, but no further. --- compiler/src/dotty/tools/dotc/transform/SetRootTree.scala | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala b/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala index bf8eee8d6cea..09098188de1e 100644 --- a/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala +++ b/compiler/src/dotty/tools/dotc/transform/SetRootTree.scala @@ -32,9 +32,6 @@ class SetRootTree extends Phase { sym.rootTreeOrProvider = td } } - traverseChildren(td) - case tpl: tpd.Template => - traverseChildren(tpl) case _ => () } From 136b0ceb4ff6419372cc94a1fb82a43c75048cc7 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 28 Nov 2018 13:53:20 +0100 Subject: [PATCH 6/6] Set incomplete `rootTreeOrProvider` in typer During typer, set `rootTreeOrProvider` to be the typed class def. This information will be overwritten in `SetRootTree` to include the whole package structure, but we may never reach this phase. --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 7424778a0a10..a268497fae03 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1642,6 +1642,12 @@ class Typer extends Namer // check value class constraints checkDerivedValueClass(cls, body1) + + // Temporarily set the typed class def as root tree so that we have at least some + // information in the IDE in case we never reach `SetRootTree`. + if (ctx.mode.is(Mode.Interactive) && ctx.settings.YretainTrees.value) + cls.rootTreeOrProvider = cdef1 + cdef1 // todo later: check that