Merge pull request #5278 from retronym/ticket/SD-120

SD-120 Non FunctionN lambdas should not be universally serializable

5 files changed, 80 insertions, 31 deletions

diff --git a/src/compiler/scala/tools/nsc/ast/TreeGen.scala b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
index bc89609a59..f53fb336e9 100644
--- a/src/compiler/scala/tools/nsc/ast/TreeGen.scala
+++ b/src/compiler/scala/tools/nsc/ast/TreeGen.scala
@@ -355,20 +355,22 @@ abstract class TreeGen extends scala.reflect.internal.TreeGen with TreeDSL {
     treeCopy.DefDef(orig, orig.mods, orig.name, orig.tparams, (selfParam :: orig.vparamss.head) :: Nil, orig.tpt, rhs).setSymbol(newSym)
   }
 
-  // TODO: the rewrite to AbstractFunction is superfluous once we compile FunctionN to a SAM type (aka functional interface)
-  def functionClassType(fun: Function): Type =
-    if (isFunctionType(fun.tpe)) abstractFunctionType(fun.vparams.map(_.symbol.tpe), fun.body.tpe.deconst)
-    else fun.tpe
-
   def expandFunction(localTyper: analyzer.Typer)(fun: Function, inConstructorFlag: Long): Tree = {
-    val parents = addSerializable(functionClassType(fun))
-    val anonClass = fun.symbol.owner newAnonymousFunctionClass(fun.pos, inConstructorFlag) addAnnotation SerialVersionUIDAnnotation
+    val anonClass = fun.symbol.owner newAnonymousFunctionClass(fun.pos, inConstructorFlag)
+    val parents = if (isFunctionType(fun.tpe)) {
+      anonClass addAnnotation SerialVersionUIDAnnotation
+      addSerializable(abstractFunctionType(fun.vparams.map(_.symbol.tpe), fun.body.tpe.deconst))
+    } else {
+      if (fun.tpe.typeSymbol.isSubClass(JavaSerializableClass))
+        anonClass addAnnotation SerialVersionUIDAnnotation
+      fun.tpe :: Nil
+    }
+    anonClass setInfo ClassInfoType(parents, newScope, anonClass)
 
     // The original owner is used in the backend for the EnclosingMethod attribute. If fun is
     // nested in a value-class method, its owner was already changed to the extension method.
     // Saving the original owner allows getting the source structure from the class symbol.
     defineOriginalOwner(anonClass, fun.symbol.originalOwner)
-    anonClass setInfo ClassInfoType(parents, newScope, anonClass)
 
     val samDef = mkMethodFromFunction(localTyper)(anonClass, fun)
     anonClass.info.decls enter samDef.symbol
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
index acedf83016..d5c4b5e201 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
@@ -15,7 +15,7 @@ import scala.tools.asm
 import GenBCode._
 import BackendReporting._
 import scala.tools.asm.Opcodes
-import scala.tools.asm.tree.MethodInsnNode
+import scala.tools.asm.tree.{MethodInsnNode, MethodNode}
 import scala.tools.nsc.backend.jvm.BCodeHelpers.{InvokeStyle, TestOp}
 
 /*
@@ -630,7 +630,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
         case Apply(fun, args) if app.hasAttachment[delambdafy.LambdaMetaFactoryCapable] =>
           val attachment = app.attachments.get[delambdafy.LambdaMetaFactoryCapable].get
           genLoadArguments(args, paramTKs(app))
-          genInvokeDynamicLambda(attachment.target, attachment.arity, attachment.functionalInterface, attachment.sam)
+          genInvokeDynamicLambda(attachment.target, attachment.arity, attachment.functionalInterface, attachment.sam, attachment.isSerializable, attachment.addScalaSerializableMarker)
           generatedType = methodBTypeFromSymbol(fun.symbol).returnType
 
         case Apply(fun, List(expr)) if currentRun.runDefinitions.isBox(fun.symbol) =>
@@ -1330,7 +1330,7 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
     def genSynchronized(tree: Apply, expectedType: BType): BType
     def genLoadTry(tree: Try): BType
 
-    def genInvokeDynamicLambda(lambdaTarget: Symbol, arity: Int, functionalInterface: Symbol, sam: Symbol) {
+    def genInvokeDynamicLambda(lambdaTarget: Symbol, arity: Int, functionalInterface: Symbol, sam: Symbol, isSerializable: Boolean, addScalaSerializableMarker: Boolean) {
       val isStaticMethod = lambdaTarget.hasFlag(Flags.STATIC)
       def asmType(sym: Symbol) = classBTypeFromSymbol(sym).toASMType
 
@@ -1343,26 +1343,24 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
           /* itf = */ isInterface)
       val receiver = if (isStaticMethod) Nil else lambdaTarget.owner :: Nil
       val (capturedParams, lambdaParams) = lambdaTarget.paramss.head.splitAt(lambdaTarget.paramss.head.length - arity)
