Preserve order of decls through pickle/unpickle (scala#6285)

Preserve order of decls through pickle/unpickle

The pickle format does not explicitly encode the order of decls.
Instead, symbols are entered into an index in the order that they 
are found by the pickler, either as a definition or as a reference. 
During unpickling, symbols are read and entered into the owner's 
decls in that order.

This is a cause of unstable compiler output: a class that mixes
in members from some trait will have a different order if it is
compiled jointly with / separately from that trait.

This commit modifies the pickler with an initial pass that reserves
index entries for all declarations in the declaration order.

The pickle format and the unpickler are unchanged.

Also tighten up the definition of "companion"-s so that
we no longer add a type alias in a package object into
the pickle of a same-named module.

Finally, scalap should not print class type param annots in decls
The annotation removed in this diff was actually from `R`!

Author: retronym

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

@@ -35,24 +35,30 @@ abstract class Pickler extends SubComponent {
   class PicklePhase(prev: Phase) extends StdPhase(prev) {
     def apply(unit: CompilationUnit): Unit = {
       def pickle(tree: Tree): Unit = {
-        def add(sym: Symbol, pickle: Pickle) = {
-          if (currentRun.compiles(sym) && !currentRun.symData.contains(sym)) {
-            debuglog("pickling " + sym)
-            pickle putSymbol sym
-            currentRun.symData(sym) = pickle
-          }
-        }
-
         tree match {
           case PackageDef(_, stats) =>
             stats foreach pickle
           case ClassDef(_, _, _, _) | ModuleDef(_, _, _) =>
             val sym = tree.symbol
-            val pickle = new Pickle(sym)
-            add(sym, pickle)
-            add(sym.companionSymbol, pickle)
-            pickle.writeArray()
-            currentRun registerPickle sym
+            def shouldPickle(sym: Symbol) = currentRun.compiles(sym) && !currentRun.symData.contains(sym)
+            if (shouldPickle(sym)) {
+              val pickle = new Pickle(sym)
+              def reserveDeclEntries(sym: Symbol): Unit = {
+                pickle.reserveEntry(sym)
+                if (sym.isClass) sym.info.decls.foreach(reserveDeclEntries)
+                else if (sym.isModule) reserveDeclEntries(sym.moduleClass)
+              }
+
+              val companion = sym.companionSymbol.filter(_.owner == sym.owner) // exclude companionship between package- and package object-owned symbols.
+              val syms = sym :: (if (shouldPickle(companion)) companion :: Nil else Nil)
+              syms.foreach(reserveDeclEntries)
+              syms.foreach { sym =>
+                pickle.putSymbol(sym)
+                currentRun.symData(sym) = pickle
+              }
+              pickle.writeArray()
+              currentRun registerPickle sym
+            }
           case _ =>
         }
       }
@@ -121,14 +127,20 @@ abstract class Pickler extends SubComponent {
     private def isExternalSymbol(sym: Symbol): Boolean = (sym != NoSymbol) && !isLocalToPickle(sym)
 
     // Phase 1 methods: Populate entries/index ------------------------------------
+    private val reserved = mutable.BitSet()
+    final def reserveEntry(sym: Symbol): Unit = {
+      reserved(ep) = true
+      putEntry(sym)
+    }
 
     /** Store entry e in index at next available position unless
      *  it is already there.
      *
      *  @return      true iff entry is new.
      */
     private def putEntry(entry: AnyRef): Boolean = index.get(entry) match {
-      case Some(_) => false
+      case Some(i) =>
+        reserved.remove(i)
       case None =>
         if (ep == entries.length) {
           val entries1 = new Array[AnyRef](ep * 2)

src/scalap/scala/tools/scalap/scalax/rules/scalasig/ScalaSigPrinter.scala

@@ -60,7 +60,7 @@ class ScalaSigPrinter(stream: PrintStream, printPrivates: Boolean) {
         case a: AliasSymbol =>
           indent
           printAlias(level, a)
-        case t: TypeSymbol if !t.isParam && !t.name.matches("_\\$\\d+")=>
+        case t: TypeSymbol if !t.name.matches("_\\$\\d+")=>
           indent
           printTypeSymbol(level, t)
         case s =>

src/scalap/scala/tools/scalap/scalax/rules/scalasig/Symbol.scala

@@ -27,7 +27,7 @@ abstract class ScalaSigSymbol extends Symbol {
   def entry: ScalaSig#Entry
   def index = entry.index
 
-  lazy val children: Seq[Symbol] = applyScalaSigRule(ScalaSigParsers.symbols) filter (_.parent == Some(this))
+  lazy val children: Seq[Symbol] = applyScalaSigRule(ScalaSigParsers.symbols) filter (sym => sym.parent == Some(this) && !sym.isParam)
   lazy val attributes: Seq[AttributeInfo] = applyScalaSigRule(ScalaSigParsers.attributes) filter (_.symbol == this)
 }
 

test/files/scalap/typeAnnotations.check

@@ -1,6 +1,5 @@
 abstract class TypeAnnotations[@scala.specialized R] extends scala.AnyRef {
   def this() = { /* compiled code */ }
-  @scala.specialized
   val x: scala.Int = { /* compiled code */ }
   @scala.specialized
   type T

test/junit/scala/tools/nsc/symtab/classfile/PicklerTest.scala

@@ -0,0 +1,40 @@
+package scala.tools.nsc.symtab.classfile
+
+import org.junit.{Assert, Test}
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+import scala.reflect.io.VirtualDirectory
+import scala.tools.nsc.Global
+import scala.tools.nsc.classpath.{AggregateClassPath, VirtualDirectoryClassPath}
+import scala.tools.testing.BytecodeTesting
+
+@RunWith(classOf[JUnit4])
+class PicklerTest extends BytecodeTesting {
+  @Test
+  def pickleUnpicklePreserveDeclOrder(): Unit = {
+    assertStableDecls("package p1; trait C { def x: T; def y: Int; class T }", "p1.C")
+    assertStableDecls("package p1; class D; object D { def x: T = null; def y: Int = 0; class T }", "p1.D")
+  }
+
+  def assertStableDecls(source: String, name: String): Unit = {
+    val compiler1 = BytecodeTesting.newCompiler(extraArgs = compilerArgs)
+    val r = new compiler1.global.Run
+    r.compileSources(compiler1.global.newSourceFile(source) :: Nil)
+    val compiler2 = BytecodeTesting.newCompiler(extraArgs = compilerArgs)
+    val out = compiler1.global.settings.outputDirs.getSingleOutput.get.asInstanceOf[VirtualDirectory]
+    def showDecls(global: Global): Seq[String] = global.exitingPickler {
+      val classSym = global.rootMirror.getClassIfDefined(name)
+      val moduleSym = global.rootMirror.getModuleIfDefined(name).moduleClass
+      val syms = List(classSym, moduleSym).filter(sym => sym.exists)
+      Assert.assertTrue(syms.nonEmpty)
+      syms.flatMap(sym => sym.name.toString :: sym.info.decls.toList.map(decl => global.definitions.fullyInitializeSymbol(decl).defString))
+    }
+    val decls1 = showDecls(compiler1.global)
+    compiler2.global.classPath
+    compiler2.global.platform.currentClassPath = Some(AggregateClassPath(new VirtualDirectoryClassPath(out) :: compiler2.global.platform.currentClassPath.get :: Nil))
+    new compiler2.global.Run
+    val decls2 = showDecls(compiler2.global)
+    Assert.assertEquals(decls1, decls2)
+  }
+}