Skip to content

Commit 5a04232

Browse files
committed
Make fresh name generation per file instead of global
This makes the compiler output more stable and avoids overcompilation during incremental compilation due to context bounds parameters changing name. This is still not fine-grained enough as noted in the documentation of CompilationUnit#freshNames. To work properly, this requires us to always have a compilation unit set in the context before compiling code, which is a good idea anyway (would be nice to improve the API to make this harder to screw up).
1 parent a64725c commit 5a04232

File tree

9 files changed

+76
-29
lines changed

9 files changed

+76
-29
lines changed

compiler/src/dotty/tools/dotc/CompilationUnit.scala

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package dotty.tools
22
package dotc
33

4-
import util.SourceFile
4+
import util.{FreshNameCreator, SourceFile}
55
import ast.{tpd, untpd}
66
import tpd.{Tree, TreeTraverser}
77
import typer.PrepareInlineable.InlineAccessors
@@ -27,6 +27,12 @@ class CompilationUnit protected (val source: SourceFile) {
2727
/** Pickled TASTY binaries, indexed by class. */
2828
var pickled: Map[ClassSymbol, Array[Byte]] = Map()
2929

30+
/** The fresh name creator for the current unit.
31+
* FIXME: This is not fine-grained enough to enable reproducible builds,
32+
* see https://github.com/scala/scala/commit/f50ec3c866263448d803139e119b33afb04ec2bc
33+
*/
34+
val freshNames: FreshNameCreator = new FreshNameCreator.Default
35+
3036
/** Will be set to `true` if contains `Quote`.
3137
* The information is used in phase `Staging` in order to avoid traversing trees that need no transformations.
3238
*/

compiler/src/dotty/tools/dotc/Run.scala

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
5252
.setTyper(new Typer)
5353
.addMode(Mode.ImplicitsEnabled)
5454
.setTyperState(new TyperState(ctx.typerState))
55-
.setFreshNames(new FreshNameCreator.Default)
5655
ctx.initialize()(start) // re-initialize the base context with start
5756
def addImport(ctx: Context, rootRef: ImportInfo.RootRef) =
5857
ctx.fresh.setImportInfo(ImportInfo.rootImport(rootRef)(ctx))
@@ -241,12 +240,15 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
241240
}
242241
}
243242

