Add bridges for overridden methods in lambda indy call

If a SAM trait's abstract method overrides a method in a
supertrait while changing the return type, the generated
invokedynamic instruction needs to pass the types of the
overridden methods to `LambdaMetaFactory` so that bridge
methods can be added to the generated lambda.

Java does this differently: it generates a default method
in the subinterface overriding the superinterface method.
Theoretically we could also generate the default bridges,
but that is a binary-incompatible change, so I think it's
safer to just add the bridges in the invokedynamic.

Honestly I'm surprised that this hasn't come up already.

Fixes scala/bug#10512.

Author: hrhino

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

@@ -1349,23 +1349,36 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
       val constrainedType = MethodBType(lambdaParams.map(p => typeToBType(p.tpe)), typeToBType(lambdaTarget.tpe.resultType)).toASMType
       val samMethodType = methodBTypeFromSymbol(sam).toASMType
       val markers = if (addScalaSerializableMarker) classBTypeFromSymbol(definitions.SerializableClass).toASMType :: Nil else Nil
-      visitInvokeDynamicInsnLMF(bc.jmethod, sam.name.toString, invokedType, samMethodType, implMethodHandle, constrainedType, isSerializable, markers)
+      val overriddenMethods = enteringErasure(sam.allOverriddenSymbols).map(o => methodBTypeFromSymbol(o).toASMType)
+      visitInvokeDynamicInsnLMF(bc.jmethod, sam.name.toString, invokedType, samMethodType, implMethodHandle, constrainedType, overriddenMethods, isSerializable, markers)
       if (isSerializable)
         addIndyLambdaImplMethod(cnode.name, implMethodHandle)
     }
   }
 
   private def visitInvokeDynamicInsnLMF(jmethod: MethodNode, samName: String, invokedType: String, samMethodType: asm.Type,
-                                        implMethodHandle: asm.Handle, instantiatedMethodType: asm.Type,
-                                        serializable: Boolean, markerInterfaces: Seq[asm.Type]) = {
+                                        implMethodHandle: asm.Handle, instantiatedMethodType: asm.Type, overriddenMethodTypes: Seq[asm.Type],
+                                        serializable: Boolean, markerInterfaces: Seq[asm.Type]): Unit = {
     import java.lang.invoke.LambdaMetafactory.{FLAG_BRIDGES, FLAG_MARKERS, FLAG_SERIALIZABLE}
     // 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 needsBridge = samMethodType != instantiatedMethodType
-    val bridges = if (needsBridge) Seq(Int.box(1), instantiatedMethodType) else Nil
+    val needsGenericBridge = samMethodType != instantiatedMethodType
+    // scala/bug#10512: 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 bridges = (
+      if (needsGenericBridge)
+        instantiatedMethodType +: overriddenMethodTypes
+      else overriddenMethodTypes
+    ).distinct.filterNot(_ == samMethodType)
+
+    /* We're saving on precious BSM arg slots by not passing 0 as the bridge count */
+    val bridgeArgs = if (bridges.nonEmpty) Int.box(bridges.length) +: bridges else Nil
+
     def flagIf(b: Boolean, flag: Int): Int = if (b) flag else 0
-    val flags = FLAG_MARKERS | flagIf(serializable, FLAG_SERIALIZABLE) | flagIf(needsBridge, FLAG_BRIDGES)
-    val bsmArgs = Seq(samMethodType, implMethodHandle, instantiatedMethodType, Int.box(flags), Int.box(markerInterfaces.length)) ++ markerInterfaces ++ bridges
+    val flags = FLAG_MARKERS | flagIf(serializable, FLAG_SERIALIZABLE) | flagIf(bridges.nonEmpty, FLAG_BRIDGES)
+    val bsmArgs = Seq(samMethodType, implMethodHandle, instantiatedMethodType, Int.box(flags), Int.box(markerInterfaces.length)) ++ markerInterfaces ++ bridgeArgs
+
     jmethod.visitInvokeDynamicInsn(samName, invokedType, lambdaMetaFactoryAltMetafactoryHandle, bsmArgs: _*)
   }
 

src/reflect/scala/reflect/internal/Symbols.scala

@@ -2385,7 +2385,13 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
       } else Nil
     }
 
-    /** Equivalent to allOverriddenSymbols.nonEmpty, but more efficient. */
+    /** Equivalent to allOverriddenSymbols.nonEmpty, but more efficient.
+      *
+      * Drive-by warning: the result of `allOverriddenSymbols` can go from
+      * non-empty to empty following `erasure`; it seems that this is always
+      * initialized pre-erasure, so post-erasure this method is no longer
+      * equivalent to `allOverriddenSymbols.nonEmpty` like it claims.
+      */
     lazy val isOverridingSymbol = (
          canMatchInheritedSymbols
       && owner.ancestors.exists(base => overriddenSymbol(base) != NoSymbol)

test/files/jvm/t10512a.scala

@@ -0,0 +1,27 @@
+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 {
+
+  implicit val stringEncoder: JsonEncoder[String] =
+    s => new JsonString
+
+  implicit def listEncoder[A](implicit encoder: JsonEncoder[A]): JsonObjectEncoder[List[A]] =
+    l => new JsonObject
+
+}
+
+object Test extends App {
+  import JsonEncoderInstances._
+
+  implicitly[JsonEncoder[List[String]]].encode("" :: Nil)
+}

test/files/jvm/t10512b.scala

@@ -0,0 +1,54 @@
+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)
+
+}