SI-7096 SubstSymMap copies trees before modifying their symbols

I removed some strange code in a06d31f and replaced it by something
incorrect: SubstSymMap should never have side-effects: otherwise,
calling 'tpe1 <: tpe2' for instance would modify the symbols in
annotations of tpe2.

SubstSymMap now always creates new trees before changing them.

src/reflect/scala/reflect/internal/Types.scala

@@ -4698,23 +4698,39 @@ trait Types extends api.Types { self: SymbolTable =>
       }
     }
 
-    override def mapOver(tree: Tree, giveup: ()=>Nothing): Tree = {
-      object trans extends TypeMapTransformer {
+    object mapTreeSymbols extends TypeMapTransformer {
+      val strictCopy = newStrictTreeCopier
 
-        def termMapsTo(sym: Symbol) = from indexOf sym match {
-          case -1   => None
-          case idx  => Some(to(idx))
-        }
+      def termMapsTo(sym: Symbol) = from indexOf sym match {
+        case -1   => None
+        case idx  => Some(to(idx))
+      }
 
-        override def transform(tree: Tree) = {
-          termMapsTo(tree.symbol) match {
-            case Some(tosym) => tree.symbol = tosym
-            case None => ()
-          }
-          super.transform(tree)
+      // if tree.symbol is mapped to another symbol, passes the new symbol into the
+      // constructor `trans` and sets the symbol and the type on the resulting tree.
+      def transformIfMapped(tree: Tree)(trans: Symbol => Tree) = termMapsTo(tree.symbol) match {
+        case Some(toSym) => trans(toSym) setSymbol toSym setType tree.tpe
+        case None => tree
+      }
+
+      // changes trees which refer to one of the mapped symbols. trees are copied before attributes are modified.
+      override def transform(tree: Tree) = {
+        // super.transform maps symbol references in the types of `tree`. it also copies trees where necessary.
+        super.transform(tree) match {
+          case id @ Ident(_) =>
+            transformIfMapped(id)(toSym =>
+              strictCopy.Ident(id, toSym.name))
+
+          case sel @ Select(qual, name) =>
+            transformIfMapped(sel)(toSym =>
+              strictCopy.Select(sel, qual, toSym.name))
+
+          case tree => tree
         }
       }
-      trans.transform(tree)
+    }
+    override def mapOver(tree: Tree, giveup: ()=>Nothing): Tree = {
+      mapTreeSymbols.transform(tree)
     }
   }
 

test/files/run/t7096.check

@@ -0,0 +1,2 @@
+testing symbol List(method foo, class Base, package ano, package <root>), param value x, xRefs List(x)
+testing symbol List(method foo, class Sub, package ano, package <root>), param value x, xRefs List(x)

test/files/run/t7096.scala

@@ -0,0 +1,36 @@
+import scala.tools.partest._
+import scala.tools.nsc._
+
+object Test extends CompilerTest {
+  import global._
+  import definitions._
+
+  override def code = """
+package ano
+
+class ann(x: Any) extends annotation.TypeConstraint
+
+abstract class Base {
+  def foo(x: String): String @ann(x.trim())
+}
+
+class Sub extends Base {
+  def foo(x: String): String @ann(x.trim()) = x
+}
+  """
+
+  object syms extends SymsInPackage("ano")
+  import syms._
+
+  def check(source: String, unit: global.CompilationUnit) {
+    afterTyper {
+      terms.filter(_.name.toString == "foo").foreach(sym => {
+        val xParam = sym.tpe.paramss.flatten.head
+        val annot = sym.tpe.finalResultType.annotations.head
+        val xRefs = annot.args.head.filter(t => t.symbol == xParam)
+        println(s"testing symbol ${sym.ownerChain}, param $xParam, xRefs $xRefs")
+        assert(xRefs.length == 1, xRefs)
+      })
+    }
+  }
+}