244-
def compileFromString(sourceCode: String): Unit = {
245-
val virtualFile = new VirtualFile("compileFromString-${java.util.UUID.randomUUID().toString}")
246-
val writer = new BufferedWriter(new OutputStreamWriter(virtualFile.output, "UTF-8")) // buffering is still advised by javadoc
247-
writer.write(sourceCode)
248-
writer.close()
249-
compileSources(List(new SourceFile(virtualFile, Codec.UTF8)))
243+
def compileFromStrings(sourceCodes: String*): Unit = {
244+
val sourceFiles = sourceCodes.map {sourceCode =>
245+
val virtualFile = new VirtualFile("compileFromString-${java.util.UUID.randomUUID().toString}")
246+
val writer = new BufferedWriter(new OutputStreamWriter(virtualFile.output, "UTF-8")) // buffering is still advised by javadoc
247+
writer.write(sourceCode)
248+
writer.close()
249+
new SourceFile(virtualFile, Codec.UTF8)
250+
}
251+
compileSources(sourceFiles.toList)
250252
}
251253

252254
/** Print summary; return # of errors encountered */

compiler/src/dotty/tools/dotc/core/Comments.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ object Comments {
121121
val tree = new Parser(SourceFile.virtual("<usecase>", code)).localDef(codePos.start)
122122
tree match {
123123
case tree: untpd.DefDef =>
124-
val newName = ctx.freshNames.newName(tree.name, NameKinds.DocArtifactName)
124+
val newName = ctx.compilationUnit.freshNames.newName(tree.name, NameKinds.DocArtifactName)
125125
untpd.cpy.DefDef(tree)(name = newName)
126126
case _ =>
127127
ctx.error(ProperDefinitionNotFound(), ctx.source.atSpan(codePos))

compiler/src/dotty/tools/dotc/core/Contexts.scala

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import Uniques._
1414
import ast.Trees._
1515
import ast.untpd
1616
import Flags.GivenOrImplicit
17-
import util.{FreshNameCreator, NoSource, SimpleIdentityMap, SourceFile}
17+
import util.{NoSource, SimpleIdentityMap, SourceFile}
1818
import typer.{Implicits, ImportInfo, Inliner, NamerContextOps, SearchHistory, SearchRoot, TypeAssigner, Typer, Nullables}
1919
import Nullables.{NotNullInfo, given}
2020
import Implicits.ContextualImplicits
@@ -44,12 +44,11 @@ object Contexts {
4444
private val (sbtCallbackLoc, store2) = store1.newLocation[AnalysisCallback]()
4545
private val (printerFnLoc, store3) = store2.newLocation[Context => Printer](new RefinedPrinter(_))
4646
private val (settingsStateLoc, store4) = store3.newLocation[SettingsState]()
47-
private val (freshNamesLoc, store5) = store4.newLocation[FreshNameCreator](new FreshNameCreator.Default)
48-
private val (compilationUnitLoc, store6) = store5.newLocation[CompilationUnit]()
49-
private val (runLoc, store7) = store6.newLocation[Run]()
50-
private val (profilerLoc, store8) = store7.newLocation[Profiler]()
51-
private val (notNullInfosLoc, store9) = store8.newLocation[List[NotNullInfo]]()
52-
private val initialStore = store9
47+
private val (compilationUnitLoc, store5) = store4.newLocation[CompilationUnit]()
48+
private val (runLoc, store6) = store5.newLocation[Run]()
49+
private val (profilerLoc, store7) = store6.newLocation[Profiler]()
50+
private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]()
51+
private val initialStore = store8
5352

5453
/** The current context */
5554
def curCtx(given ctx: Context): Context = ctx
@@ -200,9 +199,6 @@ object Contexts {
200199
/** The current settings values */
201200
def settingsState: SettingsState = store(settingsStateLoc)
202201

203-
/** The current fresh name creator */
204-
def freshNames: FreshNameCreator = store(freshNamesLoc)
205-
206202
/** The current compilation unit */
207203
def compilationUnit: CompilationUnit = store(compilationUnitLoc)
208204

@@ -471,6 +467,8 @@ object Contexts {
471467
if (prev != null) prev
472468
else {
473469
val newCtx = fresh.setSource(source)
470+
if (newCtx.compilationUnit == null)
471+
newCtx.setCompilationUnit(CompilationUnit(source))
474472
sourceCtx = sourceCtx.updated(source, newCtx)
475473
newCtx
476474
}
@@ -563,7 +561,6 @@ object Contexts {
563561
def setSettings(settingsState: SettingsState): this.type = updateStore(settingsStateLoc, settingsState)
564562
def setRun(run: Run): this.type = updateStore(runLoc, run)
565563
def setProfiler(profiler: Profiler): this.type = updateStore(profilerLoc, profiler)
566-
def setFreshNames(freshNames: FreshNameCreator): this.type = updateStore(freshNamesLoc, freshNames)
567564
def setNotNullInfos(notNullInfos: List[NotNullInfo]): this.type = updateStore(notNullInfosLoc, notNullInfos)
568565

569566
def setProperty[T](key: Key[T], value: T): this.type =

compiler/src/dotty/tools/dotc/core/NameKinds.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ object NameKinds {
215215

216216
/** Generate fresh unique term name of this kind with given prefix name */
217217
def fresh(prefix: TermName = EmptyTermName)(implicit ctx: Context): TermName =
218-
ctx.freshNames.newName(prefix, this)
218+
ctx.compilationUnit.freshNames.newName(prefix, this)
219219

220220
/** Generate fresh unique type name of this kind with given prefix name */
221221
def fresh(prefix: TypeName)(implicit ctx: Context): TypeName =

compiler/test/dotty/tools/DottyTest.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ trait DottyTest extends ContextEscapeDetection {
6464
def checkCompile(checkAfterPhase: String, source: String)(assertion: (tpd.Tree, Context) => Unit): Context = {
6565
val c = compilerWithChecker(checkAfterPhase)(assertion)
6666
val run = c.newRun
67-
run.compileFromString(source)
67+
run.compileFromStrings(source)
6868
run.runContext
6969
}
7070

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,13 @@ trait DottyBytecodeTest {
5252
ctx0.setSetting(ctx0.settings.outputDir, outputDir)
5353
}
5454

55-
/** Checks source code from raw string */
56-
def checkBCode(source: String)(checkOutput: AbstractFile => Unit): Unit = {
55+
/** Checks source code from raw strings */
56+
def checkBCode(sources: String*)(checkOutput: AbstractFile => Unit): Unit = {
5757
implicit val ctx: Context = initCtx
5858

5959
val compiler = new Compiler
6060
val run = compiler.newRun
61-
compiler.newRun.compileFromString(source)
61+
compiler.newRun.compileFromStrings(sources: _*)
6262

6363
checkOutput(ctx.settings.outputDir.value)
6464
}

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ package dotty.tools.backend.jvm
33
import org.junit.Assert._
44
import org.junit.Test
55

6+
import scala.tools.asm
7+
import asm._
8+
import asm.tree._
9+
610
import scala.tools.asm.Opcodes
711

812
class TestBCode extends DottyBytecodeTest {
@@ -743,4 +747,40 @@ class TestBCode extends DottyBytecodeTest {
743747
}
744748
}
745749
}
750+
751+
@Test def freshNames = {
752+
val sourceA =
753+
"""|class A {
754+
| def a1[T: Ordering]: Unit = {}
755+
| def a2[T: Ordering]: Unit = {}
756+
|}
757+
""".stripMargin
758+
val sourceB =
759+
"""|class B {
760+
| def b1[T: Ordering]: Unit = {}
761+
| def b2[T: Ordering]: Unit = {}
762+
|}
763+
""".stripMargin
764+
765+
checkBCode(sourceA, sourceB) { dir =>
766+
val clsNodeA = loadClassNode(dir.lookupName("A.class", directory = false).input, skipDebugInfo = false)
767+
val clsNodeB = loadClassNode(dir.lookupName("B.class", directory = false).input, skipDebugInfo = false)
768+
val a1 = getMethod(clsNodeA, "a1")
769+
val a2 = getMethod(clsNodeA, "a2")
770+
val b1 = getMethod(clsNodeB, "b1")
771+
val b2 = getMethod(clsNodeB, "b2")
772+
773+
def assertParamName(mn: MethodNode, expectedName: String) = {
774+
val actualName = mn.localVariables.get(1).name
775+
assert(actualName == expectedName,
776+
s"Method ${mn.name} has parameter $actualName but expected $expectedName")
777+
}
778+
779+
// The fresh name counter should be reset for every compilation unit
780+
assertParamName(a1, "evidence$1")
781+
assertParamName(a2, "evidence$2")
782+
assertParamName(b1, "evidence$1")
783+
assertParamName(b2, "evidence$2")
784+
}
785+
}
746786
}

staging/src/scala/quoted/staging/QuoteCompiler.scala

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,20 @@ private class QuoteCompiler extends Compiler {
5555
override def runOn(units: List[CompilationUnit])(implicit ctx: Context): List[CompilationUnit] =
5656
units.flatMap {
5757
case exprUnit: ExprCompilationUnit =>
58+
implicit val unitCtx: Context = ctx.fresh.setPhase(this.start).setCompilationUnit(exprUnit)
59+
5860
val pos = Span(0)
5961
val assocFile = new VirtualFile("<quote>")
6062

6163
// Places the contents of expr in a compilable tree for a class with the following format.
6264
// `package __root__ { class ' { def apply: Any = <expr> } }`
63-
val cls = ctx.newCompleteClassSymbol(defn.RootClass, outputClassName, EmptyFlags,
65+
val cls = unitCtx.newCompleteClassSymbol(defn.RootClass, outputClassName, EmptyFlags,
6466
defn.ObjectType :: Nil, newScope, coord = pos, assocFile = assocFile).entered.asClass
65-
cls.enter(ctx.newDefaultConstructor(cls), EmptyScope)
66-
val meth = ctx.newSymbol(cls, nme.apply, Method, ExprType(defn.AnyType), coord = pos).entered
67+
cls.enter(unitCtx.newDefaultConstructor(cls), EmptyScope)
68+
val meth = unitCtx.newSymbol(cls, nme.apply, Method, ExprType(defn.AnyType), coord = pos).entered
6769

6870
val qctx = dotty.tools.dotc.quoted.QuoteContext()
69-
val quoted = PickledQuotes.quotedExprToTree(exprUnit.exprBuilder.apply(qctx))(ctx.withOwner(meth))
71+
val quoted = PickledQuotes.quotedExprToTree(exprUnit.exprBuilder.apply(qctx))(unitCtx.withOwner(meth))
7072

7173
getLiteral(quoted) match {
7274
case Some(value) =>

0 commit comments

Comments
 (0)