Skip to content

Commit 187924c

Browse files
committed
Clean up and document some usages of flags in the backend
When re-assigning the owner of a symbol, the original owner at each phase is now stored in the symbol table. Method `ownerAtCurrentPhase` returns the owner at the current phase.
1 parent d04de01 commit 187924c

File tree

8 files changed

+168
-46
lines changed

8 files changed

+168
-46
lines changed

src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
977977
if (!isModuleInitialized &&
978978
jMethodName == INSTANCE_CONSTRUCTOR_NAME &&
979979
jname == INSTANCE_CONSTRUCTOR_NAME &&
980-
isStaticModule(siteSymbol)) {
980+
isStaticModuleClass(siteSymbol)) {
981981
isModuleInitialized = true
982982
mnode.visitVarInsn(asm.Opcodes.ALOAD, 0)
983983
mnode.visitFieldInsn(

src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
9595

9696
claszSymbol = cd.symbol
9797
isCZParcelable = isAndroidParcelableClass(claszSymbol)
98-
isCZStaticModule = isStaticModule(claszSymbol)
98+
isCZStaticModule = isStaticModuleClass(claszSymbol)
9999
isCZRemote = isRemote(claszSymbol)
100100
thisName = internalName(claszSymbol)
101101

src/compiler/scala/tools/nsc/backend/jvm/BCodeTypes.scala

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -627,18 +627,30 @@ abstract class BCodeTypes extends BCodeIdiomatic {
627627
false
628628
}
629629

630-
/*
630+
/**
631631
* must-single-thread
632+
*
633+
* True for module classes of package level objects. The backend will generate a mirror class for
634+
* such objects.
632635
*/
633-
def isTopLevelModule(sym: Symbol): Boolean = {
634-
exitingPickler { sym.isModuleClass && !sym.isImplClass && !sym.isNestedClass }
636+
def isTopLevelModuleClass(sym: Symbol): Boolean = exitingPickler {
637+
// time travel to pickler required for isNestedClass (looks at owner)
638+
val r = sym.isModuleClass && !sym.isNestedClass
639+
// The mixin phase adds the `lateMODULE` flag to trait implementation classes. Since the flag
640+
// is late, it should not be visible here inside the time travel. We check this.
641+
if (r) assert(!sym.isImplClass, s"isModuleClass should be false for impl class $sym")
642+
r
635643
}
636644

637-
/*
645+
/**
638646
* must-single-thread
647+
*
648+
* True for module classes of modules that are top-level or owned only by objects. Module classes
649+
* for such objects will get a MODULE$ flag and a corresponding static initializer.
639650
*/
640-
def isStaticModule(sym: Symbol): Boolean = {
641-
sym.isModuleClass && !sym.isImplClass && !sym.isLifted
651+
def isStaticModuleClass(sym: Symbol): Boolean = exitingPickler {
652+
// time travel to pickler required for isStatic (looks at owner)
653+
sym.isModuleClass && sym.isStatic
642654
}
643655

644656
// ---------------------------------------------------------------------
@@ -743,7 +755,7 @@ abstract class BCodeTypes extends BCodeIdiomatic {
743755
null
744756
else {
745757
val outerName = innerSym.rawowner.javaBinaryName
746-
if (isTopLevelModule(innerSym.rawowner)) nme.stripModuleSuffix(outerName)
758+
if (isTopLevelModuleClass(innerSym.rawowner)) nme.stripModuleSuffix(outerName)
747759
else outerName
748760
}
749761
}
@@ -825,7 +837,7 @@ abstract class BCodeTypes extends BCodeIdiomatic {
825837
// new instances via outerClassInstance.new InnerModuleClass$().
826838
// TODO: do this early, mark the symbol private.
827839
val privateFlag =
828-
sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModule(sym.owner))
840+
sym.isPrivate || (sym.isPrimaryConstructor && isTopLevelModuleClass(sym.owner))
829841

830842
// Symbols marked in source as `final` have the FINAL flag. (In the past, the flag was also
831843
// added to modules and module classes, not anymore since 296b706).
@@ -854,7 +866,7 @@ abstract class BCodeTypes extends BCodeIdiomatic {
854866
// emit ACC_FINAL.
855867

856868
val finalFlag = (
857-
(((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModule(sym))
869+
(((sym.rawflags & symtab.Flags.FINAL) != 0) || isTopLevelModuleClass(sym))
858870
&& !sym.enclClass.isInterface
859871
&& !sym.isClassConstructor
860872
&& !sym.isMutable // lazy vals and vars both

src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ abstract class GenBCode extends BCodeSyncAndTry {
165165

166166
// -------------- mirror class, if needed --------------
167167
val mirrorC =
168-
if (isStaticModule(claszSymbol) && isTopLevelModule(claszSymbol)) {
168+
if (isTopLevelModuleClass(claszSymbol)) {
169169
if (claszSymbol.companionClass == NoSymbol) {
170170
mirrorCodeGen.genMirrorClass(claszSymbol, cunit)
171171
} else {

src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,12 @@ abstract class SymbolLoaders {
240240
}
241241
}
242242

243+
private def phaseBeforeRefchecks: Phase = {
244+
var resPhase = phase
245+
while (resPhase.refChecked) resPhase = resPhase.prev
246+
resPhase
247+
}
248+
243249
/**
244250
* Load contents of a package
245251
*/
@@ -248,19 +254,24 @@ abstract class SymbolLoaders {
248254

249255
protected def doComplete(root: Symbol) {
250256
assert(root.isPackageClass, root)
251-
root.setInfo(new PackageClassInfoType(newScope, root))
252-
253-
if (!root.isRoot) {
254-
for (classRep <- classpath.classes) {
255-
initializeFromClassPath(root, classRep)
256-
}
257-
}
258-
if (!root.isEmptyPackageClass) {
259-
for (pkg <- classpath.packages) {
260-
enterPackage(root, pkg.name, new PackageLoader(pkg))
257+
// Time travel to a phase before refchecks avoids an initialization issue. `openPackageModule`
258+
// creates a module symbol and invokes invokes `companionModule` while the `infos` field is
259+
// still null. This calls `isModuleNotMethod`, which forces the `info` if run after refchecks.
260+
enteringPhase(phaseBeforeRefchecks) {
261+
root.setInfo(new PackageClassInfoType(newScope, root))
262+
263+
if (!root.isRoot) {
264+
for (classRep <- classpath.classes) {
265+
initializeFromClassPath(root, classRep)
266+
}
261267
}
268+
if (!root.isEmptyPackageClass) {
269+
for (pkg <- classpath.packages) {
270+
enterPackage(root, pkg.name, new PackageLoader(pkg))
271+
}
262272

263-
openPackageModule(root)
273+
openPackageModule(root)
274+
}
264275
}
265276
}
266277
}
@@ -290,7 +301,13 @@ abstract class SymbolLoaders {
290301

291302
protected def doComplete(root: Symbol) {
292303
val start = if (Statistics.canEnable) Statistics.startTimer(classReadNanos) else null
293-
classfileParser.parse(classfile, root)
304+
305+
// Running the classfile parser after refchecks can lead to "illegal class file dependency"
306+
// errors. More concretely, the classfile parser calls "sym.companionModule", which calls
307+
// "isModuleNotMethod" on the companion. After refchecks, this method forces the info, which
308+
// may run the classfile parser. This produces the error.
309+
enteringPhase(phaseBeforeRefchecks)(classfileParser.parse(classfile, root))
310+
294311
if (root.associatedFile eq NoAbstractFile) {
295312
root match {
296313
// In fact, the ModuleSymbol forwards its setter to the module class

src/compiler/scala/tools/nsc/transform/Flatten.scala

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,17 @@ abstract class Flatten extends InfoTransform {
7676
for (sym <- decls) {
7777
if (sym.isTerm && !sym.isStaticModule) {
7878
decls1 enter sym
79-
if (sym.isModule)
79+
if (sym.isModule) {
80+
// Nested, non-static moduls are transformed to methods.
81+
assert(sym.isMethod, s"Non-static $sym should have the lateMETHOD flag from RefChecks")
82+
// Note that module classes are not entered into the 'decls' of the ClassInfoType
83+
// of the outer class, only the module symbols are. So the current loop does
84+
// not visit module classes. Therefore we set the LIFTED flag here for module
85+
// classes.
86+
// TODO: should we also set the LIFTED flag for static, nested module classes?
87+
// currently they don't get the flag, even though they are lifted to the package
8088
sym.moduleClass setFlag LIFTED
89+
}
8190
} else if (sym.isClass)
8291
liftSymbol(sym)
8392
}

src/reflect/scala/reflect/internal/Symbols.scala

Lines changed: 97 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,23 +55,33 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
5555
def newFreeTypeSymbol(name: TypeName, flags: Long = 0L, origin: String): FreeTypeSymbol =
5656
new FreeTypeSymbol(name, origin) initFlags flags
5757

58-
/** The original owner of a class. Used by the backend to generate
59-
* EnclosingMethod attributes.
58+
/**
59+
* This map stores previous owners of symbols when owners are modified. This allows obtaining
60+
* the owner at each phase using phase travel and `sym.ownerAtCurrentPhase`.
61+
*
62+
* Invariant: for each symbol, the list of owners is sorted by the phase id at which the owner
63+
* was set.
6064
*/
61-
val originalOwner = perRunCaches.newMap[Symbol, Symbol]()
65+
val originalOwnerMap = perRunCaches.newMap[Symbol, List[(Symbol, Phase)]]()
6266

6367
// TODO - don't allow the owner to be changed without checking invariants, at least
6468
// when under some flag. Define per-phase invariants for owner/owned relationships,
6569
// e.g. after flatten all classes are owned by package classes, there are lots and
6670
// lots of these to be declared (or more realistically, discovered.)
67-
protected def saveOriginalOwner(sym: Symbol) {
68-
if (originalOwner contains sym) ()
69-
else originalOwner(sym) = sym.rawowner
71+
protected def saveOriginalOwner(sym: Symbol): Unit = {
72+
// some synthetic symbols have NoSymbol as owner initially
73+
if (sym.owner != NoSymbol) {
74+
val l = originalOwnerMap.getOrElse(sym, Nil)
75+
// When the owner is modified multiple times during a phase, we only store the original owner
76+
// the first time, which is the owner at the end of the previous phase.
77+
if (!l.exists(_._2 == phase))
78+
originalOwnerMap(sym) = ((sym.rawowner, phase) :: l).sortBy(_._2.id)
79+
}
7080
}
7181
protected def originalEnclosingMethod(sym: Symbol): Symbol = {
7282
if (sym.isMethod || sym == NoSymbol) sym
7383
else {
74-
val owner = originalOwner.getOrElse(sym, sym.rawowner)
84+
val owner = sym.originalOwner
7585
if (sym.isLocalDummy) owner.enclClass.primaryConstructor
7686
else originalEnclosingMethod(owner)
7787
}
@@ -757,8 +767,22 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
757767
* So "isModuleNotMethod" exists not for its achievement in
758768
* brevity, but to encapsulate the relevant condition.
759769
*/
760-
def isModuleNotMethod = isModule && !isMethod
761-
def isStaticModule = isModuleNotMethod && isStatic
770+
def isModuleNotMethod = {
771+
if (isModule) {
772+
if (phase.refChecked) this.info // force completion to make sure lateMETHOD is there.
773+
!isMethod
774+
} else false
775+
}
776+
777+
// After RefChecks, the `isStatic` check is mostly redundant: all non-static modules should
778+
// be methods (and vice versa). There's a corner case on the vice-versa with mixed-in module
779+
// symbols:
780+
// trait T { object A }
781+
// object O extends T
782+
// The module symbol A is cloned into T$impl (addInterfaces), and then cloned into O (mixin).
783+
// Since the original A is not static, it's turned into a method. The clone in O however is
784+
// static (owned by a module), but it's also a method.
785+
def isStaticModule = isModuleNotMethod && isStatic
762786

763787
final def isInitializedToDefault = !isType && hasAllFlags(DEFAULTINIT | ACCESSOR)
764788
final def isThisSym = isTerm && owner.thisSym == this
@@ -909,12 +933,33 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
909933
)
910934
final def isModuleVar = hasFlag(MODULEVAR)
911935

912-
/** Is this symbol static (i.e. with no outer instance)?
913-
* Q: When exactly is a sym marked as STATIC?
914-
* 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.
915-
* http://groups.google.com/group/scala-internals/browse_thread/thread/d385bcd60b08faf6
936+
/**
937+
* Is this symbol static (i.e. with no outer instance)?
938+
* Q: When exactly is a sym marked as STATIC?
939+
* A: If it's a member of a toplevel object, or of an object contained in a toplevel object, or
940+
* any number of levels deep.
941+
* http://groups.google.com/group/scala-internals/browse_thread/thread/d385bcd60b08faf6
942+
*
943+
* TODO: should this only be invoked on class / module symbols? because there's also `isStaticMember`.
944+
*
945+
* Note: the result of `isStatic` changes over time.
946+
* - Lambdalift local definitions to the class level, the `owner` field is modified.
947+
* object T { def foo { object O } }
948+
* After lambdalift, the OModule.isStatic is true.
949+
*
950+
* - After flatten, nested classes are moved to the package level. Invoking `owner` on a
951+
* class returns a package class, for which `isStaticOwner` is true. For example,
952+
* class C { object O }
953+
* OModuleClass.isStatic is true after flatten. Using phase travel to get before flatten,
954+
* method `owner` returns the class C.
955+
*
956+
* Why not make a stable version of `isStatic`? Maybe some parts of the compiler depend on the
957+
* current implementation. For example
958+
* trait T { def foo = 1 }
959+
* The method `foo` in the implementation class T$impl will be `isStatic`, because trait
960+
* impl classes get the `lateMODULE` flag (T$impl.isStaticOwner is true).
916961
*/
917-
def isStatic = (this hasFlag STATIC) || owner.isStaticOwner
962+
def isStatic = (this hasFlag STATIC) || ownerAtCurrentPhase.isStaticOwner
918963

919964
/** Is this symbol a static constructor? */
920965
final def isStaticConstructor: Boolean =
@@ -953,10 +998,10 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
953998

954999
/** Is this symbol defined in a block? */
9551000
@deprecated("Use isLocalToBlock instead", "2.11.0")
956-
final def isLocal: Boolean = owner.isTerm
1001+
final def isLocal: Boolean = ownerAtCurrentPhase.isTerm
9571002

9581003
/** Is this symbol defined in a block? */
959-
final def isLocalToBlock: Boolean = owner.isTerm
1004+
final def isLocalToBlock: Boolean = ownerAtCurrentPhase.isTerm
9601005

9611006
/** Is this symbol a constant? */
9621007
final def isConstant: Boolean = isStable && isConstantType(tpe.resultType)
@@ -1106,7 +1151,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
11061151
* - After lambdalift, all local method and class definitions (those not owned by a class
11071152
* or package class) change their owner to the enclosing class. This is done through
11081153
* a destructive "sym.owner = sym.owner.enclClass". The old owner is saved by
1109-
* saveOriginalOwner for the backend (needed to generate the EnclosingMethod attribute).
1154+
* saveOriginalOwner.
11101155
* - After flatten, all classes are owned by a PackageClass. This is done through a
11111156
* phase check (if after flatten) in the (overridden) method "def owner" in
11121157
* ModuleSymbol / ClassSymbol. The `rawowner` field is not modified.
@@ -1126,6 +1171,41 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
11261171
final def safeOwner: Symbol = if (this eq NoSymbol) NoSymbol else owner
11271172
final def assertOwner: Symbol = if (this eq NoSymbol) abort("no-symbol does not have an owner") else owner
11281173

1174+
/**
1175+
* The initial owner of this symbol.
1176+
*/
1177+
def originalOwner: Symbol = {
1178+
val owners = originalOwnerMap.getOrElse(this, Nil)
1179+
owners.headOption.map(_._1).getOrElse(rawowner)
1180+
}
1181+
1182+
/**
1183+
* In a better world this method would not exist. It returns the symbol's owner at the current
1184+
* phase, even after the `rawowner` field has been changed. The method `owner` only does a bit
1185+
* of that, as explained in its documentation. Example:
1186+
*
1187+
* class C { def f { def g } }
1188+
*
1189+
* After phase lambdalift, the rawowner field of g is C. Running `exitingPickler(gSym.owner)`
1190+
* returns C. `exitingPickler(gSym.ownerAtCurrentPhase)` returns f.
1191+
*
1192+
* Changing the semantics of `owner` seems to risky and a too large hit on performance.
1193+
*
1194+
* Note: similar to info transformers, if the owner is assigned in phase X, then
1195+
* `ownerAtCurrentPhase` only returns that new owner in phase X+1 (not yet in X).
1196+
*/
1197+
def ownerAtCurrentPhase: Symbol = {
1198+
// after flatten, we have to call `owner`, since that method does the actual flattening.
1199+
if (phase.flatClasses) owner
1200+
else {
1201+
val owners = originalOwnerMap.getOrElse(this, Nil)
1202+
// each pair in owners is an owner symbol, and the phase until which it is valid.
1203+
// we drop those that are expired in the current phase.
1204+
val currentPhaseOwner = owners.dropWhile(_._2.id < phase.id).headOption.map(_._1)
1205+
currentPhaseOwner.getOrElse(rawowner)
1206+
}
1207+
}
1208+
11291209
// TODO - don't allow the owner to be changed without checking invariants, at least
11301210
// when under some flag. Define per-phase invariants for owner/owned relationships,
11311211
// e.g. after flatten all classes are owned by package classes, there are lots and
@@ -1139,7 +1219,6 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
11391219
}
11401220

11411221
def ownerChain: List[Symbol] = this :: owner.ownerChain
1142-
def originalOwnerChain: List[Symbol] = this :: originalOwner.getOrElse(this, rawowner).originalOwnerChain
11431222

11441223
// Non-classes skip self and return rest of owner chain; overridden in ClassSymbol.
11451224
def enclClassChain: List[Symbol] = owner.enclClassChain

src/reflect/scala/reflect/runtime/SymbolLoaders.scala

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,15 @@ private[reflect] trait SymbolLoaders { self: SymbolTable =>
6565
class LazyPackageType extends LazyType with FlagAgnosticCompleter {
6666
override def complete(sym: Symbol) {
6767
assert(sym.isPackageClass)
68-
sym setInfo new ClassInfoType(List(), new PackageScope(sym), sym)
68+
// Time travel to a phase before refchecks avoids an initialization issue. `openPackageModule`
69+
// creates a module symbol and invokes invokes `companionModule` while the `infos` field is
70+
// still null. This calls `isModuleNotMethod`, which forces the `info` if run after refchecks.
71+
slowButSafeEnteringPhaseNotLaterThan(picklerPhase) {
72+
sym setInfo new ClassInfoType(List(), new PackageScope(sym), sym)
6973
// override def safeToString = pkgClass.toString
70-
openPackageModule(sym)
71-
markAllCompleted(sym)
74+
openPackageModule(sym)
75+
markAllCompleted(sym)
76+
}
7277
}
7378
}
7479

0 commit comments

Comments
 (0)