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 | |
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.
-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 | ||||
-rw-r--r-- | test/files/run/existentials3-new.check | 8 | ||||
-rw-r--r-- | test/files/run/reflection-java-annotations.scala | 4 | ||||
-rw-r--r-- | test/files/run/reflection-magicsymbols-invoke.check | 4 | ||||
-rw-r--r-- | test/files/run/reify_ann1a.scala | 1 | ||||
-rw-r--r-- | test/files/run/reify_ann1b.scala | 1 | ||||
-rw-r--r-- | test/files/run/reify_ann2a.scala | 1 | ||||
-rw-r--r-- | test/files/run/reify_ann3.scala | 1 | ||||
-rw-r--r-- | test/files/run/reify_ann4.scala | 1 | ||||
-rw-r--r-- | test/files/run/reify_ann5.scala | 1 | ||||
-rw-r--r-- | test/files/run/t5423.scala | 2 | ||||
-rw-r--r-- | test/files/run/t6277.check | 1 | ||||
-rw-r--r-- | test/files/run/t6277.scala | 9 |
16 files changed, 131 insertions, 43 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) + } } diff --git a/test/files/run/existentials3-new.check b/test/files/run/existentials3-new.check index bb6fe1a5e3..00614b19db 100644 --- a/test/files/run/existentials3-new.check +++ b/test/files/run/existentials3-new.check @@ -3,8 +3,8 @@ Bar, t=TypeRef, s=type Bar Test.ToS, t=RefinedType, s=f3 Test.ToS, t=RefinedType, s=f4 Test.ToS, t=RefinedType, s=f5 -() => Test.ToS, t=TypeRef, s=class Function0 -() => Test.ToS, t=TypeRef, s=class Function0 +() => Test.ToS, t=TypeRef, s=trait Function0 +() => Test.ToS, t=TypeRef, s=trait Function0 $anon, t=TypeRef, s=type $anon $anon, t=TypeRef, s=type $anon List[java.lang.Object{type T1}#T1], t=TypeRef, s=class List @@ -15,8 +15,8 @@ Bar, t=TypeRef, s=type Bar Test.ToS, t=RefinedType, s=g3 Test.ToS, t=RefinedType, s=g4 Test.ToS, t=RefinedType, s=g5 -() => Test.ToS, t=TypeRef, s=class Function0 -() => Test.ToS, t=TypeRef, s=class Function0 +() => Test.ToS, t=TypeRef, s=trait Function0 +() => Test.ToS, t=TypeRef, s=trait Function0 $anon, t=TypeRef, s=type $anon $anon, t=TypeRef, s=type $anon List[java.lang.Object{type T1}#T1], t=TypeRef, s=class List diff --git a/test/files/run/reflection-java-annotations.scala b/test/files/run/reflection-java-annotations.scala index 0b16c0d103..2e3fed48ce 100644 --- a/test/files/run/reflection-java-annotations.scala +++ b/test/files/run/reflection-java-annotations.scala @@ -2,6 +2,6 @@ object Test extends App { import scala.reflect.runtime.universe._ val sym = typeOf[JavaAnnottee].typeSymbol sym.typeSignature - sym.getAnnotations foreach (_.javaArgs) - println(sym.getAnnotations) + sym.annotations foreach (_.javaArgs) + println(sym.annotations) }
\ No newline at end of file diff --git a/test/files/run/reflection-magicsymbols-invoke.check b/test/files/run/reflection-magicsymbols-invoke.check index 520dc2bfbe..f5258efeb7 100644 --- a/test/files/run/reflection-magicsymbols-invoke.check +++ b/test/files/run/reflection-magicsymbols-invoke.check @@ -90,7 +90,7 @@ method $asInstanceOf: [T0]()T0 method $isInstanceOf: [T0]()Boolean method ==: (x$1: Any)Boolean method ==: (x$1: AnyRef)Boolean -method apply: (i: <?>)T +method apply: (i: Int)T method asInstanceOf: [T0]=> T0 method clone: ()Array[T] method eq: (x$1: AnyRef)Boolean @@ -105,7 +105,7 @@ method notify: ()Unit method notifyAll: ()Unit method synchronized: [T0](x$1: T0)T0 method toString: ()java.lang.String -method update: (i: <?>, x: <?>)Unit +method update: (i: Int, x: T)Unit method wait: ()Unit method wait: (x$1: Long)Unit method wait: (x$1: Long, x$2: Int)Unit diff --git a/test/files/run/reify_ann1a.scala b/test/files/run/reify_ann1a.scala index 88b4191195..c23048e463 100644 --- a/test/files/run/reify_ann1a.scala +++ b/test/files/run/reify_ann1a.scala @@ -21,7 +21,6 @@ object Test extends App { // test 2: import and typecheck val toolbox = cm.mkToolBox() val ttree = toolbox.typeCheck(tree) - ttree.foreach(sub => if (sub.hasSymbol) sub.symbol.typeSignature) println(ttree.toString) // test 3: import and compile diff --git a/test/files/run/reify_ann1b.scala b/test/files/run/reify_ann1b.scala index a8fb876023..29ce6021a2 100644 --- a/test/files/run/reify_ann1b.scala +++ b/test/files/run/reify_ann1b.scala @@ -21,7 +21,6 @@ object Test extends App { // test 2: import and typecheck val toolbox = cm.mkToolBox() val ttree = toolbox.typeCheck(tree) - ttree.foreach(sub => if (sub.hasSymbol) sub.symbol.typeSignature) println(ttree.toString) // test 3: import and compile diff --git a/test/files/run/reify_ann2a.scala b/test/files/run/reify_ann2a.scala index b7e5833584..53423e12c3 100644 --- a/test/files/run/reify_ann2a.scala +++ b/test/files/run/reify_ann2a.scala @@ -21,7 +21,6 @@ object Test extends App { // test 2: import and typecheck val toolbox = cm.mkToolBox() val ttree = toolbox.typeCheck(tree) - ttree.foreach(sub => if (sub.hasSymbol) sub.symbol.typeSignature) println(ttree.toString) // test 3: import and compile diff --git a/test/files/run/reify_ann3.scala b/test/files/run/reify_ann3.scala index 662d58aaf3..4162fa532f 100644 --- a/test/files/run/reify_ann3.scala +++ b/test/files/run/reify_ann3.scala @@ -15,7 +15,6 @@ object Test extends App { // test 2: import and typecheck val toolbox = cm.mkToolBox() val ttree = toolbox.typeCheck(tree) - ttree.foreach(sub => if (sub.hasSymbol) sub.symbol.typeSignature) println(ttree.toString) // test 3: import and compile diff --git a/test/files/run/reify_ann4.scala b/test/files/run/reify_ann4.scala index a85e5e3625..0aedb77b5e 100644 --- a/test/files/run/reify_ann4.scala +++ b/test/files/run/reify_ann4.scala @@ -19,7 +19,6 @@ object Test extends App { // test 2: import and typecheck val toolbox = cm.mkToolBox() val ttree = toolbox.typeCheck(tree) - ttree.foreach(sub => if (sub.hasSymbol) sub.symbol.typeSignature) println(ttree.toString) // test 3: import and compile diff --git a/test/files/run/reify_ann5.scala b/test/files/run/reify_ann5.scala index 877360180c..d27be3b6d5 100644 --- a/test/files/run/reify_ann5.scala +++ b/test/files/run/reify_ann5.scala @@ -16,7 +16,6 @@ object Test extends App { // test 2: import and typecheck val toolbox = cm.mkToolBox() val ttree = toolbox.typeCheck(tree) - ttree.foreach(sub => if (sub.hasSymbol) sub.symbol.typeSignature) println(ttree.toString) // test 3: import and compile diff --git a/test/files/run/t5423.scala b/test/files/run/t5423.scala index 9b8ba090fa..c1632126b2 100644 --- a/test/files/run/t5423.scala +++ b/test/files/run/t5423.scala @@ -7,5 +7,5 @@ final class table extends annotation.StaticAnnotation object Test extends App { val s = cm.classSymbol(classOf[A]) - println(s.getAnnotations) + println(s.annotations) }
\ No newline at end of file diff --git a/test/files/run/t6277.check b/test/files/run/t6277.check new file mode 100644 index 0000000000..f32a5804e2 --- /dev/null +++ b/test/files/run/t6277.check @@ -0,0 +1 @@ +true
\ No newline at end of file diff --git a/test/files/run/t6277.scala b/test/files/run/t6277.scala new file mode 100644 index 0000000000..41feee8a8a --- /dev/null +++ b/test/files/run/t6277.scala @@ -0,0 +1,9 @@ +import scala.reflect.runtime.universe._ + +object Test extends App { + locally { + val sym = typeOf[List[_]].typeSymbol.asClass + val q = sym.isSealed + println(q) + } +}
\ No newline at end of file |