diff options
author | Lukas Rytz <lukas.rytz@gmail.com> | 2014-06-10 10:55:06 +0200 |
---|---|---|
committer | Lukas Rytz <lukas.rytz@gmail.com> | 2014-07-08 13:08:06 +0200 |
commit | 4c2217e8a0e67ccd675aa4f1be253f87697c9025 (patch) | |
tree | d0c6b70356670ddb6e65f594d9dd41bf609ca977 /src/compiler | |
parent | b664a488acca9d89c600f5b87ba9a9ca77aace59 (diff) | |
download | scala-4c2217e8a0e67ccd675aa4f1be253f87697c9025.tar.gz scala-4c2217e8a0e67ccd675aa4f1be253f87697c9025.tar.bz2 scala-4c2217e8a0e67ccd675aa4f1be253f87697c9025.zip |
Minor cleanups and comments in GenBCode
Diffstat (limited to 'src/compiler')
5 files changed, 61 insertions, 15 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala index 51bfdf0027..07890dd5f8 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala @@ -460,6 +460,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters { // Can't call .toInterface (at this phase) or we trip an assertion. // See PackratParser#grow for a method which fails with an apparent mismatch // between "object PackratParsers$class" and "trait PackratParsers" + // TODO @lry do we have a test for that? if (sym.isImplClass) { // pos/spec-List.scala is the sole failure if we don't check for NoSymbol val traitSym = sym.owner.info.decl(tpnme.interfaceName(sym.name)) @@ -469,6 +470,8 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters { } } + // TODO @lry: code duplication between here and method asmClassType. + assert(hasInternalName(sym), s"Invoked for a symbol lacking JVM internal name: ${sym.fullName}") assert(!phantomTypeMap.contains(sym), "phantom types not supposed to reach here.") diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala index 62dfb4917d..22cb189c70 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala @@ -21,8 +21,9 @@ abstract class BCodeTypes extends BCodeIdiomatic { import global._ import bTypes._ - // when compiling the Scala library, some assertions don't hold (e.g., scala.Boolean has null superClass although it's not an interface) - val isCompilingStdLib = !(settings.sourcepath.isDefault) + // Used only for assertions. When compiling the Scala library, some assertions don't hold + // (e.g., scala.Boolean has null superClass although it's not an interface) + private val isCompilingStdLib = !(settings.sourcepath.isDefault) // special names var StringReference : ClassBType = null @@ -175,12 +176,21 @@ abstract class BCodeTypes extends BCodeIdiomatic { // ------------------------------------------------ /** - * TODO @lry should probably be a map form ClassBType to Tracked + * Type information for classBTypes. + * + * TODO rename Tracked */ - val exemplars = new java.util.concurrent.ConcurrentHashMap[BType, Tracked] + val exemplars = new java.util.concurrent.ConcurrentHashMap[ClassBType, Tracked] /** * Maps class symbols to their corresponding `Tracked` instance. + * + * This map is only used during the first backend phase (Worker1) where ClassDef trees are + * transformed into ClassNode asm trees. In this phase, ClassBTypes and their Tracked are created + * and added to the `exemplars` map. The `symExemplars` map is only used to know if a symbol has + * already been visited. + * + * TODO move this map to the builder class. it's only used during building. can be gc'd with the builder. */ val symExemplars = new java.util.concurrent.ConcurrentHashMap[Symbol, Tracked] @@ -313,7 +323,7 @@ abstract class BCodeTypes extends BCodeIdiomatic { final def isDeprecated(sym: Symbol): Boolean = { sym.annotations exists (_ matches definitions.DeprecatedAttr) } /* must-single-thread */ - final def hasInternalName(sym: Symbol) = { sym.isClass || (sym.isModule && !sym.isMethod) } + final def hasInternalName(sym: Symbol) = sym.isClass || sym.isModuleNotMethod /* must-single-thread */ def getSuperInterfaces(csym: Symbol): List[Symbol] = { @@ -702,6 +712,10 @@ abstract class BCodeTypes extends BCodeIdiomatic { var x = ics while (x ne NoSymbol) { assert(x.isClass, s"not a class symbol: ${x.fullName}") + // Uses `rawowner` because `owner` reflects changes in the owner chain due to flattening. + // The owner chain of a class only contains classes. This is because the lambdalift phase + // changes the `rawowner` destructively to point to the enclosing class. Before, the owner + // might be for example a method. val isInner = !x.rawowner.isPackageClass if (isInner) { chain ::= x @@ -794,14 +808,23 @@ abstract class BCodeTypes extends BCodeIdiomatic { * must-single-thread */ def javaFlags(sym: Symbol): Int = { - // constructors of module classes should be private - // PP: why are they only being marked private at this stage and not earlier? + // constructors of module classes should be private. introduced in b06edbc, probably to prevent + // creating module instances from java. for nested modules, the constructor needs to be public + // since they are created by the outer class and stored in a field. a java client can create + // new instances via outerClassInstance.new InnerModuleClass$(). + // TODO: do this early, mark the symbol private. val privateFlag = sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModule(sym.owner)) - // Final: the only fields which can receive ACC_FINAL are eager vals. - // Neither vars nor lazy vals can, because: + // Symbols marked in source as `final` have the FINAL flag. (In the past, the flag was also + // added to modules and module classes, not anymore since 296b706). + // Note that the presence of the `FINAL` flag on a symbol does not correspond 1:1 to emitting + // ACC_FINAL in bytecode. + // + // Top-level modules are marked ACC_FINAL in bytecode (even without the FINAL flag). Nested + // objects don't get the flag to allow overriding (under -Yoverride-objects, SI-5676). // + // For fields, only eager val fields can receive ACC_FINAL. vars or lazy vals can't: // Source: http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.5.3 // "Another problem is that the specification allows aggressive // optimization of final fields. Within a thread, it is permissible to @@ -818,7 +841,6 @@ abstract class BCodeTypes extends BCodeIdiomatic { // we can exclude lateFINAL. Such symbols are eligible for inlining, but to // avoid breaking proxy software which depends on subclassing, we do not // emit ACC_FINAL. - // Nested objects won't receive ACC_FINAL in order to allow for their overriding. val finalFlag = ( (((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModule(sym)) @@ -845,6 +867,10 @@ abstract class BCodeTypes extends BCodeIdiomatic { if (sym.isVarargsMethod) ACC_VARARGS else 0, if (sym.hasFlag(symtab.Flags.SYNCHRONIZED)) ACC_SYNCHRONIZED else 0 ) + // TODO @lry should probably also check / add "deprectated" + // all call sites of "javaFlags" seem to check for deprecation rigth after. + // Exception: the call below in javaFieldFlags. However, the caller of javaFieldFlags then + // does the check. } /* diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index 5b0fa6f7a8..aac4f9cbfc 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -235,10 +235,24 @@ abstract class BTypes[G <: Global](val __global_dont_use: G) { /** * Class or Interface type. * - * Classes are represented using their name as a slice of the `chrs` array. This representation is - * efficient because the JVM class name is initially created using `classSymbol.javaBinaryName`. - * This already adds the necessary string to the `chrs` array, so it makes sense to reuse the same - * name table in the backend. + * The information for creating a ClassBType (superClass, interfaces, etc) is obtained + * - either from a ClassSymbol, for classes being compiled or referenced from source (see + * BCodeTypes) + * - or, during inlining, from ASM ClassNodes that are parsed from class files. + * + * The class name is represented as a slice of the `chrs` array. This representation is efficient + * because the JVM class name is obtained through `classSymbol.javaBinaryName`. This already adds + * the necessary string to the `chrs` array, so it makes sense to reuse the same name table in the + * backend. + * + * Not a case class because that would expose the constructor that takes (offset, length) + * parameters (I didn't find a way to make it private, also the factory in the companion). + * + * @param offset See below + * @param length The class name is represented as offset and length in the `chrs` array. + * The (public) constructors of ClassBType take a BTypeName, which are + * hash-consed. This ensures that two ClassBType instances for the same name + * have the same offset and length. * * Not a case class because that would expose the (Int, Int) constructor (didn't find a way to * make it private, also the factory in the companion). diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 988c04f514..ce7e7c49ff 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -636,6 +636,9 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM { innerSym.rawname + innerSym.moduleSuffix // add inner classes which might not have been referenced yet + // TODO @lry according to the spec, all nested classes should be added, also local and + // anonymous. This seems to add only member classes - or not? it's exitingErasure, so maybe + // local / anonymous classes have been lifted by lambdalift. are they in the "decls" though? exitingErasure { for (sym <- List(csym, csym.linkedClassOfClass); m <- sym.info.decls.map(innerClassSymbolFor) if m.isClass) innerClassBuffer += m diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala index 9b292fee7f..6b192556d9 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala @@ -271,7 +271,7 @@ abstract class GenBCode extends BCodeSyncAndTry { override def run() { arrivalPos = 0 // just in case - scalaPrimitives.init + scalaPrimitives.init() initBCodeTypes() // initBytecodeWriter invokes fullName, thus we have to run it before the typer-dependent thread is activated. |