Fixes to TreeTypeMap

(1) Template nodes have to be treated specially. They contain
    primary constructors, self definitions and local dummys,
    all of which have to be properly mapped and re-integrated.
(2) Symbol substitutions have ot be done all together instead of
    one after the other.
(3) When creating new symbols, need to create ClassSymbols for ClassSymbols.

4 files changed, 88 insertions, 39 deletions

diff --git a/src/dotty/tools/dotc/ast/tpd.scala b/src/dotty/tools/dotc/ast/tpd.scala
index f6940f635..802743736 100644
--- a/src/dotty/tools/dotc/ast/tpd.scala
+++ b/src/dotty/tools/dotc/ast/tpd.scala
@@ -484,7 +484,7 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
       shallowFold[Option[tpd.Tree]](None)((accum, tree) => if (pred(tree)) Some(tree) else accum)
 
     def subst(from: List[Symbol], to: List[Symbol])(implicit ctx: Context): ThisTree =
-      new TreeTypeMap(typeMap = new ctx.SubstSymMap(from, to)).apply(tree)
+      new TreeTypeMap(substFrom = from, substTo = to).apply(tree)
 
     def changeOwner(from: Symbol, to: Symbol)(implicit ctx: Context): ThisTree =
       new TreeTypeMap(ownerMap = (sym => if (sym == from) to else sym)).apply(tree)
