Fix position pickler

 - align conditions for generating sourdce changes with new scheme for
   assigning source files to trees
 - Also: avoid context capture in Thicket

Author: odersky

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

@@ -33,7 +33,7 @@ abstract class Positioned(implicit @transientParam src: SourceInfo) extends Prod
   def sourcePos(implicit ctx: Context): SourcePosition = source.atPos(pos)
 
   setId(TreeIds.nextIdFor(initialFile(src)))
-  setPos(initialPos, srcfile)
+  setPos(initialSpan(), srcfile)
 
   protected def setId(id: Int): Unit = {
     myUniqueId = id
@@ -156,7 +156,7 @@ abstract class Positioned(implicit @transientParam src: SourceInfo) extends Prod
     newpd
   }
 
-  def initialFile(src: SourceInfo): AbstractFile = {
+  def elemsFile: AbstractFile = {
     def firstFile(x: Any): AbstractFile = x match {
       case x: Positioned if x.pos.exists =>
         x.srcfile
@@ -167,38 +167,53 @@ abstract class Positioned(implicit @transientParam src: SourceInfo) extends Prod
         null
     }
     def firstElemFile(n: Int): AbstractFile =
-      if (n == productArity) src.source.file
+      if (n == productArity) null
       else {
         val f = firstFile(productElement(n))
         if (f != null) f else firstElemFile(n + 1)
       }
     firstElemFile(0)
   }
 
-  /** The initial, synthetic position. This is usually the union of all positioned children's positions.
+  private def initialFile(src: SourceInfo): AbstractFile = {
+    val f = elemsFile
+    if (f != null) f else src.srcfile
+  }
+
+  /** The initial, synthetic span. This is usually the union of all positioned children's positions.
+   *  @param  ignoreTypeTrees  If true, TypeTree children are not counted for the span.
+   *                           This is important for predicting whether a position entry is
+   *                           needed for pickling, since TypeTrees are pickled as types, so
+   *                           their position is lost.
    */
-  def initialPos: Position = {
+  def initialSpan(ignoreTypeTrees: Boolean = false): Position = {
+
+    def include(p1: Position, p2: Positioned) = p2 match {
+      case _: Trees.TypeTree[_] if ignoreTypeTrees => p1
+      case _ => p1.union(p2.pos)
+    }
+
+    def includeAll(pos: Position, xs: List[_]): Position = xs match {
+      case (p: Positioned) :: xs1 if sameSource(p) => includeAll(include(pos, p), xs1)
+      case (xs0: List[_]) :: xs1 => includeAll(includeAll(pos, xs0), xs1)
+      case _ :: xs1 => includeAll(pos, xs1)
+      case _ => pos
+    }
+
     var n = productArity
     var pos = NoPosition
     while (n > 0) {
       n -= 1
       productElement(n) match {
-        case p: Positioned if sameSource(p) => pos = pos union p.pos
-        case m: untpd.Modifiers => pos = unionPos(unionPos(pos, m.mods), m.annotations)
-        case xs: List[_] => pos = unionPos(pos, xs)
+        case p: Positioned if sameSource(p) => pos = include(pos, p)
+        case m: untpd.Modifiers => pos = includeAll(includeAll(pos, m.mods), m.annotations)
+        case xs: List[_] => pos = includeAll(pos, xs)
         case _ =>
       }
     }
     pos.toSynthetic
   }
 
-  private def unionPos(pos: Position, xs: List[_]): Position = xs match {
-    case (p: Positioned) :: xs1 if sameSource(p) => unionPos(pos union p.pos, xs1)
-    case (xs0: List[_]) :: xs1 => unionPos(unionPos(pos, xs0), xs1)
-    case _ :: xs1 => unionPos(pos, xs1)
-    case _ => pos
-  }
-
   private def sameSource(that: Positioned) = srcfile == that.srcfile
 
   def contains(that: Positioned): Boolean = {

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

@@ -16,7 +16,7 @@ object TreeIds {
   @sharable private[this] val counters = new ConcurrentHashMap[AbstractFile, AtomicInteger]
   @sharable private[this] val fileOfChunk = mutable.ArrayBuffer[AbstractFile]()
 
-  def nextId(implicit src: SourceInfo): Int = nextIdFor(src.source.file)
+  def nextId(implicit src: SourceInfo): Int = nextIdFor(src.srcfile)
 
   def nextIdFor(file: AbstractFile): Int = {
     var ctr = counters.get(file)

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

@@ -598,7 +598,7 @@ object Trees {
   case class Inlined[-T >: Untyped] private[ast] (call: tpd.Tree, bindings: List[MemberDef[T]], expansion: Tree[T])(implicit @transientParam src: SourceInfo)
     extends Tree[T] {
     type ThisTree[-T >: Untyped] = Inlined[T]
-    override def initialPos: Position = call.pos
+    override def initialSpan(ignoreTypeTrees: Boolean): Position = call.pos
   }
 
   /** A type tree that represents an existing or inferred type */
@@ -790,7 +790,7 @@ object Trees {
       if (trees eq newTrees)
         this
       else
-        Thicket[T](newTrees).asInstanceOf[this.type]
+        Thicket[T](newTrees)(SourceInfo(srcfile)).asInstanceOf[this.type]
     }
 
     override def foreachInThicket(op: Tree[T] => Unit): Unit =

compiler/src/dotty/tools/dotc/core/Contexts.scala

@@ -20,7 +20,7 @@ import config.Settings._
 import config.Config
 import reporting._
 import reporting.diagnostic.Message
-import io.{AbstractFile, PlainFile, Path}
+import io.{AbstractFile, NoAbstractFile, PlainFile, Path}
 import scala.io.Codec
 import collection.mutable
 import printing._
@@ -38,8 +38,12 @@ import plugins._
 object Contexts {
 
   trait SourceInfo {
-    def source: SourceFile
-    def withSource(file: AbstractFile): SourceInfo
+    def srcfile: AbstractFile
+  }
+  object SourceInfo {
+    def apply(src: AbstractFile) = new SourceInfo {
+      def srcfile = src
+    }
   }
 
   private val (compilerCallbackLoc, store1) = Store.empty.newLocation[CompilerCallback]()
@@ -169,6 +173,8 @@ object Contexts {
     protected def source_=(source: SourceFile): Unit = _source = source
     def source: SourceFile = _source
 
+    def srcfile = source.file
+
     /** A map in which more contextual properties can be stored
      *  Typically used for attributes that are read and written only in special situations.
      */

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

@@ -48,16 +48,16 @@ class PositionPickler(pickler: TastyPickler, addrOfTree: untpd.Tree => Option[Ad
       buf.writeInt(pickler.nameBuffer.nameIndex(source.path.toTermName).index)
     }
 
-    /** True if x's position shouldn't be reconstructed automatically from its initialPos
+    /** True if x's position shouldn't be reconstructed automatically from its initial span
      */
     def alwaysNeedsPos(x: Positioned) = x match {
       case
-          // initialPos is inaccurate for trees with lazy field
+          // initialSpan 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.
+          // so we cannot use the tree initialSpan to set the symbol position.
           // Instead, we always pickle the position of definitions.
           | _: Trees.DefTree[_]
 
@@ -70,8 +70,13 @@ class PositionPickler(pickler: TastyPickler, addrOfTree: untpd.Tree => Option[Ad
       case x: untpd.Tree =>
         var sourceFile = current
         val pos = if (x.isInstanceOf[untpd.MemberDef]) x.pos else x.pos.toSynthetic
-        val sourceChange = x.source.file != current
-        if (pos.exists && (pos != x.initialPos.toSynthetic || sourceChange || alwaysNeedsPos(x))) {
+        val sourceChange =
+          x.source.file != x.elemsFile ||
+          x.elemsFile == null && x.source.file != current
+        if (pos.exists && (
+              pos != x.initialSpan(ignoreTypeTrees = true).toSynthetic ||
+              sourceChange ||
+              alwaysNeedsPos(x))) {
           addrOfTree(x) match {
             case Some(addr) if !pickledIndices.contains(addr.index) || sourceChange =>
               // we currently do not share trees when unpickling, so if one path to a tree contains