From 47d1fb1e9975998d5f8dde63c08c0b3ab1cd5ae2 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Wed, 22 Jan 2014 18:53:02 +0300 Subject: SI-6879 improves Context.freshName MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../scala/reflect/macros/contexts/Names.scala | 28 +++++++++++---- src/reflect/scala/reflect/internal/StdNames.scala | 2 ++ .../scala/reflect/internal/SymbolTable.scala | 4 +++ src/reflect/scala/reflect/macros/Names.scala | 40 ++++++++++++++++------ .../scala/reflect/runtime/JavaUniverse.scala | 2 +- test/files/run/macro-abort-fresh.check | 6 ++-- 6 files changed, 61 insertions(+), 21 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 = "" 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 * EXPERIMENTAL * * 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 { diff --git a/test/files/run/macro-abort-fresh.check b/test/files/run/macro-abort-fresh.check index 9fddee57d4..5064b96eef 100644 --- a/test/files/run/macro-abort-fresh.check +++ b/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 -- cgit v1.2.3 From 56d980c830ef9fc4c901a8c4b8e4b9c42adb49b4 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Wed, 22 Jan 2014 19:23:51 +0300 Subject: addresses pull request feedback --- src/reflect/scala/reflect/internal/FreshNames.scala | 4 ++++ src/reflect/scala/reflect/internal/SymbolTable.scala | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/reflect/scala/reflect/internal/FreshNames.scala b/src/reflect/scala/reflect/internal/FreshNames.scala index bb488aa2a8..1de8d425ad 100644 --- a/src/reflect/scala/reflect/internal/FreshNames.scala +++ b/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 diff --git a/src/reflect/scala/reflect/internal/SymbolTable.scala b/src/reflect/scala/reflect/internal/SymbolTable.scala index 733eecf3f6..571c4cfa5d 100644 --- a/src/reflect/scala/reflect/internal/SymbolTable.scala +++ b/src/reflect/scala/reflect/internal/SymbolTable.scala @@ -141,10 +141,6 @@ 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" -- cgit v1.2.3