Classfile parser and unpickler require class and module symbol arguments

In SymbolLoaders, when seeing a classfile `Foo.class`, we always
(unconditionally) create 3 symbols: a class, a module and a module
class. Some symbols get invalidated later (`.exists`).

Until now, the classfile parser (and unpickler) received the "root"
symbol as argument, which is the symbol whose type is being completed.
This is either the class symbol or the module symbol.

The classfile parser would then try to lookup the other symbol through
`root.companionClass` or `root.companionModule`. Howver, this lookup can
fail. One example is scala-dev#248: when a type alias (in a package
object) shadows a class symbol, `companionClass` will fail.

The implementations of the  classfile parser / unpickler assume that
both the `clazz` and the `staticModule` symbols are available. This
change makes sure that they are always passed in explicitly.

Before this patch, in the example of scala-dev#248, the `classRoot` of
the unpickler was NoSymbol. This caused a bug when unpickling the
module class symbol, causing a second module class symbol to be created
mistakingly. The next commit cleans up this logic, more details there.
This second symbol would then cause the crash in the backend because it
doesn't have an `associatedFile`, therefore `isCoDefinedWith` would
spuriously return `true`.

3 files changed, 34 insertions, 32 deletions

diff --git a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala
index b36d5d4ef1..eb01c8dc44 100644
--- a/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala
+++ b/src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala
@@ -52,20 +52,28 @@ abstract class SymbolLoaders {
     })
   }
 
+  def newClass(owner: Symbol, name: String): Symbol = owner.newClass(newTypeName(name))
+
   /** Enter class with given `name` into scope of `root`
    *  and give them `completer` as type.
    */
