Skip to content

Give position to symbols when unpickling #3525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Dec 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 43 additions & 11 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,23 @@ import Types._
import Scopes._
import typer.{FrontEnd, Typer, ImportInfo, RefChecks}
import Decorators._
import io.PlainFile
import io.{AbstractFile, PlainFile}
import scala.io.Codec
import util._
import util.{Set => _, _}
import reporting.Reporter
import transform.TreeChecker
import rewrite.Rewrites
import java.io.{BufferedWriter, OutputStreamWriter}
import printing.XprintMode
import typer.ImplicitRunInfo

import scala.annotation.tailrec
import dotty.tools.io.VirtualFile
import scala.util.control.NonFatal

/** A compiler run. Exports various methods to compile source files */
class Run(comp: Compiler, ictx: Context) {

var units: List[CompilationUnit] = _
import Run._

/** Produces the following contexts, from outermost to innermost
*
Expand Down Expand Up @@ -78,7 +78,7 @@ class Run(comp: Compiler, ictx: Context) {
compileSources(sources)
} catch {
case NonFatal(ex) =>
ctx.echo(i"exception occurred while compiling $units%, %")
ctx.echo(i"exception occurred while compiling ${ctx.runInfo.units}%, %")
throw ex
}

Expand All @@ -90,17 +90,17 @@ class Run(comp: Compiler, ictx: Context) {
*/
def compileSources(sources: List[SourceFile]) =
if (sources forall (_.exists)) {
units = sources map (new CompilationUnit(_))
ctx.runInfo.units = sources map (new CompilationUnit(_))
compileUnits()
}

def compileUnits(us: List[CompilationUnit]): Unit = {
units = us
ctx.runInfo.units = us
compileUnits()
}

def compileUnits(us: List[CompilationUnit], ctx: Context): Unit = {
units = us
ctx.runInfo.units = us
compileUnits()(ctx)
}

Expand All @@ -122,16 +122,16 @@ class Run(comp: Compiler, ictx: Context) {
if (phase.isRunnable)
Stats.trackTime(s"$phase ms ") {
val start = System.currentTimeMillis
units = phase.runOn(units)
ctx.runInfo.units = phase.runOn(ctx.runInfo.units)
if (ctx.settings.Xprint.value.containsPhase(phase)) {
for (unit <- units) {
for (unit <- ctx.runInfo.units) {
lastPrintedTree =
printTree(lastPrintedTree)(ctx.fresh.setPhase(phase.next).setCompilationUnit(unit))
}
}
ctx.informTime(s"$phase ", start)
Stats.record(s"total trees at end of $phase", ast.Trees.ntrees)
for (unit <- units)
for (unit <- ctx.runInfo.units)
Stats.record(s"retained typed trees at end of $phase", unit.tpdTree.treeSize)
}
}
Expand Down Expand Up @@ -191,3 +191,35 @@ class Run(comp: Compiler, ictx: Context) {
r
}
}

object Run {
/** Info that changes on each compiler run */
class RunInfo(initctx: Context) extends ImplicitRunInfo with ConstraintRunInfo {
implicit val ctx: Context = initctx

private[this] var myUnits: List[CompilationUnit] = _
private[this] var myUnitsCached: List[CompilationUnit] = _
private[this] var myFiles: Set[AbstractFile] = _

/** The compilation units currently being compiled, this may return different
* results over time.
*/
def units: List[CompilationUnit] = myUnits

private[Run] def units_=(us: List[CompilationUnit]): Unit =
myUnits = us


/** The files currently being compiled, this may return different results over time.
* These files do not have to be source files since it's possible to compile
* from TASTY.
*/
def files: Set[AbstractFile] = {
if (myUnits ne myUnitsCached) {
myUnitsCached = myUnits
myFiles = myUnits.map(_.source.file).toSet
}
myFiles
}
}
}
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
case tp: MethodType =>
def valueParam(name: TermName, info: Type): TermSymbol = {
val maybeImplicit = if (tp.isImplicitMethod) Implicit else EmptyFlags
ctx.newSymbol(sym, name, TermParam | maybeImplicit, info)
ctx.newSymbol(sym, name, TermParam | maybeImplicit, info, coord = sym.coord)
}
val params = (tp.paramNames, tp.paramInfos).zipped.map(valueParam)
val (paramss, rtp) = valueParamss(tp.instantiate(params map (_.termRef)))
Expand Down Expand Up @@ -262,7 +262,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
val parents1 =
if (parents.head.classSymbol.is(Trait)) parents.head.parents.head :: parents
else parents
val cls = ctx.newNormalizedClassSymbol(owner, tpnme.ANON_FUN, Synthetic, parents1,
val cls = ctx.newNormalizedClassSymbol(owner, tpnme.ANON_CLASS, Synthetic, parents1,
coord = fns.map(_.pos).reduceLeft(_ union _))
val constr = ctx.newConstructor(cls, Synthetic, Nil, Nil).entered
def forwarder(fn: TermSymbol, name: TermName) = {
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class ScalaSettings extends Settings.SettingGroup {
val YdetailedStats = BooleanSetting("-Ydetailed-stats", "show detailed internal compiler stats (needs Stats.enabled to be set to true).")
val Yheartbeat = BooleanSetting("-Ydetailed-stats", "show heartbeat stack trace of compiler operations (needs Stats.enabled to be set to true).")
val YprintPos = BooleanSetting("-Yprint-pos", "show tree positions.")
val YprintPosSyms = BooleanSetting("-Yprint-pos-syms", "show symbol definitions positions.")
val YnoDeepSubtypes = BooleanSetting("-Yno-deep-subtypes", "throw an exception on deep subtyping call stacks.")
val YnoPatmatOpt = BooleanSetting("-Yno-patmat-opt", "disable all pattern matching optimizations.")
val YplainPrinter = BooleanSetting("-Yplain-printer", "Pretty-print using a plain printer.")
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/ConstraintRunInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dotty.tools.dotc
package core

import Contexts._
import Run.RunInfo
import config.Printers.typr

trait ConstraintRunInfo { self: RunInfo =>
Expand Down
8 changes: 2 additions & 6 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import NameOps._
import Uniques._
import SymDenotations._
import Comments._
import Run.RunInfo
import util.Positions._
import ast.Trees._
import ast.untpd
import util.{FreshNameCreator, SimpleIdentityMap, SourceFile, NoSource}
import typer.{Implicits, ImplicitRunInfo, ImportInfo, Inliner, NamerContextOps, SearchHistory, TypeAssigner, Typer}
import typer.{Implicits, ImportInfo, Inliner, NamerContextOps, SearchHistory, TypeAssigner, Typer}
import Implicits.ContextualImplicits
import config.Settings._
import config.Config
Expand Down Expand Up @@ -675,11 +676,6 @@ object Contexts {
// @sharable val theBase = new ContextBase // !!! DEBUG, so that we can use a minimal context for reporting even in code that normally cannot access a context
}

/** Info that changes on each compiler run */
class RunInfo(initctx: Context) extends ImplicitRunInfo with ConstraintRunInfo {
implicit val ctx: Context = initctx
}

class GADTMap(initBounds: SimpleIdentityMap[Symbol, TypeBounds]) extends util.DotClass {
private[this] var myBounds = initBounds
def setBounds(sym: Symbol, b: TypeBounds): Unit =
Expand Down
24 changes: 15 additions & 9 deletions compiler/src/dotty/tools/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ trait Symbols { this: Context =>
newClassSymbol(owner, name, flags, completer, privateWithin, coord, assocFile)
}

def newRefinedClassSymbol = newCompleteClassSymbol(
ctx.owner, tpnme.REFINE_CLASS, NonMember, parents = Nil)
def newRefinedClassSymbol(coord: Coord = NoCoord) =
newCompleteClassSymbol(ctx.owner, tpnme.REFINE_CLASS, NonMember, parents = Nil, coord = coord)

/** Create a module symbol with associated module class
* from its non-info fields and a function producing the info
Expand Down Expand Up @@ -288,7 +288,7 @@ trait Symbols { this: Context =>
val tparamBuf = new mutable.ListBuffer[TypeSymbol]
val trefBuf = new mutable.ListBuffer[TypeRef]
for (name <- names) {
val tparam = newNakedSymbol[TypeName](NoCoord)
val tparam = newNakedSymbol[TypeName](owner.coord)
tparamBuf += tparam
trefBuf += TypeRef(owner.thisType, tparam)
}
Expand Down Expand Up @@ -443,9 +443,11 @@ object Symbols {
if (lastDenot == null) NoRunId else lastDenot.validFor.runId

/** Does this symbol come from a currently compiled source file? */
final def isDefinedInCurrentRun(implicit ctx: Context): Boolean = {
pos.exists && defRunId == ctx.runId
}
final def isDefinedInCurrentRun(implicit ctx: Context): Boolean =
pos.exists && defRunId == ctx.runId && {
val file = associatedFile
file != null && ctx.runInfo.files.contains(file)
}

/** Is symbol valid in current run? */
final def isValidInCurrentRun(implicit ctx: Context): Boolean =
Expand Down Expand Up @@ -539,7 +541,7 @@ object Symbols {
* Overridden in ClassSymbol
*/
def associatedFile(implicit ctx: Context): AbstractFile =
denot.topLevelClass.symbol.associatedFile
if (lastDenot == null) null else lastDenot.topLevelClass.symbol.associatedFile

/** The class file from which this class was generated, null if not applicable. */
final def binaryFile(implicit ctx: Context): AbstractFile = {
Expand All @@ -563,8 +565,12 @@ object Symbols {
}
}

/** The position of this symbol, or NoPosition is symbol was not loaded
* from source.
/** The position of this symbol, or NoPosition if the symbol was not loaded
* from source or from TASTY. This is always a zero-extent position.
*
* NOTE: If the symbol was not loaded from the current compilation unit,
* the implicit conversion `sourcePos` will return the wrong result, careful!
* TODO: Consider changing this method return type to `SourcePosition`.
*/
def pos: Position = if (coord.isPosition) coord.toPosition else NoPosition

Expand Down
16 changes: 13 additions & 3 deletions compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,21 @@ class PositionPickler(pickler: TastyPickler, addrOfTree: tpd.Tree => Option[Addr
lastPos = pos
}

/** True if x's position cannot be reconstructed automatically from its initialPos
/** True if x's position shouldn't be reconstructed automatically from its initialPos
*/
def alwaysNeedsPos(x: Positioned) = x match {
case _: WithLazyField[_] // initialPos is inaccurate for trees with lazy field
| _: Trees.PackageDef[_] => true // package defs might be split into several Tasty files
case
// initialPos is inaccurate for trees with lazy field
_: WithLazyField[_]

// A symbol is created before the corresponding tree is unpickled,
// and its position cannot be changed afterwards.
// so we cannot use the tree initialPos to set the symbol position.
// Instead, we always pickle the position of definitions.
| _: Trees.DefTree[_]

// package defs might be split into several Tasty files
| _: Trees.PackageDef[_] => true
case _ => false
}

Expand Down
47 changes: 31 additions & 16 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ class TreeUnpickler(reader: TastyReader, nameAtRef: NameRef => TermName, posUnpi
case TYPEARGtype =>
TypeArgRef(readType(), readType().asInstanceOf[TypeRef], readNat())
case BIND =>
val sym = ctx.newSymbol(ctx.owner, readName().toTypeName, BindDefinedType, readType())
val sym = ctx.newSymbol(ctx.owner, readName().toTypeName, BindDefinedType, readType(),
coord = coordAt(start))
registerSym(start, sym)
if (currentAddr != end) readType()
TypeRef(NoPrefix, sym)
Expand Down Expand Up @@ -464,11 +465,14 @@ class TreeUnpickler(reader: TastyReader, nameAtRef: NameRef => TermName, posUnpi
rootd.symbol
case _ =>
val completer = adjustIfModule(new Completer(ctx.owner, subReader(start, end)))

val coord = coordAt(start)

if (isClass)
ctx.newClassSymbol(ctx.owner, name.asTypeName, flags, completer, privateWithin, coord = start.index)
ctx.newClassSymbol(ctx.owner, name.asTypeName, flags, completer, privateWithin, coord)
else
ctx.newSymbol(ctx.owner, name, flags, completer, privateWithin, coord = start.index)
} // TODO set position somehow (but take care not to upset Symbol#isDefinedInCurrentRun)
ctx.newSymbol(ctx.owner, name, flags, completer, privateWithin, coord)
}
sym.annotations = annots
ctx.enter(sym)
registerSym(start, sym)
Expand Down Expand Up @@ -995,7 +999,7 @@ class TreeUnpickler(reader: TastyReader, nameAtRef: NameRef => TermName, posUnpi
case BIND =>
val name = readName()
val info = readType()
val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, info)
val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, info, coord = coordAt(start))
registerSym(start, sym)
Bind(sym, readTerm())
case ALTERNATIVE =>
Expand All @@ -1011,7 +1015,7 @@ class TreeUnpickler(reader: TastyReader, nameAtRef: NameRef => TermName, posUnpi
val argPats = until(end)(readTerm())
UnApply(fn, implicitArgs, argPats, patType)
case REFINEDtpt =>
val refineCls = ctx.newRefinedClassSymbol
val refineCls = ctx.newRefinedClassSymbol(coordAt(start))
typeAtAddr(start) = refineCls.typeRef
val parent = readTpt()
val refinements = readStats(refineCls, end)(localContext(refineCls))
Expand Down Expand Up @@ -1087,21 +1091,32 @@ class TreeUnpickler(reader: TastyReader, nameAtRef: NameRef => TermName, posUnpi

// ------ Setting positions ------------------------------------------------

/** Set position of `tree` at given `addr`. */
def setPos[T <: untpd.Tree](addr: Addr, tree: T)(implicit ctx: Context): tree.type =
/** Pickled position for `addr`. */
def posAt(addr: Addr)(implicit ctx: Context): Position =
if (ctx.mode.is(Mode.ReadPositions)) {
posUnpicklerOpt match {
case Some(posUnpickler) =>
//println(i"setPos $tree / ${tree.getClass} at $addr to ${posUnpickler.posAt(addr)}")
val pos = posUnpickler.posAt(addr)
if (pos.exists) tree.setPosUnchecked(pos)
tree
posUnpickler.posAt(addr)
case _ =>
//println(i"no pos $tree")
tree
NoPosition
}
}
else tree
} else NoPosition

/** Coordinate for the symbol at `addr`. */
def coordAt(addr: Addr)(implicit ctx: Context): Coord = {
val pos = posAt(addr)
if (pos.exists)
positionCoord(pos)
else
indexCoord(addr.index)
}

/** Set position of `tree` at given `addr`. */
def setPos[T <: untpd.Tree](addr: Addr, tree: T)(implicit ctx: Context): tree.type = {
val pos = posAt(addr)
if (pos.exists) tree.setPosUnchecked(pos)
tree
}
}

class LazyReader[T <: AnyRef](reader: TreeReader, op: TreeReader => Context => T) extends Trees.Lazy[T] {
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/fromtasty/TASTYRun.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import core.Contexts._

class TASTYRun(comp: Compiler, ictx: Context) extends Run(comp, ictx) {
override def compile(classNames: List[String]) = {
units = classNames.map(new TASTYCompilationUnit(_))
compileUnits()
val units = classNames.map(new TASTYCompilationUnit(_))
compileUnits(units)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class InteractiveDriver(settings: List[String]) extends Driver {

run.compileSources(List(source))
run.printSummary()
val t = run.units.head.tpdTree
val t = ctx.runInfo.units.head.tpdTree
cleanup(t)
myOpenedTrees(uri) = topLevelClassTrees(t, source)

Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,8 @@ object Parsers {
t
}

def ascription(t: Tree, location: Location.Value) = atPos(startOffset(t), in.skipToken()) {
def ascription(t: Tree, location: Location.Value) = atPos(startOffset(t)) {
in.skipToken()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change? The previous implementation would place the ^ under the : of an ascription, which seems to be what we want, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all other cases, the point is under the first letter of the name of the definition (e.g. for def foo: Int = 1 the point is under f). This change means that I can always use the point of the definition tree to set the symbol position.

in.token match {
case USCORE =>
val uscoreStart = in.skipToken()
Expand Down
Loading