Fix #24573: Add stricter checks for platform SAM compatibility (#24624)

This PR fixes issue #24573 by adding stricter checks for platform SAM
compatibility.

The regression issue starts from version 3.8.0 where `Function1` and
related traits gained the `NoInits` flag (I guess due to the way we read
from jvm bytecode or from tasty?).
This changes how the compiler determines whether a SAM type qualifies as
a platform SAM. When a trait is classified as a Java platform SAM, the
compiler uses `invokedynamic` with `LambdaMetaFactory` (LMF) to create
the lambda at runtime, rather than expanding it into an anonymous class
at compile time. This is more efficient, but LMF has limitations on what
type adaptations it can perform automatically.

It turns out the logic regarding SAM traits extending other traits has
problems from the beginning, and even in Scala 2. The code from the
issue works before 3.8 is totally by accident (because we always expand
the SAM trait extending `FunctionN`).
We can reproduce the runtime exception using a custom function class (in
Scala 2, and Scala 3.0~8):

```scala
trait F1[-T, +R]:
  def apply(t: T): R

trait SF[-T] extends F1[T, Unit]:
  def apply(t: T): Unit

@main def Test =
  val sf: SF[String] = i => println(i)
  sf("") // exception here
  // Caused by: java.lang.invoke.LambdaConversionException: 
  //Type mismatch for lambda expected return: void is not convertible to class java.lang.Object
```

From the view of Scala, `SF` is a valid SAM, because it has one abstract
function `def apply(t: T): Unit`. However, after erasure, the `apply` of
`F1` becomes `Object apply(Object)`, and the apply of `SF` becomes `void
apply(Object)`, which are two different abstract functions; hence, `SF`
is not qualifies as a function interface on JVM. This is not only for
`Unit` type, any primitive number type can cause the similar problem.

Therefore, we have to expand the rhs of `val sf: SF[String] = i =>
println(i)` into an anonymous class and implement all the bridge
functions.

This PR strengthen the SAM check on Java platform by adding a function
called `samNotNeededExpansion` to check if the SAM method's erased
signature is compatible with all overridden methods. It mirrors the
logic in `Erasure.Boxing.adaptClosure` to determine what LMF can and
cannot auto-adapt.

Author: noti0na1

compiler/src/dotty/tools/dotc/config/JavaPlatform.scala

