Skip to content

Commit

Permalink
Make fresh name generation per file instead of global
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
smarter committed Dec 2, 2019
1 parent a64725c commit e07a728
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 29 deletions.
8 changes: 7 additions & 1 deletion compiler/src/dotty/tools/dotc/CompilationUnit.scala
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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.
*/
Expand Down
16 changes: 9 additions & 7 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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 */
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Comments.scala
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ object Comments {
val tree = new Parser(SourceFile.virtual("<usecase>", 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))
Expand Down
19 changes: 8 additions & 11 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/NameKinds.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/dotty/tools/DottyTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
40 changes: 40 additions & 0 deletions compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
}
}
}
10 changes: 6 additions & 4 deletions staging/src/scala/quoted/staging/QuoteCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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("<quote>")

// Places the contents of expr in a compilable tree for a class with the following format.
// `package __root__ { class ' { def apply: Any = <expr> } }`
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) =>
Expand Down

0 comments on commit e07a728

Please sign in to comment.