Merge pull request scala#3401 from xeno-by/topic/fresh

SI-6879 improves Context.freshName

Author: xeno-by

src/compiler/scala/reflect/macros/contexts/Names.scala

@@ -4,7 +4,9 @@ package contexts
 trait Names {
   self: Context =>
 
-  def freshNameCreator = callsiteTyper.context.unit.fresh
+  import global._
+
+  def freshNameCreator = globalFreshNameCreator
 
   def fresh(): String =
     freshName()
@@ -16,11 +18,25 @@ trait Names {
     freshName[NameType](name)
 
   def freshName(): String =
-    freshName("fresh$")
-
-  def freshName(name: String): String =
-    freshNameCreator.newName(name)
+    freshName(nme.FRESH_PREFIX)
+
+  def freshName(name: String): String = {
+    // In comparison with the first version of freshName, current "fresh" names
+    // at least can't clash with legible user-written identifiers and are much less likely to clash with each other.
+    // It is still not good enough however, because the counter gets reset every time we create a new Global.
+    //
+    // This would most certainly cause problems if Scala featured something like introduceTopLevel,
+    // but even for def macros this can lead to unexpected troubles. Imagine that one Global
+    // creates a term of an anonymous type with a member featuring a "fresh" name, and then another Global
+    // imports that term with a wildcard and then generates a "fresh" name of its own. Given unlucky
+    // circumstances these "fresh" names might end up clashing.
+    //
+    // TODO: hopefully SI-7823 will provide an ultimate answer to this problem.
+    // In the meanwhile I will also keep open the original issue: SI-6879 "c.freshName is broken".
+    val sortOfUniqueSuffix = freshNameCreator.newName(nme.FRESH_SUFFIX)
+    name + "$" + sortOfUniqueSuffix
+  }
 
   def freshName[NameType <: Name](name: NameType): NameType =
-    name.mapName(freshNameCreator.newName(_)).asInstanceOf[NameType]
+    name.mapName(freshName(_)).asInstanceOf[NameType]
 }

src/reflect/scala/reflect/internal/FreshNames.scala

@@ -9,6 +9,10 @@ package internal
 import scala.reflect.internal.util.FreshNameCreator
 
 trait FreshNames { self: Names =>
+  // SI-6879 Keeps track of counters that are supposed to be globally unique
+  //         as opposed to traditional freshers that are unique to compilation units.
+  val globalFreshNameCreator = new FreshNameCreator
+
   // default fresh name creator used to abstract over currentUnit.fresh and runtime fresh name creator
   def currentFreshNameCreator: FreshNameCreator
 

src/reflect/scala/reflect/internal/StdNames.scala

@@ -305,6 +305,8 @@ trait StdNames {
     val PROTECTED_SET_PREFIX          = PROTECTED_PREFIX + "set"
     val SUPER_PREFIX_STRING           = "super$"
     val WHILE_PREFIX                  = "while$"
+    val FRESH_PREFIX                  = "fresh"
+    val FRESH_SUFFIX                  = "macro$" // uses a keyword to avoid collisions with mangled names
 
     // Compiler internal names
     val ANYname: NameType                  = "<anyname>"

src/reflect/scala/reflect/macros/Names.scala

@@ -6,33 +6,51 @@ package macros
  * <span class="badge badge-red" style="float: right;">EXPERIMENTAL</span>
  *
  *  A slice of [[scala.reflect.macros.blackbox.Context the Scala macros context]] that
- *  provides functions that generate unique names.
+ *  provides functions that generate fresh names.
+ *
+ *  In the current implementation, fresh names are more or less unique in the sense that
+ *  within the same compilation run they are guaranteed not to clash with:
+ *    1) Results of past and future invocations of functions of `freshName` family
+ *    2) User-defined or macro-generated names that don't contain dollar symbols
+ *    3) Macro-generated names that are created by concatenating names from the first, second and third categories
+ *
+ *  Uniqueness of fresh names across compilation runs is not guaranteed, but that's something
+ *  that we would like to improve upon in future releases. See [[https://issues.scala-lang.org/browse/SI-6879]] for more information.
+ *
+ *  @define freshNameNoParams
+ *  Creates a string that represents a more or less unique name.
+ *  Consult [[scala.reflect.macros.Names]] for more information on uniqueness of such names.
+ *
+ *  @define freshNameStringParam
+ *  Creates a string that represents a more or less unique name having a given prefix.
+ *  Consult [[scala.reflect.macros.Names]] for more information on uniqueness of such names.
+ *
+ *  @define freshNameNameParam
+ *  Creates a more or less unique name having a given name as a prefix and
+ *  having the same flavor (term name or type name) as the given name.
+ *  Consult [[scala.reflect.macros.Names]] for more information on uniqueness of such names.
  */
 trait Names {
   self: blackbox.Context =>
 
-  /** Creates a unique string. */
+  /** $freshNameNoParams */
   @deprecated("Use freshName instead", "2.11.0")
   def fresh(): String
 
-  /** Creates a unique string having a given prefix. */
+  /** $freshNameStringParam */
   @deprecated("Use freshName instead", "2.11.0")
   def fresh(name: String): String
 
-  /** Creates a unique name having a given name as a prefix and
-   *  having the same flavor (term name or type name) as the given name.
-   */
+  /** $freshNameNameParam */
   @deprecated("Use freshName instead", "2.11.0")
   def fresh[NameType <: Name](name: NameType): NameType
 
-  /** Creates a unique string. */
+  /** $freshNameNoParams */
   def freshName(): String
 
-  /** Creates a unique string having a given prefix. */
+  /** $freshNameStringParam */
   def freshName(name: String): String
 
-  /** Creates a unique name having a given name as a prefix and
-   *  having the same flavor (term name or type name) as the given name.
-   */
+  /** $freshNameNameParam */
   def freshName[NameType <: Name](name: NameType): NameType
 }

src/reflect/scala/reflect/runtime/JavaUniverse.scala

@@ -21,7 +21,7 @@ class JavaUniverse extends internal.SymbolTable with JavaUniverseForce with Refl
   def newStrictTreeCopier: TreeCopier = new StrictTreeCopier
   def newLazyTreeCopier: TreeCopier = new LazyTreeCopier
 
-  val currentFreshNameCreator = new reflect.internal.util.FreshNameCreator
+  def currentFreshNameCreator = globalFreshNameCreator
 
   // can't put this in runtime.Trees since that's mixed with Global in ReflectGlobal, which has the definition from internal.Trees
   object treeInfo extends {

test/files/run/macro-abort-fresh.check

@@ -1,6 +1,6 @@
-fresh$1
-qwe1
-qwe2
+fresh$macro$1
+qwe$macro$2
+qwe$macro$3
 reflective compilation has failed:
 
 blargh