@@ -50,7 +50,10 @@ class JavaPlatform extends Platform {
     cls.superClass == defn.ObjectClass &&
     cls.directlyInheritedTraits.forall(_.is(NoInits)) &&
     !ExplicitOuter.needsOuterIfReferenced(cls) &&
-    cls.typeRef.fields.isEmpty // Superaccessors already show up as abstract methods here, so no test necessary
+    // Superaccessors already show up as abstract methods here, so no test necessary
+    cls.typeRef.fields.isEmpty &&
+    // Check if the SAM can be implemented via LambdaMetaFactory
+    TypeErasure.samExpansionNotNeeded(cls)
 
   /** We could get away with excluding BoxedBooleanClass for the
    *  purpose of equality testing since it need not compare equal

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

@@ -74,7 +74,7 @@ end SourceLanguage
  *  only for isInstanceOf, asInstanceOf: PolyType, TypeParamRef, TypeBounds
  *
  */
-object TypeErasure {
+object TypeErasure:
 
   private def erasureDependsOnArgs(sym: Symbol)(using Context) =
     sym == defn.ArrayClass || sym == defn.PairClass || sym.isDerivedValueClass
@@ -586,7 +586,102 @@ object TypeErasure {
         defn.FunctionType(n = info.nonErasedParamCount)
     }
     erasure(functionType(applyInfo))
-}
+
+  /** Check if LambdaMetaFactory can handle signature adaptation between two method types.
+   *
+   *  LMF has limitations on what type adaptations it can perform automatically.
+   *  This method checks whether manual bridging is needed for params and/or result.
+   *
+   *  The adaptation rules are:
+   *  - For parameters: primitives and value classes cannot be auto-adapted by LMF
+   *    because the Scala spec requires null to be "unboxed" to the default value,
+   *    but LMF throws `NullPointerException` instead.
+   *  - For results: value classes and Unit cannot be auto-adapted by LMF.
+   *    Non-Unit primitives can be auto-adapted since LMF only needs to box (not unbox).
+   *  - LMF cannot auto-adapt between Object and Array types.
+   *
+   *  @param implParamTypes  Parameter types of the implementation method
+   *  @param implResultType  Result type of the implementation method
+   *  @param samParamTypes   Parameter types of the SAM method
+   *  @param samResultType   Result type of the SAM method
+   *
+   *  @return (paramNeeded, resultNeeded) indicating what needs bridging
+   */
+  def additionalAdaptationNeeded(
+      implParamTypes: List[Type],
+      implResultType: Type,
+      samParamTypes: List[Type],
+      samResultType: Type
+  )(using Context): (paramNeeded: Boolean, resultNeeded: Boolean) =
+    def sameClass(tp1: Type, tp2: Type) = tp1.classSymbol == tp2.classSymbol
+
+    /** Can the implementation parameter type `tp` be auto-adapted to a different
+     *  parameter type in the SAM?
+     *
+     *  For derived value classes, we always need to do the bridging manually.
+     *  For primitives, we cannot rely on auto-adaptation on the JVM because
+     *  the Scala spec requires null to be "unboxed" to the default value of
+     *  the value class, but the adaptation performed by LambdaMetaFactory
+     *  will throw a `NullPointerException` instead.
+     */
+    def autoAdaptedParam(tp: Type) = !tp.isErasedValueType && !tp.isPrimitiveValueType
+
+    /** Can the implementation result type be auto-adapted to a different result
+     *  type in the SAM?
+     *
+     *  For derived value classes, it's the same story as for parameters.
+     *  For non-Unit primitives, we can actually rely on the `LambdaMetaFactory`
+     *  adaptation, because it only needs to box, not unbox, so no special
+     *  handling of null is required.
+     */
+    def autoAdaptedResult(tp: Type) =
+      !tp.isErasedValueType && !(tp.classSymbol eq defn.UnitClass)
+
+    val paramAdaptationNeeded =
+      implParamTypes.lazyZip(samParamTypes).exists((implType, samType) =>
+        !sameClass(implType, samType) && (!autoAdaptedParam(implType)
+          // LambdaMetaFactory cannot auto-adapt between Object and Array types
+          || samType.isInstanceOf[JavaArrayType]))
+
+    val resultAdaptationNeeded =
+      !sameClass(implResultType, samResultType) && !autoAdaptedResult(implResultType)
+
+    (paramAdaptationNeeded, resultAdaptationNeeded)
+  end additionalAdaptationNeeded
+
+  /** Check if LambdaMetaFactory can handle the SAM method's required signature adaptation.
+   *
+   *  When a SAM method overrides other methods, the erased signatures must be compatible
+   *  to be qualifies as a valid functional interface on JVM.
+   *  This method returns true if all overridden methods have compatible erased signatures
+   *  that LMF can auto-adapt (or don't need adaptation).
+   *
+   *  When this returns true, the SAM class does not need to be expanded.
+   *
+   *  @param cls  The SAM class to check
+   *  @return     true if LMF can handle the required adaptation
+   */
+  def samExpansionNotNeeded(cls: ClassSymbol)(using Context): Boolean = cls.typeRef.possibleSamMethods match
+    case Seq(samMeth) =>
+      val samMethSym = samMeth.symbol
+      val erasedSamInfo = transformInfo(samMethSym, samMeth.info)
+
+      val (erasedSamParamTypes, erasedSamResultType) = erasedSamInfo match
+        case mt: MethodType => (mt.paramInfos, mt.resultType)
+        case _ => return false
+
+      samMethSym.allOverriddenSymbols.forall { overridden =>
+        val erasedOverriddenInfo = transformInfo(overridden, overridden.info)
+        erasedOverriddenInfo match
+          case mt: MethodType =>
+            val (paramNeeded, resultNeeded) =
+              additionalAdaptationNeeded(erasedSamParamTypes, erasedSamResultType, mt.paramInfos, mt.resultType)
+            !(paramNeeded || resultNeeded)
+          case _ => true
+      }
+    case _ => false
+  end samExpansionNotNeeded
+end TypeErasure
 
 import TypeErasure.*
 

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

@@ -453,41 +453,9 @@ object Erasure {
       val samParamTypes = sam.paramInfos
       val samResultType = sam.resultType
 
-      /** Can the implementation parameter type `tp` be auto-adapted to a different
-       *  parameter type in the SAM?
-       *
-       *  For derived value classes, we always need to do the bridging manually.
-       *  For primitives, we cannot rely on auto-adaptation on the JVM because
-       *  the Scala spec requires null to be "unboxed" to the default value of
-       *  the value class, but the adaptation performed by LambdaMetaFactory
-       *  will throw a `NullPointerException` instead. See `lambda-null.scala`
-       *  for test cases.
-       *
-       *  @see [LambdaMetaFactory](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/invoke/LambdaMetafactory.html)
-       */
-      def autoAdaptedParam(tp: Type) =
-        !tp.isErasedValueType && !tp.isPrimitiveValueType
-
-      /** Can the implementation result type be auto-adapted to a different result
-       *  type in the SAM?
-       *
-       *  For derived value classes, it's the same story as for parameters.
-       *  For non-Unit primitives, we can actually rely on the `LambdaMetaFactory`
-       *  adaptation, because it only needs to box, not unbox, so no special
-       *  handling of null is required.
-       */
-      def autoAdaptedResult =
-        !implResultType.isErasedValueType && !implReturnsUnit
-
-      def sameClass(tp1: Type, tp2: Type) = tp1.classSymbol == tp2.classSymbol
-
-      val paramAdaptationNeeded =
-        implParamTypes.lazyZip(samParamTypes).exists((implType, samType) =>
-          !sameClass(implType, samType) && (!autoAdaptedParam(implType)
-            // LambdaMetaFactory cannot auto-adapt between Object and Array types
-            || samType.isInstanceOf[JavaArrayType]))
-      val resultAdaptationNeeded =
-        !sameClass(implResultType, samResultType) && !autoAdaptedResult
+      // Check if bridging is needed using the common function from TypeErasure
+      val (paramAdaptationNeeded, resultAdaptationNeeded) =
+        additionalAdaptationNeeded(implParamTypes, implResultType, samParamTypes, samResultType)
 
       if paramAdaptationNeeded || resultAdaptationNeeded then
         // Instead of instantiating `scala.FunctionN`, see if we can instantiate

tests/run/i24573.check

@@ -0,0 +1,45 @@
+1
+2
+3
+11
+12
+13
+14
+15
+16
+17
+18
+19
+20
+21
+22
+23
+24
+25
+31
+32
+33
+34
+41
+42
+43
+44
+45
+46
+51
+52
+53
+55
+56
+57
+61
+62
+63
+64
+65
+71
+72
+75
+76
+81
+82