diff options
author | Eugene Burmako <xeno.by@gmail.com> | 2012-09-23 15:07:53 +0200 |
---|---|---|
committer | Eugene Burmako <xeno.by@gmail.com> | 2012-09-26 18:20:22 +0200 |
commit | 2fb507b849e2b0f86c2480bde95200f8ae30803d (patch) | |
tree | 11405d294f3d61c57eb0807d37be18adfdd92dea /src/reflect | |
parent | f7efb9505c0ede48a67fb6a256604ba3ca02bcb3 (diff) | |
download | scala-2fb507b849e2b0f86c2480bde95200f8ae30803d.tar.gz scala-2fb507b849e2b0f86c2480bde95200f8ae30803d.tar.bz2 scala-2fb507b849e2b0f86c2480bde95200f8ae30803d.zip |
SI-6277 fixes flags, annotations & privateWithin
`Symbol.getFlag`, 'Symbol.hasFlag`, `Symbol.hasAllFlags`, `Symbol.annotations`
and `Symbol.privateWithin` now trigger automatic initialization of symbols
if they are used in a runtime reflection universe and some other conditions
are met (see `Symbol.needsInitialize` for details).
As the performance testing in https://github.com/scala/scala/pull/1380 shows,
this commit introduces a ~2% performance regression of compilation speed.
Unfortunately all known solutions to the bug at hand (A, B & C - all of those)
introduce perf regressions (see the pull request linked above for details).
However we're under severe time pressure, so there's no more time to explore.
Therefore I suggest this is reasonable to accept this performance hit,
because we've just gained 6% from removing scala.reflect.base,
and even before that we were well within our performance goal for 2.10.0-final.
Diffstat (limited to 'src/reflect')
-rw-r--r-- | src/reflect/scala/reflect/api/Symbols.scala | 17 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Flags.scala | 6 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Symbols.scala | 89 | ||||
-rw-r--r-- | src/reflect/scala/reflect/runtime/SymbolTable.scala | 28 |
4 files changed, 112 insertions, 28 deletions
diff --git a/src/reflect/scala/reflect/api/Symbols.scala b/src/reflect/scala/reflect/api/Symbols.scala index 78345e27f2..d8f955ddf3 100644 --- a/src/reflect/scala/reflect/api/Symbols.scala +++ b/src/reflect/scala/reflect/api/Symbols.scala @@ -214,22 +214,7 @@ trait Symbols { self: Universe => /** A list of annotations attached to this Symbol. */ - // we cannot expose the `annotations` method because it doesn't auto-initialize a symbol (see SI-5423) - // there was an idea to use the `isCompilerUniverse` flag and auto-initialize symbols in `annotations` whenever this flag is false - // but it doesn't work, because the unpickler (that is shared between reflective universes and global universes) is very picky about initialization - // scala.reflect.internal.Types$TypeError: bad reference while unpickling scala.collection.immutable.Nil: type Nothing not found in scala.type not found. - // at scala.reflect.internal.pickling.UnPickler$Scan.toTypeError(UnPickler.scala:836) - // at scala.reflect.internal.pickling.UnPickler$Scan$LazyTypeRef.complete(UnPickler.scala:849) // auto-initialize goes boom - // at scala.reflect.internal.Symbols$Symbol.info(Symbols.scala:1140) - // at scala.reflect.internal.Symbols$Symbol.initialize(Symbols.scala:1272) // this triggers auto-initialize - // at scala.reflect.internal.Symbols$Symbol.annotations(Symbols.scala:1438) // unpickler first tries to get pre-existing annotations - // at scala.reflect.internal.Symbols$Symbol.addAnnotation(Symbols.scala:1458) // unpickler tries to add the annotation being read - // at scala.reflect.internal.pickling.UnPickler$Scan.readSymbolAnnotation(UnPickler.scala:489) // unpickler detects an annotation - // at scala.reflect.internal.pickling.UnPickler$Scan.run(UnPickler.scala:88) - // at scala.reflect.internal.pickling.UnPickler.unpickle(UnPickler.scala:37) - // at scala.reflect.runtime.JavaMirrors$JavaMirror.unpickleClass(JavaMirrors.scala:253) // unpickle from within a reflexive mirror - // def annotations: List[Annotation] - def getAnnotations: List[Annotation] + def annotations: List[Annotation] /** For a class: the module or case class factory with the same name in the same package. * For a module: the class with the same name in the same package. diff --git a/src/reflect/scala/reflect/internal/Flags.scala b/src/reflect/scala/reflect/internal/Flags.scala index 0eb839479a..bb454b1df7 100644 --- a/src/reflect/scala/reflect/internal/Flags.scala +++ b/src/reflect/scala/reflect/internal/Flags.scala @@ -296,6 +296,12 @@ class Flags extends ModifierFlags { /** These flags are pickled */ final val PickledFlags = InitialFlags & ~FlagsNotPickled + /** If we have a top-level class or module + * and someone asks us for a flag not in TopLevelPickledFlags, + * then we don't need unpickling to give a definite answer. + */ + final val TopLevelPickledFlags = PickledFlags & ~(MODULE | METHOD | PACKAGE | PARAM | EXISTENTIAL) + def getterFlags(fieldFlags: Long): Long = ACCESSOR + ( if ((fieldFlags & MUTABLE) != 0) fieldFlags & ~MUTABLE & ~PRESUPER else fieldFlags & ~PRESUPER | STABLE diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index e8c1d4ed12..647a3c631b 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -55,6 +55,16 @@ trait Symbols extends api.Symbols { self: SymbolTable => def newFreeTypeSymbol(name: TypeName, flags: Long = 0L, origin: String): FreeTypeSymbol = new FreeTypeSymbol(name, origin) initFlags flags + /** Determines whether the given information request should trigger the given symbol's completer. + * See comments to `Symbol.needsInitialize` for details. + */ + protected def shouldTriggerCompleter(symbol: Symbol, completer: Type, isFlagRelated: Boolean, mask: Long) = + completer match { + case null => false + case _: FlagAgnosticCompleter => !isFlagRelated + case _ => abort(s"unsupported completer: $completer of class ${if (completer != null) completer.getClass else null} for symbol ${symbol.fullName}") + } + /** The original owner of a class. Used by the backend to generate * EnclosingMethod attributes. */ @@ -88,7 +98,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => def toTypeIn(site: Type): Type = site.memberType(this) def toTypeConstructor: Type = typeConstructor def setTypeSignature(tpe: Type): this.type = { setInfo(tpe); this } - def getAnnotations: List[AnnotationInfo] = { initialize; annotations } def setAnnotations(annots: AnnotationInfo*): this.type = { setAnnotations(annots.toList); this } def getter: Symbol = getter(owner) @@ -218,9 +227,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => val m = newModuleSymbol(clazz.name.toTermName, clazz.pos, MODULE | newFlags) connectModuleToClass(m, clazz.asInstanceOf[ClassSymbol]) } - final def newModule(name: TermName, pos: Position = NoPosition, newFlags: Long = 0L): ModuleSymbol = { - val m = newModuleSymbol(name, pos, newFlags | MODULE) - val clazz = newModuleClass(name.toTypeName, pos, m getFlag ModuleToClassFlags) + final def newModule(name: TermName, pos: Position = NoPosition, newFlags0: Long = 0L): ModuleSymbol = { + val newFlags = newFlags0 | MODULE + val m = newModuleSymbol(name, pos, newFlags) + val clazz = newModuleClass(name.toTypeName, pos, newFlags & ModuleToClassFlags) connectModuleToClass(m, clazz) } @@ -238,9 +248,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => final def newModuleSymbol(name: TermName, pos: Position = NoPosition, newFlags: Long = 0L): ModuleSymbol = newTermSymbol(name, pos, newFlags).asInstanceOf[ModuleSymbol] - final def newModuleAndClassSymbol(name: Name, pos: Position, flags: FlagSet): (ModuleSymbol, ClassSymbol) = { - val m = newModuleSymbol(name, pos, flags | MODULE) - val c = newModuleClass(name.toTypeName, pos, m getFlag ModuleToClassFlags) + final def newModuleAndClassSymbol(name: Name, pos: Position, flags0: FlagSet): (ModuleSymbol, ClassSymbol) = { + val flags = flags0 | MODULE + val m = newModuleSymbol(name, pos, flags) + val c = newModuleClass(name.toTypeName, pos, flags & ModuleToClassFlags) connectModuleToClass(m, c) (m, c) } @@ -583,11 +594,20 @@ trait Symbols extends api.Symbols { self: SymbolTable => && owner.isPackageClass && nme.isReplWrapperName(name) ) - final def getFlag(mask: Long): Long = flags & mask + final def getFlag(mask: Long): Long = { + if (!isCompilerUniverse && needsInitialize(isFlagRelated = true, mask = mask)) initialize + flags & mask + } /** Does symbol have ANY flag in `mask` set? */ - final def hasFlag(mask: Long): Boolean = (flags & mask) != 0 + final def hasFlag(mask: Long): Boolean = { + if (!isCompilerUniverse && needsInitialize(isFlagRelated = true, mask = mask)) initialize + (flags & mask) != 0 + } /** Does symbol have ALL the flags in `mask` set? */ - final def hasAllFlags(mask: Long): Boolean = (flags & mask) == mask + final def hasAllFlags(mask: Long): Boolean = { + if (!isCompilerUniverse && needsInitialize(isFlagRelated = true, mask = mask)) initialize + (flags & mask) == mask + } def setFlag(mask: Long): this.type = { _rawflags |= mask ; this } def resetFlag(mask: Long): this.type = { _rawflags &= ~mask ; this } @@ -1136,7 +1156,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => /** See comment in HasFlags for how privateWithin combines with flags. */ private[this] var _privateWithin: Symbol = _ - def privateWithin = _privateWithin + def privateWithin = { + if (!isCompilerUniverse && needsInitialize(isFlagRelated = false, mask = 0)) initialize + _privateWithin + } def privateWithin_=(sym: Symbol) { _privateWithin = sym } def setPrivateWithin(sym: Symbol): this.type = { privateWithin_=(sym) ; this } @@ -1327,6 +1350,46 @@ trait Symbols extends api.Symbols { self: SymbolTable => this } + /** Called when the programmer requests information that might require initialization of the underlying symbol. + * + * `isFlagRelated` and `mask` describe the nature of this information. + * isFlagRelated = true means that the programmer needs particular bits in flags. + * isFlagRelated = false means that the request is unrelated to flags (annotations or privateWithin). + * + * In our current architecture, symbols for top-level classes and modules + * are created as dummies. Package symbols just call newClass(name) or newModule(name) and + * consider their job done. + * + * In order for such a dummy to provide meaningful info (e.g. a list of its members), + * it needs to go through unpickling. Unpickling is a process of reading Scala metadata + * from ScalaSignature annotations and assigning it to symbols and types. + * + * A single unpickling session takes a top-level class or module, parses the ScalaSignature annotation + * and then reads metadata for the unpicklee, its companion (if any) and all their members recursively + * (i.e. the pickle not only contains info about directly nested classes/modules, but also about + * classes/modules nested into those and so on). + * + * Unpickling is triggered automatically whenever typeSignature (info in compiler parlance) is called. + * This happens because package symbols assign completer thunks to the dummies they create. + * Therefore metadata loading happens lazily and transparently. + * + * Almost transparently. Unfortunately metadata isn't limited to just signatures (i.e. lists of members). + * It also includes flags (which determine e.g. whether a class is sealed or not), annotations and privateWithin. + * This gives rise to unpleasant effects like in SI-6277, when a flag test called on an uninitialize symbol + * produces incorrect results. + * + * One might think that the solution is simple: automatically call the completer whenever one needs + * flags, annotations and privateWithin - just like it's done for typeSignature. Unfortunately, this + * leads to weird crashes in scalac, and currently we can't attempt to fix the core of the compiler + * risk stability a few weeks before the final release. + * + * However we do need to fix this for runtime reflection, since it's not something we'd like to + * expose to reflection users. Therefore a proposed solution is to check whether we're in a + * runtime reflection universe and if yes then to commence initialization. + */ + protected def needsInitialize(isFlagRelated: Boolean, mask: Long) = + !isInitialized && (flags & LOCKED) == 0 && shouldTriggerCompleter(this, if (infos ne null) infos.info else null, isFlagRelated, mask) + /** Was symbol's type updated during given phase? */ final def isUpdatedAt(pid: Phase#Id): Boolean = { assert(isCompilerUniverse) @@ -1489,8 +1552,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => /** After the typer phase (before, look at the definition's Modifiers), contains * the annotations attached to member a definition (class, method, type, field). */ - def annotations: List[AnnotationInfo] = + def annotations: List[AnnotationInfo] = { + if (!isCompilerUniverse && needsInitialize(isFlagRelated = false, mask = 0)) initialize _annotations + } def setAnnotations(annots: List[AnnotationInfo]): this.type = { _annotations = annots diff --git a/src/reflect/scala/reflect/runtime/SymbolTable.scala b/src/reflect/scala/reflect/runtime/SymbolTable.scala index 5b9090dae5..73632be965 100644 --- a/src/reflect/scala/reflect/runtime/SymbolTable.scala +++ b/src/reflect/scala/reflect/runtime/SymbolTable.scala @@ -1,6 +1,8 @@ package scala.reflect package runtime +import scala.reflect.internal.Flags._ + /** * This symbol table trait fills in the definitions so that class information is obtained by refection. * It can be used either from a reflexive universe (class scala.reflect.runtime.JavaUniverse), or else from @@ -14,4 +16,30 @@ trait SymbolTable extends internal.SymbolTable with JavaMirrors with SymbolLoade def debugInfo(msg: => String) = if (settings.debug.value) info(msg) + /** Declares that this is a runtime reflection universe. + * + * This means that we can make certain assumptions to optimize the universe. + * For example, we may auto-initialize symbols on flag and annotation requests + * (see `shouldTriggerCompleter` below for more details). + * + * On the other hand, this also means that usage scenarios of the universe + * will differ from the conventional ones. For example, we have to do additional cleanup + * in order to prevent memory leaks: http://groups.google.com/group/scala-internals/browse_thread/thread/eabcf3d406dab8b2. + */ + override def isCompilerUniverse = false + + /** Unlike compiler universes, reflective universes can auto-initialize symbols on flag requests. + * + * scalac wasn't designed with such auto-initialization in mind, and quite often it makes assumptions + * that flag requests won't cause initialization. Therefore enabling auto-init leads to cyclic errors. + * We could probably fix those, but at the moment it's too risky. + * + * Reflective universes share codebase with scalac, but their surface is much smaller, which means less assumptions. + * These assumptions are taken care of in this overriden `shouldTriggerCompleter` method. + */ + override protected def shouldTriggerCompleter(symbol: Symbol, completer: Type, isFlagRelated: Boolean, mask: Long) = + completer match { + case _: TopClassCompleter | _: JavaClassCompleter => !isFlagRelated || (mask & TopLevelPickledFlags) != 0 + case _ => super.shouldTriggerCompleter(symbol, completer, isFlagRelated, mask) + } } |