Merge pull request #12214 from dotty-staging/fix-12211

Fix comparison of dependent function types

Author: smarter

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

@@ -1698,40 +1698,70 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
    */
   protected def hasMatchingMember(name: Name, tp1: Type, tp2: RefinedType): Boolean =
     trace(i"hasMatchingMember($tp1 . $name :? ${tp2.refinedInfo}), mbr: ${tp1.member(name).info}", subtyping) {
-      val rinfo2 = tp2.refinedInfo
-
-      // If the member is an abstract type and the prefix is a path, compare the member itself
-      // instead of its bounds. This case is needed situations like:
-      //
-      //    class C { type T }
-      //    val foo: C
-      //    foo.type <: C { type T {= , <: , >:} foo.T }
-      //
-      // or like:
-      //
-      //    class C[T]
-      //    C[?] <: C[TV]
-      //
-      // where TV is a type variable. See i2397.scala for an example of the latter.
-      def matchAbstractTypeMember(info1: Type) = info1 match {
-        case TypeBounds(lo, hi) if lo ne hi =>
-          tp2.refinedInfo match {
-            case rinfo2: TypeBounds if tp1.isStable =>
-              val ref1 = tp1.widenExpr.select(name)
-              isSubType(rinfo2.lo, ref1) && isSubType(ref1, rinfo2.hi)
-            case _ =>
-              false
-          }
-        case _ => false
-      }
 
-      def qualifies(m: SingleDenotation) =
-        isSubType(m.info.widenExpr, rinfo2.widenExpr) || matchAbstractTypeMember(m.info)
+      def qualifies(m: SingleDenotation): Boolean =
+        // If the member is an abstract type and the prefix is a path, compare the member itself
+        // instead of its bounds. This case is needed situations like:
+        //
+        //    class C { type T }
+        //    val foo: C
+        //    foo.type <: C { type T {= , <: , >:} foo.T }
+        //
+        // or like:
+        //
+        //    class C[T]
+        //    C[?] <: C[TV]
+        //
+        // where TV is a type variable. See i2397.scala for an example of the latter.
+        def matchAbstractTypeMember(info1: Type): Boolean = info1 match {
+          case TypeBounds(lo, hi) if lo ne hi =>
+            tp2.refinedInfo match {
+              case rinfo2: TypeBounds if tp1.isStable =>
+                val ref1 = tp1.widenExpr.select(name)
+                isSubType(rinfo2.lo, ref1) && isSubType(ref1, rinfo2.hi)
+              case _ =>
+                false
+            }
+          case _ => false
+        }
 
-      tp1.member(name) match { // inlined hasAltWith for performance
+        // An additional check for type member matching: If the refinement of the
+        // supertype `tp2` does not refer to a member symbol defined in the parent of `tp2`.
+        // then the symbol referred to in the subtype must have a signature that coincides
+        // in its parameters with the refinement's signature. The reason for the check
+        // is that if the refinement does not refer to a member symbol, we will have to
+        // resort to reflection to invoke the member. And reflection needs to know exact
+        // erased parameter types. See neg/i12211.scala.
+        def sigsOK(symInfo: Type, info2: Type) =
+          tp2.underlyingClassRef(refinementOK = true).member(name).exists
+          || symInfo.isInstanceOf[MethodType]
+              && symInfo.signature.consistentParams(info2.signature)
+
+        // A relaxed version of isSubType, which compares method types
+        // under the standard arrow rule which is contravarient in the parameter types,
+        // but under the condition that signatures might have to match (see sigsOK)
+        // This relaxed version is needed to correctly compare dependent function types.
+        // See pos/i12211.scala.
+        def isSubInfo(info1: Type, info2: Type, symInfo: Type): Boolean =
+          info2 match
+            case info2: MethodType =>
+              info1 match
+                case info1: MethodType =>
+                  val symInfo1 = symInfo.stripPoly
+                  matchingMethodParams(info1, info2, precise = false)
+                  && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType)
+                  && sigsOK(symInfo1, info2)
+                case _ => isSubType(info1, info2)
+            case _ => isSubType(info1, info2)
+
+        val info1 = m.info.widenExpr
+        isSubInfo(info1, tp2.refinedInfo.widenExpr, m.symbol.info.orElse(info1))
+        || matchAbstractTypeMember(m.info)
+      end qualifies
+
+      tp1.member(name) match // inlined hasAltWith for performance
         case mbr: SingleDenotation => qualifies(mbr)
         case mbr => mbr hasAltWith qualifies
-      }
     }
 
   final def ensureStableSingleton(tp: Type): SingletonType = tp.stripTypeVar match {
@@ -1841,15 +1871,20 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
   }
 
   /** Do the parameter types of `tp1` and `tp2` match in a way that allows `tp1`
-   *  to override `tp2` ? This is the case if they're pairwise `=:=`.
+   *  to override `tp2` ? Two modes: precise or not.
+   *  If `precise` is set (which is the default) this is the case if they're pairwise `=:=`.
+   *  Otherwise parameters in `tp2` must be subtypes of corresponding parameters in `tp1`.
    */
