Merge pull request #11839 from dotty-staging/fix-SAM-lambda-syntax

Fix bytecode generation for Single Abstract Method lambdas

Author: smarter

compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala

@@ -1414,7 +1414,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
     def genLoadTry(tree: Try): BType
 
     def genInvokeDynamicLambda(ctor: Symbol, lambdaTarget: Symbol, environmentSize: Int, functionalInterface: Symbol): BType = {
-      import java.lang.invoke.LambdaMetafactory.FLAG_SERIALIZABLE
+      import java.lang.invoke.LambdaMetafactory.{FLAG_BRIDGES, FLAG_SERIALIZABLE}
 
       report.debuglog(s"Using invokedynamic rather than `new ${ctor.owner}`")
       val generatedType = classBTypeFromSymbol(functionalInterface)
@@ -1445,9 +1445,9 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
       val functionalInterfaceDesc: String = generatedType.descriptor
       val desc = capturedParamsTypes.map(tpe => toTypeKind(tpe)).mkString(("("), "", ")") + functionalInterfaceDesc
       // TODO specialization
-      val constrainedType = new MethodBType(lambdaParamTypes.map(p => toTypeKind(p)), toTypeKind(lambdaTarget.info.resultType)).toASMType
+      val instantiatedMethodType = new MethodBType(lambdaParamTypes.map(p => toTypeKind(p)), toTypeKind(lambdaTarget.info.resultType)).toASMType
 
-      val abstractMethod = atPhase(erasurePhase) {
+      val samMethod = atPhase(erasurePhase) {
         val samMethods = toDenot(functionalInterface).info.possibleSamMethods.toList
         samMethods match {
           case x :: Nil => x.symbol
@@ -1457,21 +1457,40 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
         }
       }
 
-      val methodName = abstractMethod.javaSimpleName
-      val applyN = {
-        val mt = asmMethodType(abstractMethod)
-        mt.toASMType
+      val methodName = samMethod.javaSimpleName
+      val samMethodType = asmMethodType(samMethod).toASMType
+      // scala/bug#10334: make sure that a lambda object for `T => U` has a method `apply(T)U`, not only the `(Object)Object`
+      // version. Using the lambda a structural type `{def apply(t: T): U}` causes a reflective lookup for this method.
+      val needsGenericBridge = samMethodType != instantiatedMethodType
+      val bridgeMethods = atPhase(erasurePhase){
+        samMethod.allOverriddenSymbols.toList
       }
-      val bsmArgs0 = Seq(applyN, targetHandle, constrainedType)
-      val bsmArgs =
-        if (isSerializable)
-          bsmArgs0 :+ Int.box(FLAG_SERIALIZABLE)
+      val overriddenMethodTypes = bridgeMethods.map(b => asmMethodType(b).toASMType)
+
+      // any methods which `samMethod` overrides need bridges made for them
+      // this is done automatically during erasure for classes we generate, but LMF needs to have them explicitly mentioned
+      // so we have to compute them at this relatively late point.
+      val bridgeTypes = (
+        if (needsGenericBridge)
+          instantiatedMethodType +: overriddenMethodTypes
         else
-          bsmArgs0
+          overriddenMethodTypes
+      ).distinct.filterNot(_ == samMethodType)
+
+      val needsBridges = bridgeTypes.nonEmpty
+
+      def flagIf(b: Boolean, flag: Int): Int = if (b) flag else 0
+      val flags = flagIf(isSerializable, FLAG_SERIALIZABLE) | flagIf(needsBridges, FLAG_BRIDGES)
+
+      val bsmArgs0 = Seq(samMethodType, targetHandle, instantiatedMethodType)
+      val bsmArgs1 = if (flags != 0) Seq(Int.box(flags)) else Seq.empty
+      val bsmArgs2 = if needsBridges then bridgeTypes.length +: bridgeTypes else Seq.empty
+
+      val bsmArgs = bsmArgs0 ++ bsmArgs1 ++ bsmArgs2
 
       val metafactory =
-        if (isSerializable)
-          lambdaMetaFactoryAltMetafactoryHandle // altMetafactory needed to be able to pass the SERIALIZABLE flag
+        if (flags != 0)
+          lambdaMetaFactoryAltMetafactoryHandle // altMetafactory required to be able to pass the flags and additional arguments if needed
         else
           lambdaMetaFactoryMetafactoryHandle
 

compiler/src/dotty/tools/backend/jvm/GenBCode.scala

@@ -334,11 +334,13 @@ class GenBCodePipeline(val int: DottyBackendInterface, val primitives: DottyPrim
           val insn = iter.next()
           insn match {
             case indy: InvokeDynamicInsnNode
-                // No need to check the exact bsmArgs because we only generate
-                // altMetafactory indy calls for serializable lambdas.
-                if indy.bsm == BCodeBodyBuilder.lambdaMetaFactoryAltMetafactoryHandle =>
-              val implMethod = indy.bsmArgs(1).asInstanceOf[Handle]
-              indyLambdaBodyMethods += implMethod
+              if indy.bsm == BCodeBodyBuilder.lambdaMetaFactoryAltMetafactoryHandle =>
+                import java.lang.invoke.LambdaMetafactory.FLAG_SERIALIZABLE
+                val metafactoryFlags = indy.bsmArgs(3).asInstanceOf[Integer].toInt
+                val isSerializable = (metafactoryFlags & FLAG_SERIALIZABLE) != 0
+                if isSerializable then
+                  val implMethod = indy.bsmArgs(1).asInstanceOf[Handle]
+                  indyLambdaBodyMethods += implMethod
             case _ =>
           }
         }

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -928,8 +928,10 @@ object Types {
      */
     final def possibleSamMethods(using Context): Seq[SingleDenotation] = {
       record("possibleSamMethods")
-      abstractTermMembers.toList.filterConserve(m =>
-        !m.symbol.matchingMember(defn.ObjectType).exists && !m.symbol.isSuperAccessor)
+      atPhaseNoLater(erasurePhase) {
+        abstractTermMembers.toList.filterConserve(m =>
+          !m.symbol.matchingMember(defn.ObjectType).exists && !m.symbol.isSuperAccessor)
+      }.map(_.current)
     }
 
     /** The set of abstract type members of this type. */

compiler/src/dotty/tools/dotc/transform/Erasure.scala

@@ -431,8 +431,9 @@ object Erasure {
       val implParamTypes = implType.paramInfos
       val implResultType = implType.resultType
       val implReturnsUnit = implResultType.classSymbol eq defn.UnitClass
-      // The SAM that this closure should implement
-      val SAMType(sam) = lambdaType: @unchecked
+      // The SAM that this closure should implement.
+      // At this point it should be already guaranteed that there's only one method to implement
+      val Seq(sam: MethodType) = lambdaType.possibleSamMethods.map(_.info)
       val samParamTypes = sam.paramInfos
       val samResultType = sam.resultType
 
@@ -503,7 +504,7 @@ object Erasure {
               implType.derivedLambdaType(paramInfos = samParamTypes)
           else
             implType.derivedLambdaType(resType = samResultType)
-        val bridge = newSymbol(ctx.owner, AdaptedClosureName(meth.symbol.name.asTermName), Flags.Synthetic | Flags.Method, bridgeType)
+        val bridge = newSymbol(ctx.owner, AdaptedClosureName(meth.symbol.name.asTermName), Flags.Synthetic | Flags.Method | Flags.Bridge, bridgeType)
         Closure(bridge, bridgeParamss =>
           inContext(ctx.withOwner(bridge)) {
             val List(bridgeParams) = bridgeParamss

tests/run/i10068a.check

@@ -0,0 +1,3 @@
+42
+Foo
+Foo

tests/run/i10068a.scala

@@ -0,0 +1,31 @@
+sealed trait Partial
+sealed trait Total extends Partial
+
+case object Foo extends Total
+
+trait P[A] {
+  def bar(a: A): Partial
+}
+
+trait T[A] extends P[A] {
+  def bar(a: A): Total
+}
+
+object T {
+  def make[A](x: Total): T[A] =
+    a => x
+}
+
+object Test {
+  def total[A](a: A)(ev: T[A]): Total = ev.bar(a)
+  def partial[A](a: A)(ev: P[A]): Partial = ev.bar(a)
+
+  def go[A](a: A)(ev: T[A]): Unit = {
+    println(a)
+    println(total(a)(ev))
+    println(partial(a)(ev))
+  }
+
+  def main(args: Array[String]): Unit =
+    go(42)(T.make(Foo))
+}

tests/run/i10068b.scala

@@ -0,0 +1,23 @@
+trait Foo[A] {
+  def xxx(a1: A, a2: A): A
+  def xxx(a: A): A = xxx(a, a)
+}
+
+trait Bar[A] extends Foo[A] {
+  def yyy(a1: A, a2: A) = xxx(a1, a2)
+}
+
+trait Baz[A] extends Bar[A]
+
+object Test:
+  def main(args: Array[String]): Unit =
+    val foo: Foo[String] = { (s1, s2) => s1 ++ s2 }
+    val bar: Bar[String] = { (s1, s2) => s1 ++ s2 }
+    val baz: Baz[String] = { (s1, s2) => s1 ++ s2 }
+
+    val s = "abc"
+    val ss = "abcabc"
+    assert(foo.xxx(s) == ss)
+    assert(bar.yyy(s, s) == ss)
+    assert(baz.xxx(s) == ss)
+    assert(baz.yyy(s, s) == ss)

tests/run/i10068c.scala

@@ -0,0 +1,45 @@
+// Taken from: https://github.com/scala/scala/pull/6087
+
+trait JsonValue
+class JsonObject extends JsonValue
+class JsonString extends JsonValue
+
+trait JsonEncoder[A] {
+  def encode(value: A): JsonValue
+}
+
+trait JsonObjectEncoder[A] extends JsonEncoder[A] {
+  def encode(value: A): JsonObject
+}
+
+object JsonEncoderInstances {
+
+  val seWorks: JsonEncoder[String] =
+    new JsonEncoder[String] {
+      def encode(value: String) = new JsonString
+    }
+
+  implicit val stringEncoder: JsonEncoder[String] =
+    s => new JsonString
+    //new JsonEncoder[String] {
+    //  def encode(value: String) = new JsonString
+    //}
+
+  def leWorks[A](implicit encoder: JsonEncoder[A]): JsonObjectEncoder[List[A]] =
+    new JsonObjectEncoder[List[A]] {
+      def encode(value: List[A]) = new JsonObject
+    }
+
+  implicit def listEncoder[A](implicit encoder: JsonEncoder[A]): JsonObjectEncoder[List[A]] =
+    l => new JsonObject
+//    new JsonObjectEncoder[List[A]] {
+//      def encode(value: List[A]) = new JsonObject
+//    }
+
+}
+
+object Test extends App {
+  import JsonEncoderInstances._
+
+  implicitly[JsonEncoder[List[String]]].encode("" :: Nil)
+}

tests/run/i10068d.scala

@@ -0,0 +1,56 @@
+// Taken from: https://github.com/scala/scala/pull/6087
+
+trait A
+trait B extends A
+trait C extends B
+object it extends C
+
+/* try as many weird diamondy things as I can think of */
+trait SAM_A                 { def apply(): A }
+trait SAM_A1 extends SAM_A  { def apply(): A }
+trait SAM_B  extends SAM_A1 { def apply(): B }
+trait SAM_B1 extends SAM_A1 { def apply(): B }
+trait SAM_B2 extends SAM_B with SAM_B1
+trait SAM_C  extends SAM_B2 { def apply(): C }
+
+trait SAM_F  extends (() => A) with SAM_C
+trait SAM_F1 extends (() => C) with SAM_F
+
+
+object Test extends App {
+
+  val s1: SAM_A  = () => it
+  val s2: SAM_A1 = () => it
+  val s3: SAM_B  = () => it
+  val s4: SAM_B1 = () => it
+  val s5: SAM_B2 = () => it
+  val s6: SAM_C  = () => it
+  val s7: SAM_F  = () => it
+  val s8: SAM_F1 = () => it
+
+  (s1(): A)
+
+  (s2(): A)
+
+  (s3(): B)
+  (s3(): A)
+
+  (s4(): B)
+  (s4(): A)
+
+  (s5(): B)
+  (s5(): A)
+
+  (s6(): C)
+  (s6(): B)
+  (s6(): A)
+
+  (s7(): C)
+  (s7(): B)
+  (s7(): A)
+
+  (s8(): C)
+  (s8(): B)
+  (s8(): A)
+
+}

tests/run/i11676.scala

@@ -0,0 +1,25 @@
+sealed trait PartialOrdering
+sealed trait Ordering extends PartialOrdering
+
+object Ordering {
+  def fromCompare(n: Int): Ordering = new Ordering {}
+}
+
+trait PartialOrd[-A] {
+  def checkCompare(l: A, r: A): PartialOrdering
+}
+
+trait Ord[-A] extends PartialOrd[A] {
+  def checkCompare(l: A, r: A): Ordering
+}
+
+object Ord {
+  def fromScala[A](implicit ordering: scala.math.Ordering[A]): Ord[A] =
+     (l: A, r: A) => Ordering.fromCompare(ordering.compare(l, r))
+}
+
+object Test {
+  def main(args: Array[String]): Unit =
+    val intOrd = Ord.fromScala[Int]
+    intOrd.checkCompare(1, 3)
+}