Skip to content

Commit

Permalink
SI-6879 improves Context.freshName
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
xeno-by committed Jan 22, 2014
1 parent a242101 commit 47d1fb1
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 21 deletions.
28 changes: 22 additions & 6 deletions src/compiler/scala/reflect/macros/contexts/Names.scala
Expand Up @@ -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()
Expand All @@ -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]
}
2 changes: 2 additions & 0 deletions src/reflect/scala/reflect/internal/StdNames.scala
Expand Up @@ -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>"
Expand Down
4 changes: 4 additions & 0 deletions src/reflect/scala/reflect/internal/SymbolTable.scala
Expand Up @@ -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"
Expand Down
40 changes: 29 additions & 11 deletions src/reflect/scala/reflect/macros/Names.scala
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion src/reflect/scala/reflect/runtime/JavaUniverse.scala
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions 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

0 comments on commit 47d1fb1

Please sign in to comment.