Skip to content

Commit 69d60cb

Browse files
committed
Stable names for lambda lifted method and fresh names
Fresh names are created using a FreshNameCreator, which appends an increasing number to the given prefix. ``` scala> val fresh = new scala.reflect.internal.util.FreshNameCreator() fresh: scala.reflect.internal.util.FreshNameCreator = scala.reflect.internal.util.FreshNameCreator@42b84286 scala> List("foo$", "bar$", "foo$").map(fresh.newName(_)) res1: List[String] = List(foo$1, bar$1, foo$2) ``` Each compilation unit had its own fresh name creator, which is used in the regular compiler. Macros and quasiquotes make use of a global creator (at least, as of #3401). Both of these are too broadly scoped to help achieve deterministic fresh names: if sources are recompiled in a different order or separately recompiled, the fresh name counters can be different. Methods in a given compilation unit are not necessarily typechecked in a linear fashion; they might be typechecked ahead of time to provide an inferred type to a caller. This commit: - Changes all known fresh name creations within the typer phase (in which out-of-order typechecking is a factor) to use a fineer grained fresh name creator. How fine grained? A fresh name generated as some position `p` shares the fresh name generator scoped at the closest method or class that encloses that the outermost enclosing tree at the same position. This definition is designed to give a shared fresh name creator for all fresh names generated in `macro1(macro2())`, even if the fresh names are requiested from with a Typer in the macro enclosed by a synthetic method. - Changes macro fresh names to use the same fresh naming scheme as the regular typechecker. An opt-out compiler option allows the old behaviour, but I'm interested to find real-world cases where the new scheme actually causes a problem In addition, a small change is made to lambda lift to lift local methods in the order that they are encountered during traversal, rather than sorting them based on `Symbol.isLess` (which include `Symbol.id`, an order-of-typechecking dependent value).
1 parent c6ab51c commit 69d60cb

31 files changed

+375
-116
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ trait Names {
66

77
import global._
88

9-
def freshNameCreator = globalFreshNameCreator
9+
def freshNameCreator = self.callsiteTyper.fresh
1010

1111
def fresh(): String =
1212
freshName()

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package scala.reflect.macros
22
package contexts
33

4+
import scala.reflect.internal.util.FreshNameCreator
45
import scala.tools.nsc.reporters.StoreReporter
56

67
trait Parsers {
@@ -12,7 +13,9 @@ trait Parsers {
1213
val oldReporter = global.reporter
1314
try {
1415
global.reporter = sreporter
15-
val parser = newUnitParser(new CompilationUnit(newSourceFile(code, "<macro>")))
16+
val parser = newUnitParser(new CompilationUnit(newSourceFile(code, "<macro>")) {
17+
override implicit val fresh: FreshNameCreator = currentFreshNameCreator
18+
})
1619
val tree = gen.mkTreeOrBlock(parser.parseStatsOrPackages())
1720
sreporter.infos.foreach {
1821
case sreporter.Info(pos, msg, sreporter.ERROR) => throw ParseException(pos, msg)

src/compiler/scala/reflect/reify/utils/Extractors.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ trait Extractors {
7272
}
7373
val tpec = ClassDef(
7474
Modifiers(FINAL),
75-
newTypeName(global.currentUnit.fresh.newName(flavor.toString)),
75+
newTypeName(currentFreshNameCreator.newName(flavor.toString)),
7676
List(),
7777
Template(List(Ident(reifierBase)),
7878
noSelfType,

src/compiler/scala/reflect/reify/utils/SymbolTables.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ trait SymbolTables {
7777
var name = name0.toString
7878
name = name.replace(".type", "$type")
7979
name = name.replace(" ", "$")
80-
val fresh = typer.context.unit.fresh
80+
val fresh = typer.fresh
8181
newTermName(fresh.newName(name))
8282
}
8383
val bindingAttachment = reification.attachments.get[ReifyBindingAttachment].get

src/compiler/scala/tools/nsc/Global.scala

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@ package nsc
1010
import java.io.{File, FileNotFoundException, IOException}
1111
import java.net.URL
1212
import java.nio.charset.{Charset, CharsetDecoder, IllegalCharsetNameException, UnsupportedCharsetException}
13+
1314
import scala.collection.{immutable, mutable}
1415
import io.{AbstractFile, Path, SourceReader}
1516
import reporters.Reporter
1617
import util.{ClassPath, returning}
1718
import scala.reflect.ClassTag
18-
import scala.reflect.internal.util.{BatchSourceFile, NoSourceFile, ScalaClassLoader, ScriptSourceFile, SourceFile, StatisticsStatics}
19+
import scala.reflect.internal.util.{BatchSourceFile, FreshNameCreator, NoSourceFile, ScalaClassLoader, ScriptSourceFile, SourceFile, StatisticsStatics}
1920
import scala.reflect.internal.pickling.PickleBuffer
2021
import symtab.{Flags, SymbolTable, SymbolTrackers}
2122
import symtab.classfile.Pickler
@@ -26,7 +27,7 @@ import typechecker._
2627
import transform.patmat.PatternMatching
2728
import transform._
2829
import backend.{JavaPlatform, ScalaPrimitives}
29-
import backend.jvm.{GenBCode, BackendStats}
30+
import backend.jvm.{BackendStats, GenBCode}
3031
import scala.concurrent.Future
3132
import scala.language.postfixOps
3233
import scala.tools.nsc.ast.{TreeGen => AstTreeGen}
@@ -965,7 +966,9 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
965966
def currentRun: Run = curRun
966967
def currentUnit: CompilationUnit = if (currentRun eq null) NoCompilationUnit else currentRun.currentUnit
967968
def currentSource: SourceFile = if (currentUnit.exists) currentUnit.source else lastSeenSourceFile
968-
def currentFreshNameCreator = currentUnit.fresh
969+
def currentFreshNameCreator = if (curFreshNameCreator == null) currentUnit.fresh else curFreshNameCreator
970+
private[this] var curFreshNameCreator: FreshNameCreator = null
971+
private[scala] def currentFreshNameCreator_=(fresh: FreshNameCreator): Unit = curFreshNameCreator = fresh
969972

970973
def isGlobalInitialized = (
971974
definitions.isDefinitionsInitialized

src/compiler/scala/tools/nsc/ast/TreeGen.scala

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package ast
99
import scala.collection.mutable.ListBuffer
1010
import symtab.Flags._
1111
import scala.language.postfixOps
12+
import scala.reflect.internal.util.FreshNameCreator
1213

1314
/** XXX to resolve: TreeGen only assumes global is a SymbolTable, but
1415
* TreeDSL at the moment expects a Global. Can we get by with SymbolTable?
@@ -191,20 +192,24 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
191192

192193
/** Used in situations where you need to access value of an expression several times
193194
*/
194-
def evalOnce(expr: Tree, owner: Symbol, unit: CompilationUnit)(within: (() => Tree) => Tree): Tree = {
195+
def evalOnce(expr: Tree, owner: Symbol, unit: CompilationUnit)(within: (() => Tree) => Tree): Tree = evalOnce(expr, owner, unit.fresh)(within)
196+
def evalOnce(expr: Tree, owner: Symbol, fresh: FreshNameCreator)(within: (() => Tree) => Tree): Tree = {
195197
var used = false
196198
if (treeInfo.isExprSafeToInline(expr)) {
197199
within(() => if (used) expr.duplicate else { used = true; expr })
198200
}
199201
else {
200-
val (valDef, identFn) = mkPackedValDef(expr, owner, unit.freshTermName("ev$"))
202+
val (valDef, identFn) = mkPackedValDef(expr, owner, freshTermName("ev$")(fresh))
201203
val containing = within(identFn)
202204
ensureNonOverlapping(containing, List(expr))
203205
Block(List(valDef), containing) setPos (containing.pos union expr.pos)
204206
}
205207
}
206208

207-
def evalOnceAll(exprs: List[Tree], owner: Symbol, unit: CompilationUnit)(within: (List[() => Tree]) => Tree): Tree = {
209+
def evalOnceAll(exprs: List[Tree], owner: Symbol, unit: CompilationUnit)(within: (List[() => Tree]) => Tree): Tree =
210+
evalOnceAll(exprs, owner, unit.fresh)(within)
211+
212+
def evalOnceAll(exprs: List[Tree], owner: Symbol, fresh: FreshNameCreator)(within: (List[() => Tree]) => Tree): Tree = {
208213
val vdefs = new ListBuffer[ValDef]
209214
val exprs1 = new ListBuffer[() => Tree]
210215
val used = new Array[Boolean](exprs.length)
@@ -217,7 +222,7 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
217222
}
218223
}
219224
else {
220-
val (valDef, identFn) = mkPackedValDef(expr, owner, unit.freshTermName("ev$"))
225+
val (valDef, identFn) = mkPackedValDef(expr, owner, freshTermName("ev$")(fresh))
221226
vdefs += valDef
222227
exprs1 += identFn
223228
}

src/compiler/scala/tools/nsc/settings/ScalaSettings.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ trait ScalaSettings extends AbsScalaSettings
218218
val Yreifycopypaste = BooleanSetting ("-Yreify-copypaste", "Dump the reified trees in copypasteable representation.")
219219
val Ymacroexpand = ChoiceSetting ("-Ymacro-expand", "policy", "Control expansion of macros, useful for scaladoc and presentation compiler.", List(MacroExpand.Normal, MacroExpand.None, MacroExpand.Discard), MacroExpand.Normal)
220220
val Ymacronoexpand = BooleanSetting ("-Ymacro-no-expand", "Don't expand macros. Might be useful for scaladoc and presentation compiler, but will crash anything which uses macros and gets past typer.") withDeprecationMessage(s"Use ${Ymacroexpand.name}:${MacroExpand.None}") withPostSetHook(_ => Ymacroexpand.value = MacroExpand.None)
221+
val YmacroFresh = BooleanSetting ("-Ymacro-global-fresh-names", "Should fresh names in macros be unique across all compilation units")
221222
val YmacroAnnotations = BooleanSetting ("-Ymacro-annotations", "Enable support for macro annotations, formerly in macro paradise.")
222223
val Yreplclassbased = BooleanSetting ("-Yrepl-class-based", "Use classes to wrap REPL snippets instead of objects")
223224
val Yreploutdir = StringSetting ("-Yrepl-outdir", "path", "Write repl-generated classfiles to given output directory (use \"\" to generate a temporary dir)" , "")

src/compiler/scala/tools/nsc/transform/CleanUp.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL {
186186
val runDefinitions = currentRun.runDefinitions
187187
import runDefinitions._
188188

189-
gen.evalOnce(qual, currentOwner, unit) { qual1 =>
189+
gen.evalOnce(qual, currentOwner, localTyper.fresh) { qual1 =>
190190
/* Some info about the type of the method being called. */
191191
val methSym = ad.symbol
192192
val boxedResType = toBoxedType(resType) // Int -> Integer

src/compiler/scala/tools/nsc/transform/Erasure.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ abstract class Erasure extends InfoTransform
682682
else {
683683
val untyped =
684684
// util.trace("new asinstanceof test") {
685-
gen.evalOnce(qual1, context.owner, context.unit) { qual =>
685+
gen.evalOnce(qual1, context.owner, fresh) { qual =>
686686
If(Apply(Select(qual(), nme.eq), List(Literal(Constant(null)) setType NullTpe)),
687687
Literal(Constant(null)) setType targ.tpe,
688688
unbox(qual(), targ.tpe))
@@ -1044,7 +1044,7 @@ abstract class Erasure extends InfoTransform
10441044
case SingletonInstanceCheck(cmpOp, cmpArg) =>
10451045
atPos(tree.pos) { Apply(Select(cmpArg, cmpOp), List(qual)) }
10461046
case RefinedType(parents, decls) if (parents.length >= 2) =>
1047-
gen.evalOnce(qual, currentOwner, unit) { q =>
1047+
gen.evalOnce(qual, currentOwner, localTyper.fresh) { q =>
10481048
// Optimization: don't generate isInstanceOf tests if the static type
10491049
// conforms, because it always succeeds. (Or at least it had better.)
10501050
// At this writing the pattern matcher generates some instance tests
@@ -1097,7 +1097,7 @@ abstract class Erasure extends InfoTransform
10971097

10981098
global.typer.typedPos(tree.pos) {
10991099
if (level == 1) isArrayTest(qual)
1100-
else gen.evalOnce(qual, currentOwner, unit) { qual1 =>
1100+
else gen.evalOnce(qual, currentOwner, localTyper.fresh) { qual1 =>
11011101
gen.mkAnd(
11021102
gen.mkMethodCall(
11031103
qual1(),

src/compiler/scala/tools/nsc/transform/LambdaLift.scala

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ package transform
99
import symtab._
1010
import Flags._
1111
import scala.collection.mutable
12-
import scala.collection.mutable.{ LinkedHashMap, LinkedHashSet, TreeSet }
12+
import scala.collection.mutable.{ LinkedHashMap, LinkedHashSet }
1313

1414
abstract class LambdaLift extends InfoTransform {
1515
import global._
@@ -50,7 +50,7 @@ abstract class LambdaLift extends InfoTransform {
5050

5151
class LambdaLifter(unit: CompilationUnit) extends explicitOuter.OuterPathTransformer(unit) {
5252

53-
private type SymSet = TreeSet[Symbol]
53+
private type SymSet = LinkedHashSet[Symbol]
5454

5555
/** A map storing free variables of functions and classes */
5656
private val free = new LinkedHashMap[Symbol, SymSet]
@@ -64,8 +64,7 @@ abstract class LambdaLift extends InfoTransform {
6464
/** Symbols that are called from an inner class. */
6565
private val calledFromInner = new LinkedHashSet[Symbol]
6666

67-
private val ord = Ordering.fromLessThan[Symbol](_ isLess _)
68-
private def newSymSet = TreeSet.empty[Symbol](ord)
67+
private def newSymSet: LinkedHashSet[Symbol] = new LinkedHashSet[Symbol]
6968

7069
private def symSet(f: LinkedHashMap[Symbol, SymSet], sym: Symbol): SymSet =
7170
f.getOrElseUpdate(sym, newSymSet)

0 commit comments

Comments
 (0)