Merge pull request scala#1380 from scalamacros/ticket/6277

SI-6277 fix for isXXX methods in reflection

Author: xeno-by

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

@@ -219,7 +219,7 @@ abstract class SymbolLoaders {
   /**
    * Load contents of a package
    */
-  class PackageLoader(classpath: ClassPath[platform.BinaryRepr]) extends SymbolLoader {
+  class PackageLoader(classpath: ClassPath[platform.BinaryRepr]) extends SymbolLoader with FlagAgnosticCompleter {
     protected def description = "package loader "+ classpath.name
 
     protected def doComplete(root: Symbol) {
@@ -242,7 +242,7 @@ abstract class SymbolLoaders {
     }
   }
 
-  class ClassfileLoader(val classfile: AbstractFile) extends SymbolLoader {
+  class ClassfileLoader(val classfile: AbstractFile) extends SymbolLoader with FlagAssigningCompleter {
     private object classfileParser extends ClassfileParser {
       val global: SymbolLoaders.this.global.type = SymbolLoaders.this.global
     }
@@ -267,7 +267,7 @@ abstract class SymbolLoaders {
     override def sourcefile: Option[AbstractFile] = classfileParser.srcfile
   }
 
-  class MsilFileLoader(msilFile: MsilFile) extends SymbolLoader {
+  class MsilFileLoader(msilFile: MsilFile) extends SymbolLoader with FlagAssigningCompleter {
     private def typ = msilFile.msilType
     private object typeParser extends clr.TypeParser {
       val global: SymbolLoaders.this.global.type = SymbolLoaders.this.global
@@ -277,14 +277,14 @@ abstract class SymbolLoaders {
     protected def doComplete(root: Symbol) { typeParser.parse(typ, root) }
   }
 
-  class SourcefileLoader(val srcfile: AbstractFile) extends SymbolLoader {
+  class SourcefileLoader(val srcfile: AbstractFile) extends SymbolLoader with FlagAssigningCompleter {
     protected def description = "source file "+ srcfile.toString
     override def fromSource = true
     override def sourcefile = Some(srcfile)
     protected def doComplete(root: Symbol): Unit = global.currentRun.compileLate(srcfile)
   }
 
-  object moduleClassLoader extends SymbolLoader {
+  object moduleClassLoader extends SymbolLoader with FlagAssigningCompleter {
     protected def description = "module class loader"
     protected def doComplete(root: Symbol) { root.sourceModule.initialize }
   }

src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala

@@ -844,7 +844,7 @@ abstract class ClassfileParser {
     GenPolyType(ownTypeParams, tpe)
   } // sigToType
 
-  class TypeParamsType(override val typeParams: List[Symbol]) extends LazyType {
+  class TypeParamsType(override val typeParams: List[Symbol]) extends LazyType with FlagAgnosticCompleter {
     override def complete(sym: Symbol) { throw new AssertionError("cyclic type dereferencing") }
   }
 
@@ -1228,7 +1228,7 @@ abstract class ClassfileParser {
     }
   }
 
-  class LazyAliasType(alias: Symbol) extends LazyType {
+  class LazyAliasType(alias: Symbol) extends LazyType with FlagAgnosticCompleter {
     override def complete(sym: Symbol) {
       sym setInfo createFromClonedSymbols(alias.initialize.typeParams, alias.tpe)(typeFun)
     }

src/compiler/scala/tools/nsc/symtab/clr/TypeParser.scala

@@ -64,7 +64,7 @@ abstract class TypeParser {
     busy = false
   }
 
-  class TypeParamsType(override val typeParams: List[Symbol]) extends LazyType {
+  class TypeParamsType(override val typeParams: List[Symbol]) extends LazyType with FlagAgnosticCompleter {
     override def complete(sym: Symbol) { throw new AssertionError("cyclic type dereferencing") }
   }
 

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

@@ -132,7 +132,7 @@ abstract class AddInterfaces extends InfoTransform { self: Erasure =>
    *  - for every interface member of iface: its implementation method, if one is needed
    *  - every former member of iface that is implementation only
    */
-  private class LazyImplClassType(iface: Symbol) extends LazyType {
+  private class LazyImplClassType(iface: Symbol) extends LazyType with FlagAgnosticCompleter {
     /** Compute the decls of implementation class implClass,
      *  given the decls ifaceDecls of its interface.
      */

src/compiler/scala/tools/nsc/typechecker/Namers.scala

@@ -1506,7 +1506,7 @@ trait Namers extends MethodSynthesis {
 
   /** A class representing a lazy type with known type parameters.
    */
-  class PolyTypeCompleter(tparams: List[TypeDef], restp: TypeCompleter, owner: Tree, ctx: Context) extends LockingTypeCompleter {
+  class PolyTypeCompleter(tparams: List[TypeDef], restp: TypeCompleter, owner: Tree, ctx: Context) extends LockingTypeCompleter with FlagAgnosticCompleter {
     private val ownerSym    = owner.symbol
     override val typeParams = tparams map (_.symbol) //@M
     override val tree       = restp.tree

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.

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

src/reflect/scala/reflect/internal/HasFlags.scala

@@ -99,6 +99,7 @@ trait HasFlags {
   def isLabel            = hasAllFlags(LABEL | METHOD) && !hasAccessorFlag
   def isLazy             = hasFlag(LAZY)
   def isLifted           = hasFlag(LIFTED)
+  def isMacro            = hasFlag(MACRO)
   def isMutable          = hasFlag(MUTABLE)
   def isOverride         = hasFlag(OVERRIDE)
   def isParamAccessor    = hasFlag(PARAMACCESSOR)
@@ -109,6 +110,7 @@ trait HasFlags {
   def isProtectedLocal   = hasAllFlags(ProtectedLocal)
   def isPublic           = hasNoFlags(PRIVATE | PROTECTED) && !hasAccessBoundary
   def isSealed           = hasFlag(SEALED)
+  def isSpecialized      = hasFlag(SPECIALIZED)
   def isSuperAccessor    = hasFlag(SUPERACCESSOR)
   def isSynthetic        = hasFlag(SYNTHETIC)
   def isTrait            = hasFlag(TRAIT) && !hasFlag(PARAM)

src/reflect/scala/reflect/internal/Importers.scala

@@ -104,7 +104,7 @@ trait Importers extends api.Importers { self: SymbolTable =>
         mysym setFlag Flags.LOCKED
         mysym setInfo {
           val mytypeParams = sym.typeParams map importSymbol
-          new LazyPolyType(mytypeParams) {
+          new LazyPolyType(mytypeParams) with FlagAgnosticCompleter {
             override def complete(s: Symbol) {
               val result = sym.info match {
                 case from.PolyType(_, res) => res

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.
    */
@@ -67,7 +77,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     def isParamWithDefault: Boolean = this.hasDefault
     def isByNameParam: Boolean = this.isValueParameter && (this hasFlag BYNAMEPARAM)
     def isImplementationArtifact: Boolean = (this hasFlag BRIDGE) || (this hasFlag VBRIDGE) || (this hasFlag ARTIFACT)
-    def isJava: Boolean = this hasFlag JAVA
+    def isJava: Boolean = isJavaDefined
     def isVal: Boolean = isTerm && !isModule && !isMethod && !isMutable
     def isVar: Boolean = isTerm && !isModule && !isMethod && isMutable
 
@@ -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)
     }
@@ -489,14 +500,12 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     def isAliasType    = false
     def isAbstractType = false
     def isSkolem       = false
-    def isMacro        = this hasFlag MACRO
 
     /** A Type, but not a Class. */
     def isNonClassType = false
 
     /** The bottom classes are Nothing and Null, found in Definitions. */
     def isBottomClass  = false
-    def isSpecialized = this hasFlag SPECIALIZED
 
     /** These are all tests for varieties of ClassSymbol, which has these subclasses:
      *  - ModuleClassSymbol
@@ -585,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 }
@@ -1138,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 }
 
@@ -1329,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)
@@ -1491,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

src/reflect/scala/reflect/internal/Types.scala

@@ -3445,6 +3445,16 @@ trait Types extends api.Types { self: SymbolTable =>
     override def kind = "LazyType"
   }
 
+  /** A marker trait representing an as-yet unevaluated type
+   *  which doesn't assign flags to the underlying symbol.
+   */
+  trait FlagAgnosticCompleter extends LazyType
+
+  /** A marker trait representing an as-yet unevaluated type
+   *  which assigns flags to the underlying symbol.
+   */
+  trait FlagAssigningCompleter extends LazyType
+
   abstract class LazyPolyType(override val typeParams: List[Symbol]) extends LazyType {
     override def safeToString =
       (if (typeParams.isEmpty) "" else typeParamsString(this)) + super.safeToString

src/reflect/scala/reflect/internal/pickling/UnPickler.scala

@@ -848,7 +848,7 @@ abstract class UnPickler /*extends scala.reflect.generic.UnPickler*/ {
     }
 
     /** A lazy type which when completed returns type at index `i`. */
-    private class LazyTypeRef(i: Int) extends LazyType {
+    private class LazyTypeRef(i: Int) extends LazyType with FlagAgnosticCompleter {
       private val definedAtRunId = currentRunId
       private val p = phase
       override def complete(sym: Symbol) : Unit = try {