-  def matchingMethodParams(tp1: MethodType, tp2: MethodType): Boolean = {
+  def matchingMethodParams(tp1: MethodType, tp2: MethodType, precise: Boolean = true): Boolean = {
     def loop(formals1: List[Type], formals2: List[Type]): Boolean = formals1 match {
       case formal1 :: rest1 =>
         formals2 match {
           case formal2 :: rest2 =>
             val formal2a = if (tp2.isParamDependent) formal2.subst(tp2, tp1) else formal2
-            isSameTypeWhenFrozen(formal1, formal2a) && loop(rest1, rest2)
+            val paramsMatch =
+              if precise then isSameTypeWhenFrozen(formal1, formal2a)
+              else isSubTypeWhenFrozen(formal2a, formal1)
+            paramsMatch && loop(rest1, rest2)
           case nil =>
             false
         }

docs/docs/reference/changed-features/structural-types-spec.md

@@ -12,7 +12,7 @@ RefineStatSeq ::=  RefineStat {semi RefineStat}
 RefineStat    ::= ‘val’ VarDcl | ‘def’ DefDcl | ‘type’ {nl} TypeDcl
 ```
 
-## Implementation of structural types
+## Implementation of Structural Types
 
 The standard library defines a universal marker trait
 [`scala.Selectable`](https://github.com/lampepfl/dotty/blob/master/library/src/scala/Selectable.scala):
@@ -82,21 +82,49 @@ Note that `v`'s static type does not necessarily have to conform to `Selectable`
 conversion that can turn `v` into a `Selectable`, and the selection methods could also be available as
 [extension methods](../contextual/extension-methods.md).
 
-## Limitations of structural types
+## Limitations of Structural Types
 
 - Dependent methods cannot be called via structural call.
-- Overloaded methods cannot be called via structural call.
-- Refinements do not handle polymorphic methods.
 
-## Differences with Scala 2 structural types
+- Refinements may not introduce overloads: If a refinement specifies the signature
+  of a method `m`, and `m` is also defined in the parent type of the refinement, then
+  the new signature must properly override the existing one.
+
+- Subtyping of structural refinements must preserve erased parameter types: Assume
+  we want to prove `S <: T { def m(x: A): B }`. Then, as usual, `S` must have a member method `m` that can take an argument of type `A`. Furthermore, if `m` is not a member of `T` (i.e. the refinement is structural), an additional condition applies. In this case, the member _definition_ `m` of `S` will have a parameter
+  with type `A'` say. The additional condition is that the erasure of `A'` and `A` is the same. Here is an example:
+
+  ```scala
+  class Sink[A] { def put(x: A): Unit = {} }
+  val a = Sink[String]()
+  val b: { def put(x: String): Unit } = a  // error
+  b.put("abc") // looks for a method with a `String` parameter
+  ```
+  The second to last line is not well-typed, since the erasure of the parameter type of `put` in class `Sink` is `Object`, but the erasure of `put`'s parameter in the type of `b` is `String`. This additional condition is necessary, since we will have to resort to reflection to call a structural member like `put` in the type of `b` above. The condition ensures that the statically known parameter types of the refinement correspond up to erasure to the parameter types of the selected call target at runtime.
+
+  The usual reflection dispatch algorithms need to know exact erased parameter types. For instance, if the example above would typecheck, the call
+  `b.put("abc")` on the last line would look for a method `put` in the runtime type of `b` that takes a `String` parameter. But the `put` method is the one from class `Sink`, which takes an `Object` parameter. Hence the call would fail at runtime with a `NoSuchMethodException`.
+
+  One might hope for a "more intelligent" reflexive dispatch algorithm that does not require exact parameter type matching. Unfortunately, this can always run into ambiguities. For instance, continuing the example above, we might introduce a new subclass `Sink1` of `Sink` and change the definition of `a` as follows:
+
+  ```scala
+  class Sink1[A] extends Sink[A] { def put(x: "123") = ??? }
+  val a: Sink[String] = Sink1[String]()
+  ```
+
+  Now there are two `put` methods in the runtime type of `b` with erased parameter
+  types `Object` and `String`, respectively. Yet dynamic dispatch still needs to go
+  to the first `put` method, even though the second looks like a better match.
+
+## Differences with Scala 2 Structural Types
 
 - Scala 2 supports structural types by means of Java reflection. Unlike
   Scala 3, structural calls do not rely on a mechanism such as
   `Selectable`, and reflection cannot be avoided.
-- In Scala 2, structural calls to overloaded methods are possible.
+- In Scala 2, refinements can introduce overloads.
 - In Scala 2, mutable `var`s are allowed in refinements. In Scala 3,
   they are no longer allowed.
-
+- Scala 2 does not impose the "same-erasure" restriction on subtyping of structural types. It allows some calls to fail at runtime instead.
 
 ## Context
 

tasty/test/dotty/tools/tasty/TastyHeaderUnpicklerTest.scala

@@ -57,8 +57,8 @@ object TastyHeaderUnpicklerTest {
     buf.writeNat(exp)
     buf.writeNat(compilerBytes.length)
     buf.writeBytes(compilerBytes, compilerBytes.length)
-    buf.writeUncompressedLong(237478l)
-    buf.writeUncompressedLong(324789l)
+    buf.writeUncompressedLong(237478L)
+    buf.writeUncompressedLong(324789L)
     buf
   }
 

tests/neg/i12211.scala

@@ -0,0 +1,15 @@
+
+import reflect.Selectable.*
+
+val x: { def f(x: Any): String } = new { def f(x: Any) = x.toString }
+val y: { def f(x: String): String } = x  // error: type mismatch (different signatures)
+
+class Sink[A] { def put(x: A): Unit = {} }
+class Sink1[A] extends Sink[A] { def put(x: "123") = ??? }
+
+@main def Test =
+  println(y.f("abc"))
+  val a = new Sink[String]
+  val b: { def put(x: String): Unit } = a // error: type mismatch (different signatures)
+  b.put("") // gave a NoSuchMethodException: Sink.put(java.lang.String)
+  val c: Sink[String] = Sink1[String]()

tests/run/structuralNoSuchMethod.scala renamed to tests/neg/structuralNoSuchMethod.scala

@@ -11,10 +11,10 @@ object Test {
     def f(x: X, y: String): String = "f1"
   }
 
-  val x: T = new C[String]
+  val x: T = new C[String] // error
 
   def main(args: Array[String]) =
-    try println(x.f("", ""))  // throws NoSuchMethodException
+    try println(x.f("", ""))  // used to throw NoSuchMethodException
     catch {
       case ex: NoSuchMethodException =>
         println("no such method")

tests/pos/i12211.scala

@@ -0,0 +1,21 @@
+
+def fst0[A, B[_]](a: A)(b: B[a.type]): a.type = a
+
+def fst[A, B[_]]: (a: A) => (b: B[a.type]) => a.type =
+  (a: A) => (b: B[a.type]) => a
+
+def snd[A, B[_]]: (a: A) => () => (b: B[a.type]) => b.type =
+  (a: A) => () => (b: B[a.type]) => b
+
+def fst1[A, B[_]]: (a: A) => (b: B[a.type]) => a.type = fst0
+
+def test1[A, B[_]]: (a: A) => () => (b: B[a.type]) => Any =
+  snd[A, B]
+
+def test2[A, B[_]]: (a: A) => (b: B[a.type]) => A = fst[A, B]
+
+class AA
+class BB[T]
+
+def test3: (a: AA) => (b: BB[a.type]) => BB[?] =
+  (a: AA) => (b: BB[a.type]) => b

tests/run/enum-values.scala

@@ -50,7 +50,7 @@ enum ClassOnly: // this should still generate the `ordinal` and `fromOrdinal` co
         s"$c does not `eq` companion.fromOrdinal(${c.ordinal}), got ${companion.fromOrdinal(c.ordinal)}")
 
   def notFromOrdinal[T <: AnyRef & reflect.Enum](companion: FromOrdinal[T], compare: T): Unit =
-    cantFind(companion, compare.ordinal)
+    cantFind(companion.asInstanceOf[FromOrdinal[Any]], compare.ordinal)
 
   def cantFind[T](companion: FromOrdinal[T], ordinal: Int): Unit =
     try