From e07a72888d11d824a9076ba5c025e033c250c7fb Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 26 Nov 2019 18:24:02 +0100 Subject: [PATCH] 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). --- .../dotty/tools/dotc/CompilationUnit.scala | 8 +++- compiler/src/dotty/tools/dotc/Run.scala | 16 ++++---- .../src/dotty/tools/dotc/core/Comments.scala | 2 +- .../src/dotty/tools/dotc/core/Contexts.scala | 19 ++++----- .../src/dotty/tools/dotc/core/NameKinds.scala | 2 +- compiler/test/dotty/tools/DottyTest.scala | 2 +- .../tools/backend/jvm/DottyBytecodeTest.scala | 6 +-- .../backend/jvm/DottyBytecodeTests.scala | 40 +++++++++++++++++++ .../scala/quoted/staging/QuoteCompiler.scala | 10 +++-- 9 files changed, 76 insertions(+), 29 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/CompilationUnit.scala b/compiler/src/dotty/tools/dotc/CompilationUnit.scala index 0682d171b1b2..527770b1731a 100644 --- a/compiler/src/dotty/tools/dotc/CompilationUnit.scala +++ b/compiler/src/dotty/tools/dotc/CompilationUnit.scala @@ -1,7 +1,7 @@ package dotty.tools package dotc -import util.SourceFile +import util.{FreshNameCreator, SourceFile} import ast.{tpd, untpd} import tpd.{Tree, TreeTraverser} import typer.PrepareInlineable.InlineAccessors @@ -27,6 +27,12 @@ class CompilationUnit protected (val source: SourceFile) { /** Pickled TASTY binaries, indexed by class. */ var pickled: Map[ClassSymbol, Array[Byte]] = Map() + /** The fresh name creator for the current unit. + * FIXME(#7661): This is not fine-grained enough to enable reproducible builds, + * see https://github.com/scala/scala/commit/f50ec3c866263448d803139e119b33afb04ec2bc + */ + val freshNames: FreshNameCreator = new FreshNameCreator.Default + /** Will be set to `true` if contains `Quote`. * The information is used in phase `Staging` in order to avoid traversing trees that need no transformations. */ diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index 954daf240e65..729baec2476f 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -52,7 +52,6 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint .setTyper(new Typer) .addMode(Mode.ImplicitsEnabled) .setTyperState(new TyperState(ctx.typerState)) - .setFreshNames(new FreshNameCreator.Default) ctx.initialize()(start) // re-initialize the base context with start def addImport(ctx: Context, rootRef: ImportInfo.RootRef) = ctx.fresh.setImportInfo(ImportInfo.rootImport(rootRef)(ctx)) @@ -241,12 +240,15 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint } } - def compileFromString(sourceCode: String): Unit = { - val virtualFile = new VirtualFile("compileFromString-${java.util.UUID.randomUUID().toString}") - val writer = new BufferedWriter(new OutputStreamWriter(virtualFile.output, "UTF-8")) // buffering is still advised by javadoc - writer.write(sourceCode) - writer.close() - compileSources(List(new SourceFile(virtualFile, Codec.UTF8))) + def compileFromStrings(sourceCodes: String*): Unit = { + val sourceFiles = sourceCodes.map {sourceCode => + val virtualFile = new VirtualFile("compileFromString-${java.util.UUID.randomUUID().toString}") + val writer = new BufferedWriter(new OutputStreamWriter(virtualFile.output, "UTF-8")) // buffering is still advised by javadoc + writer.write(sourceCode) + writer.close() + new SourceFile(virtualFile, Codec.UTF8) + } + compileSources(sourceFiles.toList) } /** Print summary; return # of errors encountered */ diff --git a/compiler/src/dotty/tools/dotc/core/Comments.scala b/compiler/src/dotty/tools/dotc/core/Comments.scala index a9512dcf381a..bba3f9aef001 100644 --- a/compiler/src/dotty/tools/dotc/core/Comments.scala +++ b/compiler/src/dotty/tools/dotc/core/Comments.scala @@ -121,7 +121,7 @@ object Comments { val tree = new Parser(SourceFile.virtual("", code)).localDef(codePos.start) tree match { case tree: untpd.DefDef => - val newName = ctx.freshNames.newName(tree.name, NameKinds.DocArtifactName) + val newName = ctx.compilationUnit.freshNames.newName(tree.name, NameKinds.DocArtifactName) untpd.cpy.DefDef(tree)(name = newName) case _ => ctx.error(ProperDefinitionNotFound(), ctx.source.atSpan(codePos)) diff --git a/compiler/src/dotty/tools/dotc/core/Contexts.scala b/compiler/src/dotty/tools/dotc/core/Contexts.scala index 342021713c94..3379ed2dc50c 100644 --- a/compiler/src/dotty/tools/dotc/core/Contexts.scala +++ b/compiler/src/dotty/tools/dotc/core/Contexts.scala @@ -14,7 +14,7 @@ import Uniques._ import ast.Trees._ import ast.untpd import Flags.GivenOrImplicit -import util.{FreshNameCreator, NoSource, SimpleIdentityMap, SourceFile} +import util.{NoSource, SimpleIdentityMap, SourceFile} import typer.{Implicits, ImportInfo, Inliner, NamerContextOps, SearchHistory, SearchRoot, TypeAssigner, Typer, Nullables} import Nullables.{NotNullInfo, given} import Implicits.ContextualImplicits @@ -44,12 +44,11 @@ object Contexts { private val (sbtCallbackLoc, store2) = store1.newLocation[AnalysisCallback]() private val (printerFnLoc, store3) = store2.newLocation[Context => Printer](new RefinedPrinter(_)) private val (settingsStateLoc, store4) = store3.newLocation[SettingsState]() - private val (freshNamesLoc, store5) = store4.newLocation[FreshNameCreator](new FreshNameCreator.Default) - private val (compilationUnitLoc, store6) = store5.newLocation[CompilationUnit]() - private val (runLoc, store7) = store6.newLocation[Run]() - private val (profilerLoc, store8) = store7.newLocation[Profiler]() - private val (notNullInfosLoc, store9) = store8.newLocation[List[NotNullInfo]]() - private val initialStore = store9 + private val (compilationUnitLoc, store5) = store4.newLocation[CompilationUnit]() + private val (runLoc, store6) = store5.newLocation[Run]() + private val (profilerLoc, store7) = store6.newLocation[Profiler]() + private val (notNullInfosLoc, store8) = store7.newLocation[List[NotNullInfo]]() + private val initialStore = store8 /** The current context */ def curCtx(given ctx: Context): Context = ctx @@ -200,9 +199,6 @@ object Contexts { /** The current settings values */ def settingsState: SettingsState = store(settingsStateLoc) - /** The current fresh name creator */ - def freshNames: FreshNameCreator = store(freshNamesLoc) - /** The current compilation unit */ def compilationUnit: CompilationUnit = store(compilationUnitLoc) @@ -471,6 +467,8 @@ object Contexts { if (prev != null) prev else { val newCtx = fresh.setSource(source) + if (newCtx.compilationUnit == null) + newCtx.setCompilationUnit(CompilationUnit(source)) sourceCtx = sourceCtx.updated(source, newCtx) newCtx } @@ -563,7 +561,6 @@ object Contexts { def setSettings(settingsState: SettingsState): this.type = updateStore(settingsStateLoc, settingsState) def setRun(run: Run): this.type = updateStore(runLoc, run) def setProfiler(profiler: Profiler): this.type = updateStore(profilerLoc, profiler) - def setFreshNames(freshNames: FreshNameCreator): this.type = updateStore(freshNamesLoc, freshNames) def setNotNullInfos(notNullInfos: List[NotNullInfo]): this.type = updateStore(notNullInfosLoc, notNullInfos) def setProperty[T](key: Key[T], value: T): this.type = diff --git a/compiler/src/dotty/tools/dotc/core/NameKinds.scala b/compiler/src/dotty/tools/dotc/core/NameKinds.scala index af173b1a9bd5..d5de95813a6c 100644 --- a/compiler/src/dotty/tools/dotc/core/NameKinds.scala +++ b/compiler/src/dotty/tools/dotc/core/NameKinds.scala @@ -215,7 +215,7 @@ object NameKinds { /** Generate fresh unique term name of this kind with given prefix name */ def fresh(prefix: TermName = EmptyTermName)(implicit ctx: Context): TermName = - ctx.freshNames.newName(prefix, this) + ctx.compilationUnit.freshNames.newName(prefix, this) /** Generate fresh unique type name of this kind with given prefix name */ def fresh(prefix: TypeName)(implicit ctx: Context): TypeName = diff --git a/compiler/test/dotty/tools/DottyTest.scala b/compiler/test/dotty/tools/DottyTest.scala index fe0ef7353c8c..1e6f565d6f5b 100644 --- a/compiler/test/dotty/tools/DottyTest.scala +++ b/compiler/test/dotty/tools/DottyTest.scala @@ -64,7 +64,7 @@ trait DottyTest extends ContextEscapeDetection { def checkCompile(checkAfterPhase: String, source: String)(assertion: (tpd.Tree, Context) => Unit): Context = { val c = compilerWithChecker(checkAfterPhase)(assertion) val run = c.newRun - run.compileFromString(source) + run.compileFromStrings(source) run.runContext } diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala index 5d5626888183..807a56fe93b2 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala @@ -52,13 +52,13 @@ trait DottyBytecodeTest { ctx0.setSetting(ctx0.settings.outputDir, outputDir) } - /** Checks source code from raw string */ - def checkBCode(source: String)(checkOutput: AbstractFile => Unit): Unit = { + /** Checks source code from raw strings */ + def checkBCode(sources: String*)(checkOutput: AbstractFile => Unit): Unit = { implicit val ctx: Context = initCtx val compiler = new Compiler val run = compiler.newRun - compiler.newRun.compileFromString(source) + compiler.newRun.compileFromStrings(sources: _*) checkOutput(ctx.settings.outputDir.value) } diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index 2f170d3876e7..341906e72034 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -3,6 +3,10 @@ package dotty.tools.backend.jvm import org.junit.Assert._ import org.junit.Test +import scala.tools.asm +import asm._ +import asm.tree._ + import scala.tools.asm.Opcodes class TestBCode extends DottyBytecodeTest { @@ -743,4 +747,40 @@ class TestBCode extends DottyBytecodeTest { } } } + + @Test def freshNames = { + val sourceA = + """|class A { + | def a1[T: Ordering]: Unit = {} + | def a2[T: Ordering]: Unit = {} + |} + """.stripMargin + val sourceB = + """|class B { + | def b1[T: Ordering]: Unit = {} + | def b2[T: Ordering]: Unit = {} + |} + """.stripMargin + + checkBCode(sourceA, sourceB) { dir => + val clsNodeA = loadClassNode(dir.lookupName("A.class", directory = false).input, skipDebugInfo = false) + val clsNodeB = loadClassNode(dir.lookupName("B.class", directory = false).input, skipDebugInfo = false) + val a1 = getMethod(clsNodeA, "a1") + val a2 = getMethod(clsNodeA, "a2") + val b1 = getMethod(clsNodeB, "b1") + val b2 = getMethod(clsNodeB, "b2") + + def assertParamName(mn: MethodNode, expectedName: String) = { + val actualName = mn.localVariables.get(1).name + assert(actualName == expectedName, + s"Method ${mn.name} has parameter $actualName but expected $expectedName") + } + + // The fresh name counter should be reset for every compilation unit + assertParamName(a1, "evidence$1") + assertParamName(a2, "evidence$2") + assertParamName(b1, "evidence$1") + assertParamName(b2, "evidence$2") + } + } } diff --git a/staging/src/scala/quoted/staging/QuoteCompiler.scala b/staging/src/scala/quoted/staging/QuoteCompiler.scala index 620f924ae0ea..c886e878002c 100644 --- a/staging/src/scala/quoted/staging/QuoteCompiler.scala +++ b/staging/src/scala/quoted/staging/QuoteCompiler.scala @@ -55,18 +55,20 @@ private class QuoteCompiler extends Compiler { override def runOn(units: List[CompilationUnit])(implicit ctx: Context): List[CompilationUnit] = units.flatMap { case exprUnit: ExprCompilationUnit => + implicit val unitCtx: Context = ctx.fresh.setPhase(this.start).setCompilationUnit(exprUnit) + val pos = Span(0) val assocFile = new VirtualFile("") // Places the contents of expr in a compilable tree for a class with the following format. // `package __root__ { class ' { def apply: Any = } }` - val cls = ctx.newCompleteClassSymbol(defn.RootClass, outputClassName, EmptyFlags, + val cls = unitCtx.newCompleteClassSymbol(defn.RootClass, outputClassName, EmptyFlags, defn.ObjectType :: Nil, newScope, coord = pos, assocFile = assocFile).entered.asClass - cls.enter(ctx.newDefaultConstructor(cls), EmptyScope) - val meth = ctx.newSymbol(cls, nme.apply, Method, ExprType(defn.AnyType), coord = pos).entered + cls.enter(unitCtx.newDefaultConstructor(cls), EmptyScope) + val meth = unitCtx.newSymbol(cls, nme.apply, Method, ExprType(defn.AnyType), coord = pos).entered val qctx = dotty.tools.dotc.quoted.QuoteContext() - val quoted = PickledQuotes.quotedExprToTree(exprUnit.exprBuilder.apply(qctx))(ctx.withOwner(meth)) + val quoted = PickledQuotes.quotedExprToTree(exprUnit.exprBuilder.apply(qctx))(unitCtx.withOwner(meth)) getLiteral(quoted) match { case Some(value) =>