-  def enterClass(owner: Symbol, name: String, completer: SymbolLoader): Symbol = {
-    val clazz = owner.newClass(newTypeName(name))
+  def enterClass(owner: Symbol, name: String, completer: SymbolLoader): Symbol =
+    enterClass(owner, newClass(owner, name), completer)
+
+  def enterClass(owner: Symbol, clazz: Symbol, completer: SymbolLoader): Symbol = {
     clazz setInfo completer
     enterIfNew(owner, clazz, completer)
   }
 
+  def newModule(owner: Symbol, name: String): Symbol = owner.newModule(newTermName(name))
+
   /** Enter module with given `name` into scope of `root`
    *  and give them `completer` as type.
    */
-  def enterModule(owner: Symbol, name: String, completer: SymbolLoader): Symbol = {
-    val module = owner.newModule(newTermName(name))
+  def enterModule(owner: Symbol, name: String, completer: SymbolLoader): Symbol =
+    enterModule(owner, newModule(owner, name), completer)
+
+  def enterModule(owner: Symbol, module: Symbol, completer: SymbolLoader): Symbol = {
     module setInfo completer
     module.moduleClass setInfo moduleClassLoader
     enterIfNew(owner, module, completer)
@@ -113,9 +121,12 @@ abstract class SymbolLoaders {
   /** Enter class and module with given `name` into scope of `root`
    *  and give them `completer` as type.
    */
-  def enterClassAndModule(root: Symbol, name: String, completer: SymbolLoader) {
-    val clazz = enterClass(root, name, completer)
-    val module = enterModule(root, name, completer)
+  def enterClassAndModule(root: Symbol, name: String, getCompleter: (Symbol, Symbol) => SymbolLoader) {
+    val clazz = newClass(root, name)
+    val module = newModule(root, name)
+    val completer = getCompleter(clazz, module)
+    enterClass(root, clazz, completer)
+    enterModule(root, module, completer)
     if (!clazz.isAnonymousClass) {
       // Diagnostic for SI-7147
       def msg: String = {
@@ -136,7 +147,7 @@ abstract class SymbolLoaders {
    *  (overridden in interactive.Global).
    */
   def enterToplevelsFromSource(root: Symbol, name: String, src: AbstractFile) {
-    enterClassAndModule(root, name, new SourcefileLoader(src))
+    enterClassAndModule(root, name, (_, _) => new SourcefileLoader(src))
   }
 
   /** The package objects of scala and scala.reflect should always
@@ -162,17 +173,10 @@ abstract class SymbolLoaders {
         if (settings.verbose) inform("[symloader] no class, picked up source file for " + src.path)
         enterToplevelsFromSource(owner, classRep.name, src)
       case (Some(bin), _) =>
-        enterClassAndModule(owner, classRep.name, newClassLoader(bin))
+        enterClassAndModule(owner, classRep.name, new ClassfileLoader(bin, _, _))
     }
   }
 
-  /** Create a new loader from a binary classfile.
-   *  This is intended as a hook allowing to support loading symbols from
-   *  files other than .class files.
-   */
-  protected def newClassLoader(bin: AbstractFile): SymbolLoader =
-    new ClassfileLoader(bin)
-
   /**
    * A lazy type that completes itself by calling parameter doComplete.
    * Any linked modules/classes or module classes are also initialized.
@@ -277,7 +281,7 @@ abstract class SymbolLoaders {
     }
   }
 
-  class ClassfileLoader(val classfile: AbstractFile) extends SymbolLoader with FlagAssigningCompleter {
+  class ClassfileLoader(val classfile: AbstractFile, clazz: Symbol, module: Symbol) extends SymbolLoader with FlagAssigningCompleter {
     private object classfileParser extends {
       val symbolTable: SymbolLoaders.this.symbolTable.type = SymbolLoaders.this.symbolTable
     } with ClassfileParser {
@@ -309,7 +313,7 @@ abstract class SymbolLoaders {
       // 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))
+      enteringPhase(phaseBeforeRefchecks)(classfileParser.parse(classfile, clazz, module))
 
       if (root.associatedFile eq NoAbstractFile) {
         root match {
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
index e69c9f816a..6d5b697925 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -132,17 +132,12 @@ abstract class ClassfileParser {
     finally loaders.parentsLevel -= 1
   }
 
-  def parse(file: AbstractFile, root: Symbol): Unit = {
-    debuglog("[class] >> " + root.fullName)
-
+  def parse(file: AbstractFile, clazz: Symbol, module: Symbol): Unit = {
     this.file = file
-    pushBusy(root) {
+    pushBusy(clazz) {
       this.in           = new AbstractFileReader(file)
-      this.clazz        = if (root.isModule) root.companionClass else root
-      // WARNING! do no use clazz.companionModule to find staticModule.
-      // In a situation where root can be defined, but its companionClass not,
-      // this would give incorrect results (see SI-5031 in separate compilation scenario)
-      this.staticModule = if (root.isModule) root else root.companionModule
+      this.clazz        = clazz
+      this.staticModule = module
       this.isScala      = false
 
       parseHeader()
@@ -1020,7 +1015,6 @@ abstract class ClassfileParser {
 
     def enterClassAndModule(entry: InnerClassEntry, file: AbstractFile) {
       def jflags      = entry.jflags
-      val completer   = new loaders.ClassfileLoader(file)
       val name        = entry.originalName
       val sflags      = jflags.toScalaFlags
       val owner       = ownerForFlags(jflags)
@@ -1031,8 +1025,11 @@ abstract class ClassfileParser {
       val (innerClass, innerModule) = if (file == NoAbstractFile) {
         (newStub(name.toTypeName), newStub(name.toTermName))
       } else {
-        val cls = owner.newClass(name.toTypeName, NoPosition, sflags) setInfo completer
-        val mod = owner.newModule(name.toTermName, NoPosition, sflags) setInfo completer
+        val cls = owner.newClass(name.toTypeName, NoPosition, sflags)
+        val mod = owner.newModule(name.toTermName, NoPosition, sflags)
+        val completer = new loaders.ClassfileLoader(file, cls, mod)
+        cls setInfo completer
+        mod setInfo completer
         mod.moduleClass setInfo loaders.moduleClassLoader
         List(cls, mod.moduleClass) foreach (_.associatedFile = file)
         (cls, mod)
diff --git a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
index c6cb0d0223..3a80ee2ccf 100644
--- a/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
+++ b/src/reflect/scala/reflect/internal/pickling/UnPickler.scala
@@ -29,12 +29,13 @@ abstract class UnPickler {
    *  from an array of bytes.
    *  @param bytes      bytearray from which we unpickle
    *  @param offset     offset from which unpickling starts
-   *  @param classRoot  the top-level class which is unpickled, or NoSymbol if inapplicable
-   *  @param moduleRoot the top-level module which is unpickled, or NoSymbol if inapplicable
+   *  @param classRoot  the top-level class which is unpickled
+   *  @param moduleRoot the top-level module which is unpickled
    *  @param filename   filename associated with bytearray, only used for error messages
    */
   def unpickle(bytes: Array[Byte], offset: Int, classRoot: Symbol, moduleRoot: Symbol, filename: String) {
     try {
+      assert(classRoot != NoSymbol && moduleRoot != NoSymbol, s"The Unpickler expects a class and module symbol: $classRoot - $moduleRoot")
       new Scan(bytes, offset, classRoot, moduleRoot, filename).run()
     } catch {
       case ex: IOException =>