-      // Requires https://github.com/scala/scala-java8-compat on the runtime classpath
       val invokedType = asm.Type.getMethodDescriptor(asmType(functionalInterface), (receiver ::: capturedParams).map(sym => typeToBType(sym.info).toASMType): _*)
-
       val constrainedType = new MethodBType(lambdaParams.map(p => typeToBType(p.tpe)), typeToBType(lambdaTarget.tpe.resultType)).toASMType
-      val samName = sam.name.toString
       val samMethodType = methodBTypeFromSymbol(sam).toASMType
-
-      val flags = java.lang.invoke.LambdaMetafactory.FLAG_SERIALIZABLE | java.lang.invoke.LambdaMetafactory.FLAG_MARKERS
-
-      val ScalaSerializable = classBTypeFromSymbol(definitions.SerializableClass).toASMType
-      bc.jmethod.visitInvokeDynamicInsn(samName, invokedType, lambdaMetaFactoryAltMetafactoryHandle,
-        /* samMethodType          = */ samMethodType,
-        /* implMethod             = */ implMethodHandle,
-        /* instantiatedMethodType = */ constrainedType,
-        /* flags                  = */ flags.asInstanceOf[AnyRef],
-        /* markerInterfaceCount   = */ 1.asInstanceOf[AnyRef],
-        /* markerInterfaces[0]    = */ ScalaSerializable,
-        /* bridgeCount            = */ 0.asInstanceOf[AnyRef]
-      )
-      indyLambdaHosts += cnode.name
+      val markers = if (addScalaSerializableMarker) classBTypeFromSymbol(definitions.SerializableClass).toASMType :: Nil else Nil
+      visitInvokeDynamicInsnLMF(bc.jmethod, sam.name.toString, invokedType, samMethodType, implMethodHandle, constrainedType, isSerializable, markers)
+      if (isSerializable)
+        indyLambdaHosts += cnode.name
     }
   }
+
+  private def visitInvokeDynamicInsnLMF(jmethod: MethodNode, samName: String, invokedType: String, samMethodType: asm.Type,
+                                        implMethodHandle: asm.Handle, instantiatedMethodType: asm.Type,
+                                        serializable: Boolean, markerInterfaces: Seq[asm.Type]) = {
+    import java.lang.invoke.LambdaMetafactory.{FLAG_MARKERS, FLAG_SERIALIZABLE}
+    def flagIf(b: Boolean, flag: Int): Int = if (b) flag else 0
+    val flags = FLAG_MARKERS | flagIf(serializable, FLAG_SERIALIZABLE)
+    val bsmArgs = Seq(samMethodType, implMethodHandle, instantiatedMethodType, Int.box(flags), Int.box(markerInterfaces.length)) ++ markerInterfaces
+    jmethod.visitInvokeDynamicInsn(samName, invokedType, lambdaMetaFactoryAltMetafactoryHandle, bsmArgs: _*)
+  }
+
 }
diff --git a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
index 88837842fa..855e53710b 100644
--- a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
+++ b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala
@@ -28,7 +28,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
   /** the following two members override abstract members in Transform */
   val phaseName: String = "delambdafy"
 
