Further optimize name freshening

retronym · retronym · commit 920fa5837338 · 2018-05-04T16:50:49.000+10:00
- Introduce a per-Global store of names
  - Avoid double freshening names
  - Use $async$ in all names to avoid clashes with, e.g. lambda lifted methods.
diff --git a/src/main/scala/scala/async/internal/AnfTransform.scala b/src/main/scala/scala/async/internal/AnfTransform.scala
@@ -77,7 +77,7 @@ private[async] trait AnfTransform {
             stats :+ expr :+ api.typecheck(atPos(expr.pos)(Throw(Apply(Select(New(gen.mkAttributedRef(defn.IllegalStateExceptionClass)), nme.CONSTRUCTOR), Nil))))
           expr match {
             case Apply(fun, args) if isAwait(fun) =>
-              val valDef = defineVal(name.await, expr, tree.pos)
+              val valDef = defineVal(name.await(), expr, tree.pos)
               val ref = gen.mkAttributedStableRef(valDef.symbol).setType(tree.tpe)
               val ref1 = if (ref.tpe =:= definitions.UnitTpe)
                 // https://github.com/scala/async/issues/74
@@ -109,7 +109,7 @@ private[async] trait AnfTransform {
               } else if (expr.tpe =:= definitions.NothingTpe) {
                 statsExprThrow
               } else {
-                val varDef = defineVar(name.ifRes, expr.tpe, tree.pos)
+                val varDef = defineVar(name.ifRes(), expr.tpe, tree.pos)
                 def typedAssign(lhs: Tree) =
                   api.typecheck(atPos(lhs.pos)(Assign(Ident(varDef.symbol), mkAttributedCastPreservingAnnotations(lhs, tpe(varDef.symbol)))))
 
@@ -140,7 +140,7 @@ private[async] trait AnfTransform {
               } else if (expr.tpe =:= definitions.NothingTpe) {
                 statsExprThrow
               } else {
-                val varDef = defineVar(name.matchRes, expr.tpe, tree.pos)
+                val varDef = defineVar(name.matchRes(), expr.tpe, tree.pos)
                 def typedAssign(lhs: Tree) =
                   api.typecheck(atPos(lhs.pos)(Assign(Ident(varDef.symbol), mkAttributedCastPreservingAnnotations(lhs, tpe(varDef.symbol)))))
                 val casesWithAssign = cases map {
@@ -163,14 +163,14 @@ private[async] trait AnfTransform {
           }
         }
 
-        def defineVar(prefix: TermName, tp: Type, pos: Position): ValDef = {
-          val sym = api.currentOwner.newTermSymbol(name.fresh(prefix), pos, MUTABLE | SYNTHETIC).setInfo(uncheckedBounds(tp))
+        def defineVar(name: TermName, tp: Type, pos: Position): ValDef = {
+          val sym = api.currentOwner.newTermSymbol(name, pos, MUTABLE | SYNTHETIC).setInfo(uncheckedBounds(tp))
           valDef(sym, mkZero(uncheckedBounds(tp))).setType(NoType).setPos(pos)
         }
       }
 
-      def defineVal(prefix: TermName, lhs: Tree, pos: Position): ValDef = {
-        val sym = api.currentOwner.newTermSymbol(name.fresh(prefix), pos, SYNTHETIC).setInfo(uncheckedBounds(lhs.tpe))
+      def defineVal(name: TermName, lhs: Tree, pos: Position): ValDef = {
+        val sym = api.currentOwner.newTermSymbol(name, pos, SYNTHETIC).setInfo(uncheckedBounds(lhs.tpe))
         internal.valDef(sym, internal.changeOwner(lhs, api.currentOwner, sym)).setType(NoType).setPos(pos)
       }
 
@@ -212,7 +212,7 @@ private[async] trait AnfTransform {
                   case Arg(expr, _, argName)                                    =>
                     linearize.transformToList(expr) match {
                       case stats :+ expr1 =>
-                        val valDef = defineVal(argName, expr1, expr1.pos)
+                        val valDef = defineVal(name.freshen(argName), expr1, expr1.pos)
                         require(valDef.tpe != null, valDef)
                         val stats1 = stats :+ valDef
                         (stats1, atPos(tree.pos.makeTransparent)(gen.stabilize(gen.mkAttributedIdent(valDef.symbol))))
@@ -279,8 +279,9 @@ private[async] trait AnfTransform {
                   // TODO we can move this into ExprBuilder once we get rid of `AsyncDefinitionUseAnalyzer`.
                   val block = linearize.transformToBlock(body)
                   val (valDefs, mappings) = (pat collect {
-                    case b@Bind(name, _) =>
-                      val vd = defineVal(name.toTermName + AnfTransform.this.name.bindSuffix, gen.mkAttributedStableRef(b.symbol).setPos(b.pos), b.pos)
+                    case b@Bind(bindName, _) =>
+                      val vd = defineVal(name.freshen(bindName.toTermName), gen.mkAttributedStableRef(b.symbol).setPos(b.pos), b.pos)
+                      vd.symbol.updateAttachment(SyntheticBindVal)
                       (vd, (b.symbol, vd.symbol))
                   }).unzip
                   val (from, to) = mappings.unzip
@@ -333,7 +334,7 @@ private[async] trait AnfTransform {
             // Otherwise, create the matchres var. We'll callers of the label def below.
             // Remember: we're iterating through the statement sequence in reverse, so we'll get
             // to the LabelDef and mutate `matchResults` before we'll get to its callers.
-            val matchResult = linearize.defineVar(name.matchRes, param.tpe, ld.pos)
+            val matchResult = linearize.defineVar(name.matchRes(), param.tpe, ld.pos)
             matchResults += matchResult
             caseDefToMatchResult(ld.symbol) = matchResult.symbol
             val rhs2 = ld.rhs.substituteSymbols(param.symbol :: Nil, matchResult.symbol :: Nil)
@@ -408,3 +409,5 @@ private[async] trait AnfTransform {
     }).asInstanceOf[Block]
   }
 }
+
+object SyntheticBindVal
diff --git a/src/main/scala/scala/async/internal/AsyncMacro.scala b/src/main/scala/scala/async/internal/AsyncMacro.scala
@@ -1,10 +1,19 @@
 package scala.async.internal
 
+import java.util
+
 object AsyncMacro {
+  private val nameCache = new util.WeakHashMap[Object, AsyncNames[_]]()
   def apply(c0: reflect.macros.Context, base: AsyncBase)(body0: c0.Tree): AsyncMacro { val c: c0.type } = {
     import language.reflectiveCalls
+    val asyncNames0 = nameCache.synchronized[AsyncNames[_]] {
+      nameCache.computeIfAbsent(c0.universe, new java.util.function.Function[Object, AsyncNames[_]] {
+        override def apply(t: Object): AsyncNames[_] = new AsyncNames[c0.universe.type](c0.universe)
+      })
+    }
     new AsyncMacro { self =>
       val c: c0.type                                             = c0
+      val asyncNames: AsyncNames[c.universe.type]                = asyncNames0.asInstanceOf[AsyncNames[c.universe.type]]
       val body: c.Tree = body0
       // This member is required by `AsyncTransform`:
       val asyncBase: AsyncBase                                   = base
@@ -23,6 +32,7 @@ private[async] trait AsyncMacro
   val c: scala.reflect.macros.Context
   val body: c.Tree
   var containsAwait: c.Tree => Boolean
+  val asyncNames: AsyncNames[c.universe.type]
 
   lazy val macroPos: c.universe.Position = c.macroApplication.pos.makeTransparent
   def atMacroPos(t: c.Tree): c.Tree = c.universe.atPos(macroPos)(t)
diff --git a/src/main/scala/scala/async/internal/AsyncNames.scala b/src/main/scala/scala/async/internal/AsyncNames.scala
@@ -0,0 +1,109 @@
+package scala.async.internal
+
+import java.util.concurrent.atomic.AtomicInteger
+
+import scala.collection.mutable
+import scala.collection.mutable.ArrayBuffer
+import scala.reflect.api.Names
+
+/**
+  * A per-global cache of names needed by the Async macro.
+  */
+final class AsyncNames[U <: Names with Singleton](val u: U) {
+  self =>
+  import u._
+
+  abstract class NameCache[N <: U#Name](base: String) {
+    val cached = new ArrayBuffer[N]()
+    protected def newName(s: String): N
+    def apply(i: Int): N = {
+      if (cached.isDefinedAt(i)) cached(i)
+      else {
+        assert(cached.length == i)
+        val name = newName(freshenString(base, i))
+        cached += name
+        name
+      }
+    }
+  }
+
+  final class TermNameCache(base: String) extends NameCache[U#TermName](base) {
+    override protected def newName(s: String): U#TermName = newTermName(s)
+  }
+  final class TypeNameCache(base: String) extends NameCache[U#TypeName](base) {
+    override protected def newName(s: String): U#TypeName = newTypeName(s)
+  }
+  private val matchRes: TermNameCache = new TermNameCache("match")
+  private val ifRes: TermNameCache = new TermNameCache("if")
+  private val await: TermNameCache = new TermNameCache("await")
+
+  private val resume = newTermName("resume")
+  private val completed: TermName = newTermName("completed$async")
+  private val apply = newTermName("apply")
+  private val stateMachine  = newTermName("stateMachine$async")
+  private val stateMachineT = stateMachine.toTypeName
+  private val state: u.TermName = newTermName("state$async")
+  private val execContext = newTermName("execContext$async")
+  private val tr: u.TermName = newTermName("tr$async")
+  private val t: u.TermName = newTermName("throwable$async")
+
+  final class NameSource[N <: U#Name](cache: NameCache[N]) {
+    private val count = new AtomicInteger(0)
+    def apply(): N = cache(count.getAndIncrement())
+  }
+
+  class AsyncName {
+    final val matchRes = new NameSource[U#TermName](self.matchRes)
+    final val ifRes = new NameSource[U#TermName](self.matchRes)
+    final val await = new NameSource[U#TermName](self.await)
+    final val completed = self.completed
+    final val result = self.resume
+    final val apply = self.apply
+    final val stateMachine = self.stateMachine
+    final val stateMachineT = self.stateMachineT
+    final val state: u.TermName = self.state
+    final val execContext = self.execContext
+    final val tr: u.TermName = self.tr
+    final val t: u.TermName = self.t
+
+    private val seenPrefixes = mutable.AnyRefMap[Name, AtomicInteger]()
+    private val freshened = mutable.HashSet[Name]()
+
+    final def freshenIfNeeded(name: TermName): TermName = {
+      seenPrefixes.getOrNull(name) match {
+        case null =>
+          seenPrefixes.put(name, new AtomicInteger())
+          name
+        case counter =>
+          freshen(name, counter)
+      }
+    }
+    final def freshenIfNeeded(name: TypeName): TypeName = {
+      seenPrefixes.getOrNull(name) match {
+        case null =>
+          seenPrefixes.put(name, new AtomicInteger())
+          name
+        case counter =>
+          freshen(name, counter)
+      }
+    }
+    final def freshen(name: TermName): TermName = {
+      val counter = seenPrefixes.getOrElseUpdate(name, new AtomicInteger())
+      freshen(name, counter)
+    }
+    final def freshen(name: TypeName): TypeName = {
+      val counter = seenPrefixes.getOrElseUpdate(name, new AtomicInteger())
+      freshen(name, counter)
+    }
+    private def freshen(name: TermName, counter: AtomicInteger): TermName = {
+      if (freshened.contains(name)) name
+      else TermName(freshenString(name.toString, counter.incrementAndGet()))
+    }
+    private def freshen(name: TypeName, counter: AtomicInteger): TypeName = {
+      if (freshened.contains(name)) name
+      else TypeName(freshenString(name.toString, counter.incrementAndGet()))
+    }
+  }
+
+  private def freshenString(name: String, counter: Int): String = name.toString + "$async$" + counter
+}
diff --git a/src/main/scala/scala/async/internal/ExprBuilder.scala b/src/main/scala/scala/async/internal/ExprBuilder.scala
@@ -95,7 +95,7 @@ trait ExprBuilder {
         c.Expr[futureSystem.Tryy[Any] => Unit](fun), c.Expr[futureSystem.ExecContext](Ident(name.execContext))).tree
       val tryGetOrCallOnComplete: List[Tree] =
         if (futureSystemOps.continueCompletedFutureOnSameThread) {
-          val tempName = name.fresh(name.completed)
+          val tempName = name.completed
           val initTemp = ValDef(NoMods, tempName, TypeTree(futureSystemOps.tryType[Any]), futureSystemOps.getCompleted[Any](c.Expr[futureSystem.Fut[Any]](awaitable.expr)).tree)
           val ifTree = If(Apply(Select(Literal(Constant(null)), TermName("ne")), Ident(tempName) :: Nil),
             adaptToUnit(ifIsFailureTree[T](Ident(tempName)) :: Nil),
@@ -520,7 +520,7 @@ trait ExprBuilder {
   }
 
   private def isSyntheticBindVal(tree: Tree) = tree match {
-    case vd@ValDef(_, lname, _, Ident(rname)) => lname.toString.contains(name.bindSuffix)
+    case vd@ValDef(_, lname, _, Ident(rname)) => attachments(vd.symbol).contains[SyntheticBindVal.type]
     case _                                    => false
   }
 
diff --git a/src/main/scala/scala/async/internal/FutureSystem.scala b/src/main/scala/scala/async/internal/FutureSystem.scala
@@ -75,6 +75,7 @@ trait FutureSystem {
 
   def mkOps(c0: Context): Ops { val c: c0.type }
 
+  @deprecated("No longer honoured by the macro, all generated names now contain $async to avoid accidental clashes with lambda lifted names", "0.9.7")
   def freshenAllNames: Boolean = false
   def emitTryCatch: Boolean = true
   def resultFieldName: String = "result"
diff --git a/src/main/scala/scala/async/internal/Lifter.scala b/src/main/scala/scala/async/internal/Lifter.scala
@@ -120,13 +120,13 @@ trait Lifter {
             val rhs1 = if (sym.asTerm.isLazy) rhs else EmptyTree
             treeCopy.ValDef(vd, Modifiers(sym.flags), sym.name, TypeTree(tpe(sym)).setPos(t.pos), rhs1)
           case dd@DefDef(_, _, tparams, vparamss, tpt, rhs) =>
-            sym.setName(this.name.fresh(sym.name.toTermName))
+            sym.setName(this.name.freshen(sym.name.toTermName))
             sym.setFlag(PRIVATE | LOCAL)
             // Was `DefDef(sym, rhs)`, but this ran afoul of `ToughTypeSpec.nestedMethodWithInconsistencyTreeAndInfoParamSymbols`
             // due to the handling of type parameter skolems in `thisMethodType` in `Namers`
             treeCopy.DefDef(dd, Modifiers(sym.flags), sym.name, tparams, vparamss, tpt, rhs)
           case cd@ClassDef(_, _, tparams, impl)             =>
-            sym.setName(newTypeName(name.fresh(sym.name.toString).toString))
+            sym.setName(name.freshen(sym.name.toTypeName))
             companionship.companionOf(cd.symbol) match {
               case NoSymbol     =>
               case moduleSymbol =>
@@ -137,13 +137,13 @@ trait Lifter {
           case md@ModuleDef(_, _, impl)                     =>
             companionship.companionOf(md.symbol) match {
               case NoSymbol    =>
-                sym.setName(name.fresh(sym.name.toTermName))
+                sym.setName(name.freshen(sym.name.toTermName))
                 sym.asModule.moduleClass.setName(sym.name.toTypeName)
               case classSymbol => // will be renamed by `case ClassDef` above.
             }
             treeCopy.ModuleDef(md, Modifiers(sym.flags), sym.name, impl)
           case td@TypeDef(_, _, tparams, rhs)               =>
-            sym.setName(newTypeName(name.fresh(sym.name.toString).toString))
+            sym.setName(name.freshen(sym.name.toTypeName))
             treeCopy.TypeDef(td, Modifiers(sym.flags), sym.name, tparams, rhs)
         }
         atPos(t.pos)(treeLifted)
diff --git a/src/main/scala/scala/async/internal/TransformUtils.scala b/src/main/scala/scala/async/internal/TransformUtils.scala
@@ -17,42 +17,8 @@ private[async] trait TransformUtils {
   import c.internal._
   import decorators._
 
-  private object baseNames {
-
-    val matchRes = "matchres"
-    val ifRes = "ifres"
-    val bindSuffix = "$bind"
-    val completed = newTermName("completed")
-
-    val state = newTermName("state")
-    val result = newTermName(self.futureSystem.resultFieldName)
-    val execContext = newTermName("execContext")
-    val tr = newTermName("tr")
-    val t = newTermName("throwable")
-  }
-  
-  object name {
-    val matchRes      = newTermName(baseNames.matchRes)
-    val ifRes         = newTermName(baseNames.ifRes)
-    def bindSuffix    = baseNames.bindSuffix
-    def completed     = baseNames.completed
-
-    val state         = maybeFresh(baseNames.state)
-    val result        = baseNames.result
-    val execContext   = maybeFresh(baseNames.execContext)
-    val tr            = maybeFresh(baseNames.tr)
-    val t             = maybeFresh(baseNames.t)
-
-    val await = newTermName("await")
-    val resume = newTermName("resume")
-    val apply = newTermName("apply")
-    val stateMachine  = newTermName(fresh("stateMachine"))
-    val stateMachineT = stateMachine.toTypeName
-
-    def maybeFresh(name: TermName): TermName = if (self.asyncBase.futureSystem.freshenAllNames) fresh(name) else name
-    def maybeFresh(name: String): String = if (self.asyncBase.futureSystem.freshenAllNames) fresh(name) else name
-    def fresh(name: TermName): TermName = c.freshName(name)
-
+  object name extends asyncNames.AsyncName {
+    def fresh(name: TermName): TermName = freshenIfNeeded(name)
     def fresh(name: String): String = c.freshName(name)
   }
 
diff --git a/src/test/scala/scala/async/TreeInterrogation.scala b/src/test/scala/scala/async/TreeInterrogation.scala
@@ -38,7 +38,7 @@ class TreeInterrogation {
     val varDefs = tree1.collect {
       case vd @ ValDef(mods, name, _, _) if mods.hasFlag(Flag.MUTABLE) && vd.symbol.owner.isClass => name
     }
-    varDefs.map(_.decoded.trim).toSet.toList.sorted mustStartWith (List("await$macro$", "await$macro$", "state"))
+    varDefs.map(_.decoded.trim).toSet.toList.sorted mustStartWith (List("await$async$", "await$async", "state$async"))
 
     val defDefs = tree1.collect {
       case t: Template =>
@@ -49,11 +49,11 @@ class TreeInterrogation {
               && !dd.symbol.asTerm.isAccessor && !dd.symbol.asTerm.isSetter => dd.name
         }
     }.flatten
-    defDefs.map(_.decoded.trim) mustStartWith List("foo$macro$", "<init>", "apply", "apply")
+    defDefs.map(_.decoded.trim) mustStartWith List("foo$async$", "<init>", "apply", "apply")
   }
 }
 
-object TreeInterrogation extends App {
+object TreeInterrogationApp extends App {
   def withDebug[T](t: => T): T = {
     def set(level: String, value: Boolean) = System.setProperty(s"scala.async.$level", value.toString)
     val levels = Seq("trace", "debug")
@@ -65,7 +65,7 @@ object TreeInterrogation extends App {
 
   withDebug {
     val cm = reflect.runtime.currentMirror
-    val tb = mkToolbox(s"-cp ${toolboxClasspath} -Xprint:typer -uniqid")
+    val tb = mkToolbox(s"-cp ${toolboxClasspath} -Xprint:typer")
     import scala.async.internal.AsyncId._
     val tree = tb.parse(
       """
@@ -75,6 +75,9 @@ object TreeInterrogation extends App {
         |   while(await(b)) {
         |     b = false
         |   }
+        |   (1, 1) match {
+        |     case (x, y) => await(2); println(x)
+        |   }
         |   await(b)
         | }
         |
diff --git a/src/test/scala/scala/async/run/anf/AnfTransformSpec.scala b/src/test/scala/scala/async/run/anf/AnfTransformSpec.scala
@@ -403,7 +403,8 @@ class AnfTransformSpec {
       """.stripMargin
     })
     val applyImplicitView = tree.collect { case x if x.getClass.getName.endsWith("ApplyImplicitView") => x }
-    applyImplicitView.map(_.toString) mustStartWith List("view(a$macro$")
+    println(applyImplicitView)
+    applyImplicitView.map(_.toString) mustStartWith List("view(")
   }
 
   @Test

Original file line number	Diff line number	Diff line change
`@@ -403,7 +403,8 @@ class AnfTransformSpec {`
`403`	`403`	`""".stripMargin`
`404`	`404`	`})`
`405`	`405`	`val applyImplicitView = tree.collect { case x if x.getClass.getName.endsWith("ApplyImplicitView") => x }`
`406`		`- applyImplicitView.map(_.toString) mustStartWith List("view(a$macro$")`
	`406`	`+ println(applyImplicitView)`
	`407`	`+ applyImplicitView.map(_.toString) mustStartWith List("view(")`
`407`	`408`	`}`
`408`	`409`
`409`	`410`	`@Test`