SI-6879 improves Context.freshName

Instead of per-compilation unit unique counters, the freshName API now
uses a per-Global counter. Fresh names now also contain dollars to exclude
clashes with supported user-defined names (the ones without dollar signs).

This doesn’t fix the bug, because per-Global counters get created anew
every time a new Global is instantiated, and that provides some potential
for name clashes even for def macros, but at least it completely excludes
clashes in typical situations.

5 files changed, 58 insertions, 18 deletions

diff --git a/src/compiler/scala/reflect/macros/contexts/Names.scala b/src/compiler/scala/reflect/macros/contexts/Names.scala
index c2f14cf0f1..299af40b94 100644
--- a/src/compiler/scala/reflect/macros/contexts/Names.scala
+++ b/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]
 }
\ No newline at end of file
diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala
index ed3e7dbc4c..7015105261 100644
--- a/src/reflect/scala/reflect/internal/StdNames.scala
+++ b/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>"
diff --git a/src/reflect/scala/reflect/internal/SymbolTable.scala b/src/reflect/scala/reflect/internal/SymbolTable.scala
index 571c4cfa5d..733eecf3f6 100644
--- a/src/reflect/scala/reflect/internal/SymbolTable.scala
+++ b/src/reflect/scala/reflect/internal/SymbolTable.scala
@@ -141,6 +141,10 @@ abstract class SymbolTable extends macros.Universe
     )
   }
 
+  // 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
+
   /** Dump each symbol to stdout after shutdown.
    */
   final val traceSymbolActivity = sys.props contains "scalac.debug.syms"
diff --git a/src/reflect/scala/reflect/macros/Names.scala b/src/reflect/scala/reflect/macros/Names.scala
index af60dffbfc..4f3448e1ed 100644
--- a/src/reflect/scala/reflect/macros/Names.scala
+++ b/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
 }
diff --git a/src/reflect/scala/reflect/runtime/JavaUniverse.scala b/src/reflect/scala/reflect/runtime/JavaUniverse.scala
index 54b75b8e5b..cfd66744ff 100644
--- a/src/reflect/scala/reflect/runtime/JavaUniverse.scala
+++ b/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 {