-  final case class LambdaMetaFactoryCapable(target: Symbol, arity: Int, functionalInterface: Symbol, sam: Symbol)
+  final case class LambdaMetaFactoryCapable(target: Symbol, arity: Int, functionalInterface: Symbol, sam: Symbol, isSerializable: Boolean, addScalaSerializableMarker: Boolean)
 
   /**
     * Get the symbol of the target lifted lambda body method from a function. I.e. if
@@ -95,6 +95,8 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
 
       // no need for adaptation when the implemented sam is of a specialized built-in function type
       val lambdaTarget = if (isSpecialized) target else createBoxingBridgeMethodIfNeeded(fun, target, functionalInterface, sam)
+      val isSerializable = samUserDefined == NoSymbol || samUserDefined.owner.isNonBottomSubClass(definitions.JavaSerializableClass)
+      val addScalaSerializableMarker = samUserDefined == NoSymbol
 
       // The backend needs to know the target of the lambda and the functional interface in order
       // to emit the invokedynamic instruction. We pass this information as tree attachment.
@@ -102,7 +104,7 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre
       // see https://docs.oracle.com/javase/8/docs/api/java/lang/invoke/LambdaMetafactory.html
       //   instantiatedMethodType is derived from lambdaTarget's signature
       //   samMethodType is derived from samOf(functionalInterface)'s signature
-      apply.updateAttachment(LambdaMetaFactoryCapable(lambdaTarget, fun.vparams.length, functionalInterface, sam))
+      apply.updateAttachment(LambdaMetaFactoryCapable(lambdaTarget, fun.vparams.length, functionalInterface, sam, isSerializable, addScalaSerializableMarker))
 
       apply
     }
diff --git a/test/files/run/lambda-serialization.scala b/test/files/run/lambda-serialization.scala
index 0eee1193d7..08e235b1cb 100644
--- a/test/files/run/lambda-serialization.scala
+++ b/test/files/run/lambda-serialization.scala
@@ -1,6 +1,6 @@
 import java.io.{ByteArrayInputStream, ObjectInputStream, ObjectOutputStream, ByteArrayOutputStream}
 
-trait IntToString { def apply(i: Int): String }
+trait IntToString extends java.io.Serializable { def apply(i: Int): String }
 
 object Test {
   def main(args: Array[String]): Unit = {
diff --git a/test/files/run/sammy_seriazable.scala b/test/files/run/sammy_seriazable.scala
new file mode 100644
index 0000000000..458b99238a
--- /dev/null
+++ b/test/files/run/sammy_seriazable.scala
@@ -0,0 +1,47 @@
+import java.io._
+
+trait NotSerializableInterface { def apply(a: Any): Any }
+abstract class NotSerializableClass { def apply(a: Any): Any }
+// SAM type that supports lambdas-as-invoke-dynamic
+trait IsSerializableInterface extends java.io.Serializable { def apply(a: Any): Any }
+// SAM type that still requires lambdas-as-anonhmous-classes
+abstract class IsSerializableClass extends java.io.Serializable { def apply(a: Any): Any }
+
+object Test {
+  def main(args: Array[String]) {
+    val nsi: NotSerializableInterface = x => x
+    val nsc: NotSerializableClass = x => x
+
+    import SerDes._
+    assertNotSerializable(nsi)
+    assertNotSerializable(nsc)
+    assert(serializeDeserialize[IsSerializableInterface](x => x).apply("foo") == "foo")
+    assert(serializeDeserialize[IsSerializableClass](x => x).apply("foo") == "foo")
+    assert(ObjectStreamClass.lookup(((x => x): IsSerializableClass).getClass).getSerialVersionUID == 0)
+  }
+}
+
+object SerDes {
+  def assertNotSerializable(a: AnyRef): Unit = {
+    try {
+      serialize(a)
+      assert(false)
+    } catch {
+      case _: NotSerializableException => // okay
+    }
+  }
+
+  def serialize(obj: AnyRef): Array[Byte] = {
+    val buffer = new ByteArrayOutputStream
+    val out = new ObjectOutputStream(buffer)
+    out.writeObject(obj)
+    buffer.toByteArray
+  }
+
+  def deserialize(a: Array[Byte]): AnyRef = {
+    val in = new ObjectInputStream(new ByteArrayInputStream(a))
+    in.readObject
+  }
+
+  def serializeDeserialize[T <: AnyRef](obj: T) = deserialize(serialize(obj)).asInstanceOf[T]
+}