summaryrefslogtreecommitdiff
path: root/src/compiler/scala/tools/nsc
diff options
context:
space:
mode:
authorGrzegorz Kossakowski <grzegorz.kossakowski@gmail.com>2013-07-27 01:38:45 -0700
committerGrzegorz Kossakowski <grzegorz.kossakowski@gmail.com>2013-07-27 14:09:40 -0700
commitafbee09c8e0e7b1a4da1f8517c723dad9f1adb6f (patch)
tree9e0eba8ce1bff0af357aa4f84b3a905e00325bd9 /src/compiler/scala/tools/nsc
parent5eb4cdf1a7a466276a979d2952dbc30d416f423d (diff)
downloadscala-afbee09c8e0e7b1a4da1f8517c723dad9f1adb6f.tar.gz
scala-afbee09c8e0e7b1a4da1f8517c723dad9f1adb6f.tar.bz2
scala-afbee09c8e0e7b1a4da1f8517c723dad9f1adb6f.zip
Refactor the cake so SymbolTable does not depend on Global
This is rather large commit so I'll first explain the motivation behind it and then go through all changes in detail explaining the choices I made. The motivation behind this refactoring was to make SymbolTable unit testable. I wanted a lightweight way of initializing SymbolTable and then writing unit tests for subtyping algorithm, various functionality related to Symbols, etc. All of that should be possible by precisely controlling what we test, e.g., create types and symbols by hand and not have them defined in source code as we normally do in partest (functional) tests. The other motivation was to reduce and clarify dependencies we have in the compiler. Explicit dependencies lead to cleaner design. Also, explicit and reduces dependencies help incremental compilation which is a big problem for us in compiler's code base at the moment. One of the challenges I faced during that refactoring was cyclic dependency between Platform and SymbolLoaders. Platform depended on `SymbolLoaders.SymbolLoader` because it would define a root loader. SymbolLoaders depended on Platform for numerous reasons like deferring decision how to load a given symbol based on some Platform-specific hooks. I decided to break that cycle by removing methods related to symbol loading from Platform interface. One could argue, that better fix would be to make SymbolLoaders to not depend on Platform (backend) concept but that would be much bigger refactoring. Also, we have a new concept for dealing with symbol loading: Mirrors. For those reasons both `newClassLoader` and `rootLoader` were dropped from Platform interface. Note that JavaPlatform still depends on Global so it can access phases defined in Global to implement `platformPhases` method. Both GenICode and BCodeBodyBuilder have some Platform specific logic that requires casting because pattern matcher doesn't narrow types to give them a proper refinement. Check the changes for details. Some logging utilities has been moved from Global to SymbolTable because they are accessed by SymbolTable. Since Global inherits from SymbolTable this should be a source compatible change. The SymbolLoaders has dependency on `compileLate` method defined in Global. The purpose behind `compileLate` is not clear to me but the dependency looks a little bit dubious. At least we made that dependency explicit. ScaladocGlobal and Global defined in interactive has been adapted in a way that makes them compile both with quick.comp and 2.11.0-M4 so my refactorings are not blocking the modularization effort.
Diffstat (limited to 'src/compiler/scala/tools/nsc')
-rw-r--r--src/compiler/scala/tools/nsc/Global.scala27
-rw-r--r--src/compiler/scala/tools/nsc/backend/JavaPlatform.scala7
-rw-r--r--src/compiler/scala/tools/nsc/backend/Platform.scala10
-rw-r--r--src/compiler/scala/tools/nsc/backend/icode/GenICode.scala8
-rw-r--r--src/compiler/scala/tools/nsc/backend/icode/ICodes.scala4
-rw-r--r--src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala10
-rw-r--r--src/compiler/scala/tools/nsc/symtab/BrowsingLoaders.scala5
-rw-r--r--src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala41
-rw-r--r--src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala27
-rw-r--r--src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala2
10 files changed, 91 insertions, 50 deletions
diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index d1d5cd1f03..6417a54977 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -51,7 +51,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
class GlobalMirror extends Roots(NoSymbol) {
val universe: self.type = self
- def rootLoader: LazyType = platform.rootLoader
+ def rootLoader: LazyType = new loaders.PackageLoader(classPath)
override def toString = "compiler mirror"
}
@@ -80,10 +80,13 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
// platform specific elements
- type ThisPlatform = Platform { val global: Global.this.type }
+ protected class GlobalPlatform extends {
+ val global: Global.this.type = Global.this
+ val settings: Settings = Global.this.settings
+ } with JavaPlatform
- lazy val platform: ThisPlatform =
- new { val global: Global.this.type = Global.this } with JavaPlatform
+ type ThisPlatform = Platform { val symbolTable: Global.this.type }
+ lazy val platform: ThisPlatform = new GlobalPlatform
type PlatformClassPath = ClassPath[AbstractFile]
type OptClassPath = Option[PlatformClassPath]
@@ -265,12 +268,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
log("!!! " + msg) // such warnings always at least logged
}
- private def elapsedMessage(msg: String, start: Long) =
- msg + " in " + (currentTime - start) + "ms"
-
def informComplete(msg: String): Unit = reporter.withoutTruncating(inform(msg))
- def informProgress(msg: String) = if (settings.verbose) inform("[" + msg + "]")
- def informTime(msg: String, start: Long) = informProgress(elapsedMessage(msg, start))
def logError(msg: String, t: Throwable): Unit = ()
@@ -354,9 +352,11 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
getSourceFile(f)
}
- lazy val loaders = new SymbolLoaders {
- val global: Global.this.type = Global.this
- def lookupMemberAtTyperPhaseIfPossible(sym: Symbol, name: Name): Symbol = {
+ lazy val loaders = new {
+ val symbolTable: Global.this.type = Global.this
+ val platform: Global.this.platform.type = Global.this.platform
+ } with SymbolLoaders {
+ protected override def lookupMemberAtTyperPhaseIfPossible(sym: Symbol, name: Name): Symbol = {
def lookup = sym.info.member(name)
// if loading during initialization of `definitions` typerPhase is not yet set.
// in that case we simply load the member at the current phase
@@ -365,6 +365,9 @@ class Global(var currentSettings: Settings, var reporter: Reporter)
else
enteringTyper { lookup }
}
+ protected override def compileLate(srcfile: AbstractFile): Unit =
+ currentRun.compileLate(srcfile)
+
}
/** Returns the mirror that loaded given symbol */
diff --git a/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala b/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
index 780bc81bcc..32b5a98b98 100644
--- a/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
+++ b/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
@@ -11,6 +11,8 @@ import util.{ClassPath,MergedClassPath,DeltaClassPath}
import scala.tools.util.PathResolver
trait JavaPlatform extends Platform {
+ val global: Global
+ override val symbolTable: global.type = global
import global._
import definitions._
@@ -25,8 +27,6 @@ trait JavaPlatform extends Platform {
def updateClassPath(subst: Map[ClassPath[AbstractFile], ClassPath[AbstractFile]]) =
currentClassPath = Some(new DeltaClassPath(currentClassPath.get, subst))
- def rootLoader = new loaders.PackageLoader(classPath)
-
private def classEmitPhase =
if (settings.isBCodeActive) genBCode
else genASM
@@ -55,9 +55,6 @@ trait JavaPlatform extends Platform {
(sym isNonBottomSubClass BoxedBooleanClass)
}
- def newClassLoader(bin: AbstractFile): loaders.SymbolLoader =
- new loaders.ClassfileLoader(bin)
-
def doLoad(cls: ClassPath[AbstractFile]#ClassRep): Boolean = true
def needCompile(bin: AbstractFile, src: AbstractFile) =
diff --git a/src/compiler/scala/tools/nsc/backend/Platform.scala b/src/compiler/scala/tools/nsc/backend/Platform.scala
index 25a6e6f36a..3bca16635b 100644
--- a/src/compiler/scala/tools/nsc/backend/Platform.scala
+++ b/src/compiler/scala/tools/nsc/backend/Platform.scala
@@ -12,8 +12,8 @@ import io.AbstractFile
/** The platform dependent pieces of Global.
*/
trait Platform {
- val global: Global
- import global._
+ val symbolTable: symtab.SymbolTable
+ import symbolTable._
/** The binary classfile representation type */
@deprecated("BinaryRepr is not an abstract type anymore. It's an alias that points at AbstractFile. It'll be removed before Scala 2.11 is released.", "2.11.0-M5")
@@ -22,9 +22,6 @@ trait Platform {
/** The compiler classpath. */
def classPath: ClassPath[AbstractFile]
- /** The root symbol loader. */
- def rootLoader: LazyType
-
/** Update classpath with a substitution that maps entries to entries */
def updateClassPath(subst: Map[ClassPath[AbstractFile], ClassPath[AbstractFile]])
@@ -37,9 +34,6 @@ trait Platform {
/** The various ways a boxed primitive might materialize at runtime. */
def isMaybeBoxed(sym: Symbol): Boolean
- /** Create a new class loader to load class file `bin` */
- def newClassLoader(bin: AbstractFile): loaders.SymbolLoader
-
/**
* Tells whether a class should be loaded and entered into the package
* scope. On .NET, this method returns `false` for all synthetic classes
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index e6f21fc1e3..448ce9cb5b 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -1479,12 +1479,16 @@ abstract class GenICode extends SubComponent {
if (mustUseAnyComparator) {
// when -optimise is on we call the @inline-version of equals, found in ScalaRunTime
- val equalsMethod =
+ val equalsMethod: Symbol =
if (!settings.optimise) {
def default = platform.externalEquals
platform match {
+ // TODO: define `externalEqualsNumNum`, `externalEqualsNumChar` and `externalEqualsNumObject` in Platform
+ // so we don't need special casing here
case x: JavaPlatform =>
- import x._
+ // We need this cast because pattern matcher doesn't narrow type properly
+ val javaPlatformRefined = x.asInstanceOf[JavaPlatform { val global: GenICode.this.global.type }]
+ import javaPlatformRefined._
if (l.tpe <:< BoxedNumberClass.tpe) {
if (r.tpe <:< BoxedNumberClass.tpe) externalEqualsNumNum
else if (r.tpe <:< BoxedCharacterClass.tpe) externalEqualsNumChar
diff --git a/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala b/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
index 1fd24d4e1f..fca18eb09e 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
@@ -10,6 +10,7 @@ package icode
import java.io.PrintWriter
import analysis.{ Liveness, ReachingDefinitions }
import scala.tools.nsc.symtab.classfile.ICodeReader
+import scala.reflect.io.AbstractFile
/** Glue together ICode parts.
*
@@ -118,6 +119,9 @@ abstract class ICodes extends AnyRef
else
enteringTyper { lookup }
}
+ lazy val symbolTable: global.type = global
+ lazy val loaders: global.loaders.type = global.loaders
+ def classPath: util.ClassPath[AbstractFile] = ICodes.this.global.platform.classPath
}
/** A phase which works on icode. */
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
index a7f43eefed..365aecf4a1 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
@@ -1187,13 +1187,17 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
}
if (mustUseAnyComparator) {
- val equalsMethod = {
+ val equalsMethod: Symbol = {
- def default = platform.externalEquals
+ def default: Symbol = platform.externalEquals
platform match {
+ // TODO: define `externalEqualsNumNum`, `externalEqualsNumChar` and `externalEqualsNumObject` in Platform
+ // so we don't need special casing here
case x: JavaPlatform =>
- import x._
+ // We need this cast because pattern matcher doesn't narrow type properly
+ val javaPlatformRefined = x.asInstanceOf[JavaPlatform { val global: BCodeBodyBuilder.this.global.type }]
+ import javaPlatformRefined._
if (l.tpe <:< BoxedNumberClass.tpe) {
if (r.tpe <:< BoxedNumberClass.tpe) externalEqualsNumNum
else if (r.tpe <:< BoxedCharacterClass.tpe) externalEqualsNumChar
diff --git a/src/compiler/scala/tools/nsc/symtab/BrowsingLoaders.scala b/src/compiler/scala/tools/nsc/symtab/BrowsingLoaders.scala
index f93be78a3f..0d756fd309 100644
--- a/src/compiler/scala/tools/nsc/symtab/BrowsingLoaders.scala
+++ b/src/compiler/scala/tools/nsc/symtab/BrowsingLoaders.scala
@@ -13,8 +13,11 @@ import scala.tools.nsc.io.AbstractFile
* are managed automatically.
*/
abstract class BrowsingLoaders extends SymbolLoaders {
- import global._
+ val global: Global
+ val symbolTable: global.type
+ protected def compileLate(srcfile: AbstractFile) = global.currentRun.compileLate(srcfile)
+ import global._
import syntaxAnalyzer.{OutlineParser, MalformedInput}
/*
diff --git a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala
index 201a2315ab..d22352154b 100644
--- a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala
+++ b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala
@@ -20,13 +20,23 @@ import scala.reflect.io.{ AbstractFile, NoAbstractFile }
* @version 1.0
*/
abstract class SymbolLoaders {
- val global: Global
- import global._
-
+ val symbolTable: symtab.SymbolTable {
+ def settings: Settings
+ }
+ val platform: backend.Platform {
+ val symbolTable: SymbolLoaders.this.symbolTable.type
+ }
+ import symbolTable._
/**
* Required by ClassfileParser. Check documentation in that class for details.
*/
protected def lookupMemberAtTyperPhaseIfPossible(sym: Symbol, name: Name): Symbol
+ /**
+ * Should forward to `Run.compileLate`. The more principled fix would be to
+ * determine why this functionality is needed and extract it into a separate
+ * interface.
+ */
+ protected def compileLate(srcfile: AbstractFile): Unit
import SymbolLoadersStats._
protected def enterIfNew(owner: Symbol, member: Symbol, completer: SymbolLoader): Symbol = {
@@ -80,14 +90,14 @@ abstract class SymbolLoaders {
name+"\none of them needs to be removed from classpath"
)
else if (settings.termConflict.value == "package") {
- global.warning(
+ warning(
"Resolving package/object name conflict in favor of package " +
preExisting.fullName + ". The object will be inaccessible."
)
root.info.decls.unlink(preExisting)
}
else {
- global.warning(
+ warning(
"Resolving package/object name conflict in favor of object " +
preExisting.fullName + ". The package will be inaccessible."
)
@@ -149,12 +159,12 @@ abstract class SymbolLoaders {
case (Some(bin), Some(src))
if platform.needCompile(bin, src) && !binaryOnly(owner, classRep.name) =>
if (settings.verbose) inform("[symloader] picked up newer source file for " + src.path)
- global.loaders.enterToplevelsFromSource(owner, classRep.name, src)
+ enterToplevelsFromSource(owner, classRep.name, src)
case (None, Some(src)) =>
if (settings.verbose) inform("[symloader] no class, picked up source file for " + src.path)
- global.loaders.enterToplevelsFromSource(owner, classRep.name, src)
+ enterToplevelsFromSource(owner, classRep.name, src)
case (Some(bin), _) =>
- global.loaders.enterClassAndModule(owner, classRep.name, platform.newClassLoader(bin))
+ enterClassAndModule(owner, classRep.name, new ClassfileLoader(bin))
}
}
@@ -250,12 +260,23 @@ abstract class SymbolLoaders {
class ClassfileLoader(val classfile: AbstractFile) extends SymbolLoader with FlagAssigningCompleter {
private object classfileParser extends {
- val global: SymbolLoaders.this.global.type = SymbolLoaders.this.global
+ val symbolTable: SymbolLoaders.this.symbolTable.type = SymbolLoaders.this.symbolTable
} with ClassfileParser {
override protected type ThisConstantPool = ConstantPool
override protected def newConstantPool: ThisConstantPool = new ConstantPool
override protected def lookupMemberAtTyperPhaseIfPossible(sym: Symbol, name: Name): Symbol =
SymbolLoaders.this.lookupMemberAtTyperPhaseIfPossible(sym, name)
+ /*
+ * The type alias and the cast (where the alias is used) is needed due to problem described
+ * in SI-7585. In this particular case, the problem is that we need to make sure that symbol
+ * table used by symbol loaders is exactly the same as they one used by classfileParser.
+ * If you look at the path-dependent types we have here everything should work out ok but
+ * due to issue described in SI-7585 type-checker cannot tie the knot here.
+ *
+ */
+ private type SymbolLoadersRefined = SymbolLoaders { val symbolTable: classfileParser.symbolTable.type }
+ val loaders = SymbolLoaders.this.asInstanceOf[SymbolLoadersRefined]
+ val classPath = platform.classPath
}
protected def description = "class file "+ classfile.toString
@@ -282,7 +303,7 @@ abstract class SymbolLoaders {
protected def description = "source file "+ srcfile.toString
override def fromSource = true
override def sourcefile = Some(srcfile)
- protected def doComplete(root: Symbol): Unit = global.currentRun.compileLate(srcfile)
+ protected def doComplete(root: Symbol): Unit = compileLate(srcfile)
}
object moduleClassLoader extends SymbolLoader with FlagAssigningCompleter {
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
index 4743260c79..ad2b024935 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -17,6 +17,7 @@ import scala.reflect.internal.{ JavaAccFlags }
import scala.reflect.internal.pickling.{PickleBuffer, ByteCodecs}
import scala.tools.nsc.io.AbstractFile
+import util.ClassPath
/** This abstract class implements a class file parser.
*
@@ -24,9 +25,14 @@ import scala.tools.nsc.io.AbstractFile
* @version 1.0
*/
abstract class ClassfileParser {
- val global: Global
- import global._
+ val symbolTable: SymbolTable {
+ def settings: Settings
+ }
+ val loaders: SymbolLoaders {
+ val symbolTable: ClassfileParser.this.symbolTable.type
+ }
+ import symbolTable._
/**
* If typer phase is defined then perform member lookup of a symbol
* `sym` at typer phase. This method results from refactoring. The
@@ -37,6 +43,9 @@ abstract class ClassfileParser {
*/
protected def lookupMemberAtTyperPhaseIfPossible(sym: Symbol, name: Name): Symbol
+ /** The compiler classpath. */
+ def classPath: ClassPath[AbstractFile]
+
import definitions._
import scala.reflect.internal.ClassfileConstants._
import Flags._
@@ -64,7 +73,7 @@ abstract class ClassfileParser {
def srcfile = srcfile0
- private def optimized = global.settings.optimise.value
+ private def optimized = settings.optimise.value
private def currentIsTopLevel = !(currentClass.decodedName containsChar '$')
// u1, u2, and u4 are what these data types are called in the JVM spec.
@@ -84,7 +93,7 @@ abstract class ClassfileParser {
private def readType() = pool getType u2
private object unpickler extends scala.reflect.internal.pickling.UnPickler {
- val global: ClassfileParser.this.global.type = ClassfileParser.this.global
+ val symbolTable: ClassfileParser.this.symbolTable.type = ClassfileParser.this.symbolTable
}
private def handleMissing(e: MissingRequirementError) = {
@@ -344,7 +353,7 @@ abstract class ClassfileParser {
}
private def loadClassSymbol(name: Name): Symbol = {
- val file = global.classPath findSourceFile ("" +name) getOrElse {
+ val file = classPath findSourceFile ("" +name) getOrElse {
// SI-5593 Scaladoc's current strategy is to visit all packages in search of user code that can be documented
// therefore, it will rummage through the classpath triggering errors whenever it encounters package objects
// that are not in their correct place (see bug for details)
@@ -352,7 +361,7 @@ abstract class ClassfileParser {
warning(s"Class $name not found - continuing with a stub.")
return NoSymbol.newClass(name.toTypeName)
}
- val completer = new global.loaders.ClassfileLoader(file)
+ val completer = new loaders.ClassfileLoader(file)
var owner: Symbol = rootMirror.RootClass
var sym: Symbol = NoSymbol
var ss: Name = null
@@ -990,7 +999,7 @@ abstract class ClassfileParser {
def enterClassAndModule(entry: InnerClassEntry, file: AbstractFile) {
def jflags = entry.jflags
- val completer = new global.loaders.ClassfileLoader(file)
+ val completer = new loaders.ClassfileLoader(file)
val name = entry.originalName
val sflags = jflags.toScalaFlags
val owner = ownerForFlags(jflags)
@@ -998,7 +1007,7 @@ abstract class ClassfileParser {
val innerClass = owner.newClass(name.toTypeName, NoPosition, sflags) setInfo completer
val innerModule = owner.newModule(name.toTermName, NoPosition, sflags) setInfo completer
- innerModule.moduleClass setInfo global.loaders.moduleClassLoader
+ innerModule.moduleClass setInfo loaders.moduleClassLoader
List(innerClass, innerModule.moduleClass) foreach (_.associatedFile = file)
scope enter innerClass
@@ -1019,7 +1028,7 @@ abstract class ClassfileParser {
for (entry <- innerClasses.entries) {
// create a new class member for immediate inner classes
if (entry.outerName == currentClass) {
- val file = global.classPath.findSourceFile(entry.externalName.toString) getOrElse {
+ val file = classPath.findSourceFile(entry.externalName.toString) getOrElse {
throw new AssertionError(entry.externalName)
}
enterClassAndModule(entry, file)
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
index 5c8819945a..f704d8ac89 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
@@ -20,6 +20,8 @@ import scala.reflect.internal.JavaAccFlags
*/
abstract class ICodeReader extends ClassfileParser {
val global: Global
+ val symbolTable: global.type
+ val loaders: global.loaders.type
import global._
import icodes._