[backport] SI-9375 add synthetic readResolve only for static modules

For inner modules, the synthetic readResolve method would cause the module constructor to be invoked on de-serialization in certain situations. See the discussion in the ticket. Adds a comprehensive test around serializing and de-serializing modules.
scala · Sep 22, 2015 · 9a47e81 · 9a47e81
1 parent 0f72dd3
commit 9a47e81
Show file tree

Hide file tree

Showing 9 changed files with 371 additions and 9 deletions.
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -1182,11 +1182,23 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
     private def eliminateModuleDefs(moduleDef: Tree): List[Tree] = exitingRefchecks {
       val ModuleDef(_, _, impl) = moduleDef
       val module        = moduleDef.symbol
+      val moduleClass   = module.moduleClass
       val site          = module.owner
       val moduleName    = module.name.toTermName
       // The typer doesn't take kindly to seeing this ClassDef; we have to
       // set NoType so it will be ignored.
-      val cdef          = ClassDef(module.moduleClass, impl) setType NoType
+      val cdef          = ClassDef(moduleClass, impl) setType NoType
+
+      // This code is related to the fix of SI-9375, which stops adding `readResolve` methods to
+      // non-static (nested) modules. Before the fix, the method would cause the module accessor
+      // to become notPrivate. To prevent binary changes in the 2.11.x branch, we mimic that behavior.
+      // There is a bit of code duplication between here and SyntheticMethods. We cannot call
+      // makeNotPrivate already in SyntheticMethod: that is during type checking, and not all references
+      // are resolved yet, so we cannot rename a definition. This code doesn't exist in the 2.12.x branch.
+      def hasConcreteImpl(name: Name) = moduleClass.info.member(name).alternatives exists (m => !m.isDeferred)
+      val hadReadResolveBeforeSI9375 = moduleClass.isSerializable && !hasConcreteImpl(nme.readResolve)
+      if (hadReadResolveBeforeSI9375)
+        moduleClass.sourceModule.makeNotPrivate(moduleClass.sourceModule.owner)
 
       // Create the module var unless the immediate owner is a class and
       // the module var already exists there. See SI-5012, SI-6712.
@@ -1210,7 +1222,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
       }
       def matchingInnerObject() = {
         val newFlags = (module.flags | STABLE) & ~MODULE
-        val newInfo  = NullaryMethodType(module.moduleClass.tpe)
+        val newInfo  = NullaryMethodType(moduleClass.tpe)
         val accessor = site.newMethod(moduleName, module.pos, newFlags) setInfoAndEnter newInfo
 
         DefDef(accessor, Select(This(site), module)) :: Nil

diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala
@@ -322,6 +322,7 @@ trait SyntheticMethods extends ast.TreeDSL {
          clazz.isModuleClass
       && clazz.isSerializable
       && !hasConcreteImpl(nme.readResolve)
+      && clazz.isStatic
     )
 
     def synthesize(): List[Tree] = {

diff --git a/test/files/neg/t6666d.check b/test/files/neg/t6666d.check
diff --git a/test/files/neg/t6666d.scala → test/files/pos/t6666d.scala b/test/files/neg/t6666d.scala → test/files/pos/t6666d.scala
diff --git a/test/files/run/idempotency-case-classes.check b/test/files/run/idempotency-case-classes.check
@@ -47,8 +47,7 @@ C(2,3)
     case <synthetic> def unapply(x$0: C): Option[(Int, Int)] = if (x$0.==(null))
       scala.this.None
     else
-      Some.apply[(Int, Int)](scala.Tuple2.apply[Int, Int](x$0.x, x$0.y));
-    <synthetic> private def readResolve(): Object = C
+      Some.apply[(Int, Int)](scala.Tuple2.apply[Int, Int](x$0.x, x$0.y))
   };
   Predef.println(C.apply(2, 3))
 }

diff --git a/test/files/run/repl-serialization.check b/test/files/run/repl-serialization.check
@@ -20,6 +20,5 @@ u: U = U
   evaluating O
   constructing A
 == reconstituting into a fresh classloader
-  evaluating O
 == evaluating reconstituted lambda
   constructing A
diff --git a/test/files/run/t9375.check b/test/files/run/t9375.check
@@ -0,0 +1,60 @@
+  konstruktor: class A
+  konstruktor: class A$O$12$
+  konstruktor: class A$$anon$1
+  konstruktor: class A$A
+  konstruktor: class A$C
+  konstruktor: class C
+  konstruktor: class T$O$15$
+  konstruktor: class T$$anon$2
+  konstruktor: class T$A
+  konstruktor: class T$C
+  konstruktor: class A$N$
+  konstruktor: class T$N$
+serializing outer objects should not initialize any nested objects
+now initializing nested objects
+  konstruktor: class A$O$
+  konstruktor: class A$Op$
+  konstruktor: class A$N$O$
+  konstruktor: class A$N$Op$
+  konstruktor: class A$A$O$
+  konstruktor: class A$A$Op$
+  konstruktor: class A$T$O$
+  konstruktor: class A$T$Op$
+  konstruktor: class A$O$11$
+  konstruktor: class A$$anonfun$1$O$13$
+  konstruktor: class A$$anon$1$O$
+  konstruktor: class A$$anon$1$Op$
+  konstruktor: class T$O$
+  konstruktor: class T$Op$
+  konstruktor: class T$N$O$
+  konstruktor: class T$N$Op$
+  konstruktor: class T$A$O$
+  konstruktor: class T$A$Op$
+  konstruktor: class T$T$O$
+  konstruktor: class T$T$Op$
+  konstruktor: class T$O$14$
+  konstruktor: class T$$anonfun$2$O$16$
+  konstruktor: class T$$anon$2$O$
+  konstruktor: class T$$anon$2$Op$
+no object konstruktors called when serializing / deserializing objects (starting at the outer or the object itself)
+deserializing outer objects with non-initialized inners again
+accessing modules triggers initialization
+  konstruktor: class A$O$
+  konstruktor: class A$Op$
+  konstruktor: class A$N$O$
+  konstruktor: class A$N$Op$
+deserializing creates a new object graph, including new scala 'object' instances, no matter where serialization starts
+init static module M and field v
+  konstruktor: class M$
+  konstruktor: class M$O$18$
+serDeser does not initialize nested static modules
+init M.O
+  konstruktor: class M$O$
+serDeser nested static module
+objects declared in field decls are not static modules, so they deserialize to new instances
+init lazy val M.w
+objects declared in lazy val are not static modules either
+  konstruktor: class M$O$19$
+object declared in a function: new instance created on each invocation
+  konstruktor: class M$$anonfun$3$O$20$
+  konstruktor: class M$$anonfun$3$O$20$