From 0ccdb151ffe9caa9eae7d01a4f2eacc87fa8f5ff Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Tue, 10 Jun 2014 11:21:13 +0200 Subject: Clean up and document some usages of flags in the backend --- .../tools/nsc/backend/jvm/BCodeBodyBuilder.scala | 2 +- .../tools/nsc/backend/jvm/BCodeSkelBuilder.scala | 2 +- .../scala/tools/nsc/backend/jvm/BCodeTypes.scala | 50 ++++++++++++--- .../scala/tools/nsc/backend/jvm/GenBCode.scala | 2 +- .../scala/tools/nsc/symtab/SymbolLoaders.scala | 41 ++++++++---- .../scala/tools/nsc/transform/Flatten.scala | 11 +++- .../scala/tools/nsc/transform/LazyVals.scala | 4 +- src/reflect/scala/reflect/internal/Symbols.scala | 75 +++++++++++++++++----- .../scala/reflect/runtime/SymbolLoaders.scala | 11 +++- 9 files changed, 154 insertions(+), 44 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala index bffa4bc51d..b79cf01f41 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala @@ -977,7 +977,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder { if (!isModuleInitialized && jMethodName == INSTANCE_CONSTRUCTOR_NAME && jname == INSTANCE_CONSTRUCTOR_NAME && - isStaticModule(siteSymbol)) { + isStaticModuleClass(siteSymbol)) { isModuleInitialized = true mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) mnode.visitFieldInsn( diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala index effc68c5e3..f8ecb8e643 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala @@ -95,7 +95,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers { claszSymbol = cd.symbol isCZParcelable = isAndroidParcelableClass(claszSymbol) - isCZStaticModule = isStaticModule(claszSymbol) + isCZStaticModule = isStaticModuleClass(claszSymbol) isCZRemote = isRemote(claszSymbol) thisName = internalName(claszSymbol) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala index 9b131cb123..b373f8d74d 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala @@ -627,20 +627,52 @@ abstract class BCodeTypes extends BCodeIdiomatic { false } - /* + /** * must-single-thread + * + * True for module classes of package level objects. The backend will generate a mirror class for + * such objects. */ - def isTopLevelModule(sym: Symbol): Boolean = { - exitingPickler { sym.isModuleClass && !sym.isImplClass && !sym.isNestedClass } + def isTopLevelModuleClass(sym: Symbol): Boolean = exitingPickler { + // phase travel to pickler required for isNestedClass (looks at owner) + val r = sym.isModuleClass && !sym.isNestedClass + // The mixin phase adds the `lateMODULE` flag to trait implementation classes. Since the flag + // is late, it should not be visible here inside the time travel. We check this. + if (r) assert(!sym.isImplClass, s"isModuleClass should be false for impl class $sym") + r } - /* + /** * must-single-thread + * + * True for module classes of modules that are top-level or owned only by objects. Module classes + * for such objects will get a MODULE$ flag and a corresponding static initializer. */ - def isStaticModule(sym: Symbol): Boolean = { - sym.isModuleClass && !sym.isImplClass && !sym.isLifted + def isStaticModuleClass(sym: Symbol): Boolean = { + /* The implementation of this method is tricky because it is a source-level property. Various + * phases changed the symbol's properties in the meantime. + * + * (1) Phase travel to to pickler is required to exclude implementation classes; they have the + * lateMODULEs after mixin, so isModuleClass would be true. + * + * (2) We cannot use `sym.isStatic` because lambdalift modified (destructively) the owner. For + * example, in + * object T { def f { object U } } + * the owner of U is T, so UModuleClass.isStatic is true. Phase travel does not help here. + * So we basically re-implement `sym.isStaticOwner`, but using the original owner chain. + */ + + def isOriginallyStaticOwner(sym: Symbol): Boolean = { + sym.isPackageClass || sym.isModuleClass && isOriginallyStaticOwner(sym.originalOwner) + } + + exitingPickler { // (1) + sym.isModuleClass && + isOriginallyStaticOwner(sym.originalOwner) // (2) + } } + // --------------------------------------------------------------------- // ---------------- InnerClasses attribute (JVMS 4.7.6) ---------------- // --------------------------------------------------------------------- @@ -743,7 +775,7 @@ abstract class BCodeTypes extends BCodeIdiomatic { null else { val outerName = innerSym.rawowner.javaBinaryName - if (isTopLevelModule(innerSym.rawowner)) nme.stripModuleSuffix(outerName) + if (isTopLevelModuleClass(innerSym.rawowner)) nme.stripModuleSuffix(outerName) else outerName } } @@ -825,7 +857,7 @@ abstract class BCodeTypes extends BCodeIdiomatic { // new instances via outerClassInstance.new InnerModuleClass$(). // TODO: do this early, mark the symbol private. val privateFlag = - sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModule(sym.owner)) + sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModuleClass(sym.owner)) // 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). @@ -854,7 +886,7 @@ abstract class BCodeTypes extends BCodeIdiomatic { // emit ACC_FINAL. val finalFlag = ( - (((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModule(sym)) + (((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModuleClass(sym)) && !sym.enclClass.isInterface && !sym.isClassConstructor && !sym.isMutable // lazy vals and vars both diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala index 6b192556d9..af5d01458a 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala @@ -165,7 +165,7 @@ abstract class GenBCode extends BCodeSyncAndTry { // -------------- mirror class, if needed -------------- val mirrorC = - if (isStaticModule(claszSymbol) && isTopLevelModule(claszSymbol)) { + if (isTopLevelModuleClass(claszSymbol)) { if (claszSymbol.companionClass == NoSymbol) { mirrorCodeGen.genMirrorClass(claszSymbol, cunit) } else { diff --git a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala index 447fa66ae4..82c2a4d6ed 100644 --- a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala +++ b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala @@ -240,6 +240,12 @@ abstract class SymbolLoaders { } } + private def phaseBeforeRefchecks: Phase = { + var resPhase = phase + while (resPhase.refChecked) resPhase = resPhase.prev + resPhase + } + /** * Load contents of a package */ @@ -248,19 +254,24 @@ abstract class SymbolLoaders { protected def doComplete(root: Symbol) { assert(root.isPackageClass, root) - root.setInfo(new PackageClassInfoType(newScope, root)) - - if (!root.isRoot) { - for (classRep <- classpath.classes) { - initializeFromClassPath(root, classRep) - } - } - if (!root.isEmptyPackageClass) { - for (pkg <- classpath.packages) { - enterPackage(root, pkg.name, new PackageLoader(pkg)) + // Time travel to a phase before refchecks avoids an initialization issue. `openPackageModule` + // creates a module symbol and invokes invokes `companionModule` while the `infos` field is + // still null. This calls `isModuleNotMethod`, which forces the `info` if run after refchecks. + enteringPhase(phaseBeforeRefchecks) { + root.setInfo(new PackageClassInfoType(newScope, root)) + + if (!root.isRoot) { + for (classRep <- classpath.classes) { + initializeFromClassPath(root, classRep) + } } + if (!root.isEmptyPackageClass) { + for (pkg <- classpath.packages) { + enterPackage(root, pkg.name, new PackageLoader(pkg)) + } - openPackageModule(root) + openPackageModule(root) + } } } } @@ -290,7 +301,13 @@ abstract class SymbolLoaders { protected def doComplete(root: Symbol) { val start = if (Statistics.canEnable) Statistics.startTimer(classReadNanos) else null - classfileParser.parse(classfile, root) + + // Running the classfile parser after refchecks can lead to "illegal class file dependency" + // errors. More concretely, the classfile parser calls "sym.companionModule", which calls + // "isModuleNotMethod" on the companion. After refchecks, this method forces the info, which + // may run the classfile parser. This produces the error. + enteringPhase(phaseBeforeRefchecks)(classfileParser.parse(classfile, root)) + if (root.associatedFile eq NoAbstractFile) { root match { // In fact, the ModuleSymbol forwards its setter to the module class diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala index c3fbfae322..fa53ef48b5 100644 --- a/src/compiler/scala/tools/nsc/transform/Flatten.scala +++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala @@ -76,8 +76,17 @@ abstract class Flatten extends InfoTransform { for (sym <- decls) { if (sym.isTerm && !sym.isStaticModule) { decls1 enter sym - if (sym.isModule) + if (sym.isModule) { + // Nested, non-static moduls are transformed to methods. + assert(sym.isMethod, s"Non-static $sym should have the lateMETHOD flag from RefChecks") + // Note that module classes are not entered into the 'decls' of the ClassInfoType + // of the outer class, only the module symbols are. So the current loop does + // not visit module classes. Therefore we set the LIFTED flag here for module + // classes. + // TODO: should we also set the LIFTED flag for static, nested module classes? + // currently they don't get the flag, even though they are lifted to the package sym.moduleClass setFlag LIFTED + } } else if (sym.isClass) liftSymbol(sym) } diff --git a/src/compiler/scala/tools/nsc/transform/LazyVals.scala b/src/compiler/scala/tools/nsc/transform/LazyVals.scala index b71d14a04f..38671ebaae 100644 --- a/src/compiler/scala/tools/nsc/transform/LazyVals.scala +++ b/src/compiler/scala/tools/nsc/transform/LazyVals.scala @@ -192,13 +192,15 @@ abstract class LazyVals extends Transform with TypingTransformers with ast.TreeD def mkSlowPathDef(clazz: Symbol, lzyVal: Symbol, cond: Tree, syncBody: List[Tree], stats: List[Tree], retVal: Tree): Tree = { + // Q: is there a reason to first set owner to `clazz` (by using clazz.newMethod), and then + // changing it to lzyVal.owner very soon after? Could we just do lzyVal.owner.newMethod? val defSym = clazz.newMethod(nme.newLazyValSlowComputeName(lzyVal.name.toTermName), lzyVal.pos, STABLE | PRIVATE) defSym setInfo MethodType(List(), lzyVal.tpe.resultType) defSym.owner = lzyVal.owner debuglog(s"crete slow compute path $defSym with owner ${defSym.owner} for lazy val $lzyVal") if (bitmaps.contains(lzyVal)) bitmaps(lzyVal).map(_.owner = defSym) - val rhs: Tree = (gen.mkSynchronizedCheck(clazz, cond, syncBody, stats)).changeOwner(currentOwner -> defSym) + val rhs: Tree = gen.mkSynchronizedCheck(clazz, cond, syncBody, stats).changeOwner(currentOwner -> defSym) DefDef(defSym, addBitmapDefs(lzyVal, BLOCK(rhs, retVal))) } diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index cc750fb88a..b6cce4524b 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -55,23 +55,29 @@ trait Symbols extends api.Symbols { self: SymbolTable => def newFreeTypeSymbol(name: TypeName, flags: Long = 0L, origin: String): FreeTypeSymbol = new FreeTypeSymbol(name, origin) initFlags flags - /** The original owner of a class. Used by the backend to generate - * EnclosingMethod attributes. + /** + * This map stores the original owner the the first time the owner of a symbol is re-assigned. + * The original owner of a symbol is needed in some places in the backend. Ideally, owners should + * be versioned like the type history. */ - val originalOwner = perRunCaches.newMap[Symbol, Symbol]() + private val originalOwnerMap = perRunCaches.newMap[Symbol, Symbol]() // TODO - don't allow the owner to be changed without checking invariants, at least // when under some flag. Define per-phase invariants for owner/owned relationships, // e.g. after flatten all classes are owned by package classes, there are lots and // lots of these to be declared (or more realistically, discovered.) - protected def saveOriginalOwner(sym: Symbol) { - if (originalOwner contains sym) () - else originalOwner(sym) = sym.rawowner + protected def saveOriginalOwner(sym: Symbol): Unit = { + // some synthetic symbols have NoSymbol as owner initially + if (sym.owner != NoSymbol) { + if (originalOwnerMap contains sym) () + else originalOwnerMap(sym) = sym.rawowner + } } + protected def originalEnclosingMethod(sym: Symbol): Symbol = { if (sym.isMethod || sym == NoSymbol) sym else { - val owner = originalOwner.getOrElse(sym, sym.rawowner) + val owner = sym.originalOwner if (sym.isLocalDummy) owner.enclClass.primaryConstructor else originalEnclosingMethod(owner) } @@ -757,8 +763,22 @@ trait Symbols extends api.Symbols { self: SymbolTable => * So "isModuleNotMethod" exists not for its achievement in * brevity, but to encapsulate the relevant condition. */ - def isModuleNotMethod = isModule && !isMethod - def isStaticModule = isModuleNotMethod && isStatic + def isModuleNotMethod = { + if (isModule) { + if (phase.refChecked) this.info // force completion to make sure lateMETHOD is there. + !isMethod + } else false + } + + // After RefChecks, the `isStatic` check is mostly redundant: all non-static modules should + // be methods (and vice versa). There's a corner case on the vice-versa with mixed-in module + // symbols: + // trait T { object A } + // object O extends T + // The module symbol A is cloned into T$impl (addInterfaces), and then cloned into O (mixin). + // Since the original A is not static, it's turned into a method. The clone in O however is + // static (owned by a module), but it's also a method. + def isStaticModule = isModuleNotMethod && isStatic final def isInitializedToDefault = !isType && hasAllFlags(DEFAULTINIT | ACCESSOR) final def isThisSym = isTerm && owner.thisSym == this @@ -909,10 +929,31 @@ trait Symbols extends api.Symbols { self: SymbolTable => ) final def isModuleVar = hasFlag(MODULEVAR) - /** Is this symbol static (i.e. with no outer instance)? - * Q: When exactly is a sym marked as STATIC? - * A: If it's a member of a toplevel object, or of an object contained in a toplevel object, or any number of levels deep. - * http://groups.google.com/group/scala-internals/browse_thread/thread/d385bcd60b08faf6 + /** + * Is this symbol static (i.e. with no outer instance)? + * Q: When exactly is a sym marked as STATIC? + * A: If it's a member of a toplevel object, or of an object contained in a toplevel object, or + * any number of levels deep. + * http://groups.google.com/group/scala-internals/browse_thread/thread/d385bcd60b08faf6 + * + * TODO: should this only be invoked on class / module symbols? because there's also `isStaticMember`. + * + * Note: the result of `isStatic` changes over time. + * - Lambdalift local definitions to the class level, the `owner` field is modified. + * object T { def foo { object O } } + * After lambdalift, the OModule.isStatic is true. + * + * - After flatten, nested classes are moved to the package level. Invoking `owner` on a + * class returns a package class, for which `isStaticOwner` is true. For example, + * class C { object O } + * OModuleClass.isStatic is true after flatten. Using phase travel to get before flatten, + * method `owner` returns the class C. + * + * Why not make a stable version of `isStatic`? Maybe some parts of the compiler depend on the + * current implementation. For example + * trait T { def foo = 1 } + * The method `foo` in the implementation class T$impl will be `isStatic`, because trait + * impl classes get the `lateMODULE` flag (T$impl.isStaticOwner is true). */ def isStatic = (this hasFlag STATIC) || owner.isStaticOwner @@ -1106,7 +1147,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => * - After lambdalift, all local method and class definitions (those not owned by a class * or package class) change their owner to the enclosing class. This is done through * a destructive "sym.owner = sym.owner.enclClass". The old owner is saved by - * saveOriginalOwner for the backend (needed to generate the EnclosingMethod attribute). + * saveOriginalOwner. * - After flatten, all classes are owned by a PackageClass. This is done through a * phase check (if after flatten) in the (overridden) method "def owner" in * ModuleSymbol / ClassSymbol. The `rawowner` field is not modified. @@ -1129,6 +1170,11 @@ trait Symbols extends api.Symbols { self: SymbolTable => final def safeOwner: Symbol = if (this eq NoSymbol) NoSymbol else owner final def assertOwner: Symbol = if (this eq NoSymbol) abort("no-symbol does not have an owner") else owner + /** + * The initial owner of this symbol. + */ + def originalOwner: Symbol = originalOwnerMap.getOrElse(this, rawowner) + // TODO - don't allow the owner to be changed without checking invariants, at least // when under some flag. Define per-phase invariants for owner/owned relationships, // e.g. after flatten all classes are owned by package classes, there are lots and @@ -1142,7 +1188,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => } def ownerChain: List[Symbol] = this :: owner.ownerChain - def originalOwnerChain: List[Symbol] = this :: originalOwner.getOrElse(this, rawowner).originalOwnerChain // Non-classes skip self and return rest of owner chain; overridden in ClassSymbol. def enclClassChain: List[Symbol] = owner.enclClassChain diff --git a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala index 7ba68b8733..50ea8d9868 100644 --- a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala +++ b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala @@ -65,10 +65,15 @@ private[reflect] trait SymbolLoaders { self: SymbolTable => class LazyPackageType extends LazyType with FlagAgnosticCompleter { override def complete(sym: Symbol) { assert(sym.isPackageClass) - sym setInfo new ClassInfoType(List(), new PackageScope(sym), sym) + // Time travel to a phase before refchecks avoids an initialization issue. `openPackageModule` + // creates a module symbol and invokes invokes `companionModule` while the `infos` field is + // still null. This calls `isModuleNotMethod`, which forces the `info` if run after refchecks. + slowButSafeEnteringPhaseNotLaterThan(picklerPhase) { + sym setInfo new ClassInfoType(List(), new PackageScope(sym), sym) // override def safeToString = pkgClass.toString - openPackageModule(sym) - markAllCompleted(sym) + openPackageModule(sym) + markAllCompleted(sym) + } } } -- cgit v1.2.3