@@ -549,36 +549,66 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
   /** A map that applies three functions together to a tree and makes sure
    *  they are coordinated so that the result is well-typed. The functions are
    *  @param  typeMap  A function from Type to type that gets applied to the
-   *                   type of every tree node and to all locally defined symbols
+   *                   type of every tree node and to all locally defined symbols,
+   *                   followed by the substitution [substFrom := substTo].
    *  @param ownerMap  A function that translates owners of top-level local symbols
    *                   defined in the mapped tree.
    *  @param treeMap   A transformer that translates all encountered subtrees in
    *                   prefix traversal order.
+   *  @param substFrom The symbols that need to be substituted
+   *  @param substTo   The substitution targets
+   *
+   *  The reason the substitution is broken out form the rest of the type map is
+   *  that all symbols have to be substituted at the same time. If we do not do this,
+   *  we risk data races on named types. Example: Say we have `outer#1.inner#2` and we
+   *  have two substitutons S1 = [outer#1 := outer#3], S2 = [inner#2 := inner#4] where
+   *  hashtags precede symbol ids. If we do S1 first, we get outer#2.inner#3. If we then
+   *  do S2 we get outer#2.inner#4. But that means that the named type outer#2.inner
+   *  gets two different denotations in the same period. Hence, if -YnoDoubleBindings is
+   *  set, we would get a data race assertion error.
    */
   final class TreeTypeMap(
       val typeMap: Type => Type = IdentityTypeMap,
       val ownerMap: Symbol => Symbol = identity _,
-      val treeMap: Tree => Tree = identity _)(implicit ctx: Context) extends TreeMap {
+      val treeMap: Tree => Tree = identity _,
+      val substFrom: List[Symbol] = Nil,
+      val substTo: List[Symbol] = Nil)(implicit ctx: Context) extends TreeMap {
+
+    def mapType(tp: Type) = typeMap(tp).substSym(substFrom, substTo)
 
     override def transform(tree: tpd.Tree)(implicit ctx: Context): tpd.Tree = {
       val tree1 = treeMap(tree)
-      tree1.withType(typeMap(tree1.tpe)) match {
-        case ddef @ DefDef(mods, name, tparams, vparamss, tpt, rhs) =>
-          val (tmap1, tparams1) = transformDefs(ddef.tparams)
-          val (tmap2, vparamss1) = tmap1.transformVParamss(vparamss)
-          cpy.DefDef(ddef)(mods, name, tparams1, vparamss1, tmap2.transform(tpt), tmap2.transform(rhs))
-        case blk @ Block(stats, expr) =>
-          val (tmap1, stats1) = transformDefs(stats)
-          val expr1 = tmap1.transform(expr)
-          cpy.Block(blk)(stats1, expr1)
-        case cdef @ CaseDef(pat, guard, rhs) =>
-          val tmap = withMappedSyms(patVars(pat))
-          val pat1 = tmap.transform(pat)
-          val guard1 = tmap.transform(guard)
-          val rhs1 = tmap.transform(rhs)
-          cpy.CaseDef(tree)(pat1, guard1, rhs1)
-        case tree1 =>
-          super.transform(tree1)
+      tree1 match {
+        case impl @ Template(constr, parents, self, body) =>
+          val selfToMap = if (self.symbol.exists) self.symbol :: Nil else Nil
+          val symsToMap = impl.symbol :: impl.constr.symbol :: selfToMap
+          val mappedSyms = ctx.mapSymbols(symsToMap, typeMap, ownerMap, substFrom, substTo)
+          val tmap = withSubstitution(symsToMap, mappedSyms)
+          val constr1 = tmap.transformSub(constr)
+          val parents1 = parents mapconserve transform
+          var self1 = tmap.transformSub(self)
+          if (self.symbol.exists) self1 = self1.withType(mappedSyms.last.termRef)
+          val body1 = tmap.transformStats(body)
+          cpy.Template(impl)(constr1, parents1, self1, body1).withType(mappedSyms.head.nonMemberTermRef)
+        case _ =>
+          tree1.withType(mapType(tree1.tpe)) match {
+            case ddef @ DefDef(mods, name, tparams, vparamss, tpt, rhs) =>
+              val (tmap1, tparams1) = transformDefs(ddef.tparams)
+              val (tmap2, vparamss1) = tmap1.transformVParamss(vparamss)
+              cpy.DefDef(ddef)(mods, name, tparams1, vparamss1, tmap2.transform(tpt), tmap2.transform(rhs))
+            case blk @ Block(stats, expr) =>
+              val (tmap1, stats1) = transformDefs(stats)
+              val expr1 = tmap1.transform(expr)
+              cpy.Block(blk)(stats1, expr1)
+            case cdef @ CaseDef(pat, guard, rhs) =>
+              val tmap = withMappedSyms(patVars(pat))
+              val pat1 = tmap.transform(pat)
+              val guard1 = tmap.transform(guard)
+              val rhs1 = tmap.transform(rhs)
+              cpy.CaseDef(tree)(pat1, guard1, rhs1)
+            case tree1 =>
+              super.transform(tree1)
+          }
       }
     }
 
@@ -609,20 +639,27 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
     /** The current tree map composed with a substitution [from -> to] */
     def withSubstitution(from: List[Symbol], to: List[Symbol]): TreeTypeMap =
       if (from eq to) this
-      else new TreeTypeMap(
-        typeMap andThen (_.substSym(from, to)),
-        ownerMap andThen { sym =>
-          val idx = from.indexOf(sym)
-          if (idx >= 0) to(idx) else sym
-        },
-        treeMap)
+      else {
+        // assert that substitution stays idempotent, assuming its parts are
+        assert(!from.exists(substTo contains _))
+        assert(!to.exists(substFrom contains _))
+        new TreeTypeMap(
+          typeMap,
+          ownerMap andThen { sym =>
+            val idx = from.indexOf(sym)
+            if (idx >= 0) to(idx) else sym
+          },
+          treeMap,
+          from ++ substFrom,
+          to ++ substTo)
+      }
 
     /** Apply `typeMap` and `ownerMap` to given symbols `syms`
      *  and return a treemap that contains the substitution
      *  between original and mapped symbols.
      */
     def withMappedSyms(syms: List[Symbol]): TreeTypeMap = {
-      val mapped = ctx.mapSymbols(syms, typeMap, ownerMap)
+      val mapped = ctx.mapSymbols(syms, typeMap, ownerMap, substFrom, substTo)
       withSubstitution(syms, mapped)
     }
   }
diff --git a/src/dotty/tools/dotc/core/Periods.scala b/src/dotty/tools/dotc/core/Periods.scala
index e0d9e3b5d..66c26e381 100644
--- a/src/dotty/tools/dotc/core/Periods.scala
+++ b/src/dotty/tools/dotc/core/Periods.scala
@@ -129,7 +129,6 @@ object Periods {
     /** The interval consisting of all periods of given run id */
     def allInRun(rid: RunId) =
       apply(rid, 0, PhaseMask)
-
   }
 
   final val Nowhere = new Period(0)
diff --git a/src/dotty/tools/dotc/core/Symbols.scala b/src/dotty/tools/dotc/core/Symbols.scala
index 06414818f..4156d59d1 100644
--- a/src/dotty/tools/dotc/core/Symbols.scala
+++ b/src/dotty/tools/dotc/core/Symbols.scala
@@ -261,29 +261,42 @@ trait Symbols { this: Context =>
     newSymbol(owner, name, SyntheticArtifact,
       if (name.isTypeName) TypeAlias(ErrorType) else ErrorType)
 
-  /** Map given symbols, subjecting all types to given type map and owner map.
+  /** Map given symbols, subjecting all types to given type map and owner map,
+   *  as well as to the substition [substFrom := substTo].
    *  Cross symbol references are brought over from originals to copies.
    *  Do not copy any symbols if all attributes of all symbols stay the same.
    */
   def mapSymbols(
       originals: List[Symbol],
       typeMap: Type => Type = IdentityTypeMap,
-      ownerMap: Symbol => Symbol = identity)
+      ownerMap: Symbol => Symbol = identity,
+      substFrom: List[Symbol] = Nil,
+      substTo: List[Symbol] = Nil)
   =
     if (originals forall (sym =>
-        (typeMap(sym.info) eq sym.info) && (ownerMap(sym.owner) eq sym.owner)))
+        (typeMap(sym.info) eq sym.info) &&
+        (sym.info.substSym(substFrom, substTo) eq sym.info) &&
+        (ownerMap(sym.owner) eq sym.owner)))
       originals
     else {
       val copies: List[Symbol] = for (original <- originals) yield
-        newNakedSymbol[original.ThisName](original.coord)
-      val treeMap = new TreeTypeMap(typeMap, ownerMap)
+        original match {
+          case original: ClassSymbol =>
+            newNakedClassSymbol(original.coord, original.assocFile)
+          case _ =>
+            newNakedSymbol[original.ThisName](original.coord)
+        }
+      val treeMap = new TreeTypeMap(typeMap, ownerMap, substFrom = substFrom, substTo = substTo)
         .withSubstitution(originals, copies)
       (originals, copies).zipped foreach {(original, copy) =>
+        copy.denot = original.denot // preliminar denotation, so that we can access symbols in subsequent transform
+      }
+      (originals, copies).zipped foreach {(original, copy) =>
         val odenot = original.denot
         copy.denot = odenot.copySymDenotation(
           symbol = copy,
           owner = treeMap.ownerMap(odenot.owner),
-          info = treeMap.typeMap(odenot.info),
+          info = treeMap.mapType(odenot.info),
           privateWithin = ownerMap(odenot.privateWithin), // since this refers to outer symbols, need not include copies (from->to) in ownermap here.
           annotations = odenot.annotations.mapConserve(treeMap.apply))
       }
@@ -318,7 +331,7 @@ object Symbols {
     type ThisName <: Name
 
     private[this] var _id: Int = nextId
-    //assert(_id != 5859)
+    //assert(_id != 30214)
 
     /** The unique id of this symbol */
     def id = _id
@@ -479,7 +492,7 @@ object Symbols {
   type TermSymbol = Symbol { type ThisName = TermName }
   type TypeSymbol = Symbol { type ThisName = TypeName }
 
-  class ClassSymbol private[Symbols] (coord: Coord, assocFile: AbstractFile)
+  class ClassSymbol private[Symbols] (coord: Coord, val assocFile: AbstractFile)
     extends Symbol(coord) {
 
     type ThisName = TypeName
diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala
index 63f9eb0ab..82b2fc71a 100644
--- a/src/dotty/tools/dotc/core/Types.scala
+++ b/src/dotty/tools/dotc/core/Types.scala
@@ -2475,7 +2475,7 @@ object Types {
     }
 
     def mapOver(syms: List[Symbol]): List[Symbol] =
-      ctx.mapSymbols(syms, this)
+      ctx.mapSymbols(syms, typeMap = this)
 
     def mapOver(scope: Scope): Scope = {
       val elems = scope.toList
@@ -2488,7 +2488,7 @@ object Types {
       annot.derivedAnnotation(mapOver(annot.tree))
 
     def mapOver(tree: Tree): Tree =
-      new TreeTypeMap(this).apply(tree)
+      new TreeTypeMap(typeMap = this).apply(tree)
 
     /** Can be overridden. By default, only the prefix is mapped. */
     protected def mapClassInfo(tp: ClassInfo): ClassInfo =