Closes #7359 - Processing SAM with overridden methods of Object fails

neshkeev · neshkeev · commit f183cc010b72 · 2019-11-14T15:49:56.000+03:00
Dotty used to call the Types.abstractTermMembers method to find all the
methods that might be used as candidates for SAM. But the method ignores
the methods of java.lang.Object's when processing a list of a term's
members and returns all the members that have no implementation. This
used to cause the problem when there are overridden java.lang.Object
methods in a trait. Such methods receive implementation by default so they
cannot be considered as candidates for SAM methods. This problem prevents
Dotty from recognising some traits as SAM.

Now the Types.possibleSamMethods method is used in order to filter out
all the overridden java.lang.Object methods that does not have an
implementation yet.

This patch applies additional predicate to Types.abstractTermMembers to
filter out all the overridden java.lang.Object methods.

Signed-off-by: Nikita Eshkeev &lt;kastolom@gmail.com&gt;
diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala
@@ -846,7 +846,8 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma
     def addRemoteRemoteExceptionAnnotation: Unit = ()
 
     def samMethod(): Symbol = ctx.atPhase(ctx.erasurePhase) {
-      toDenot(sym).info.abstractTermMembers.toList match {
+      val samMethods = toDenot(sym).info.possibleSamMethods.toList
+      samMethods match {
         case x :: Nil => x.symbol
         case Nil => abort(s"${sym.show} is not a functional interface. It doesn't have abstract methods")
         case xs => abort(s"${sym.show} is not a functional interface. " +
diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala
@@ -37,6 +37,8 @@ import java.lang.ref.WeakReference
 import scala.annotation.internal.sharable
 import scala.annotation.threadUnsafe
 
+import dotty.tools.dotc.transform.SymUtils._
+
 object Types {
 
   @sharable private var nextId = 0
@@ -767,6 +769,26 @@ object Types {
           (name, buf) => buf ++= nonPrivateMember(name).altsWith(_.is(Deferred)))
     }
 
+    /**
+     * Returns the set of methods that are abstract and do not overlap with any of
+     * [[java.lang.Object]] methods.
+     *
+     * Conceptually, a SAM (functional interface) has exactly one abstract method.
+     * If an interface declares an abstract method overriding one of the public
+     * methods of [[java.lang.Object]], that also does not count toward the interface's
+     * abstract method count.
+     *
+     * @see https://docs.oracle.com/javase/8/docs/api/java/lang/FunctionalInterface.html
+     *
+     * @return the set of methods that are abstract and do not match any of [[java.lang.Object]]
+     *
+     */
+    final def possibleSamMethods(implicit ctx: Context): Seq[SingleDenotation] = {
+      record("possibleSamMethods")
+      abstractTermMembers
+        .filterNot(m => m.symbol.matchingMember(defn.ObjectType).exists || m.symbol.isSuperAccessor)
+    }
+
     /** The set of abstract type members of this type. */
     final def abstractTypeMembers(implicit ctx: Context): Seq[SingleDenotation] = {
       record("abstractTypeMembers")
@@ -4369,8 +4391,7 @@ object Types {
     }
     def unapply(tp: Type)(implicit ctx: Context): Option[MethodType] =
       if (isInstantiatable(tp)) {
-        val absMems = tp.abstractTermMembers
-        // println(s"absMems: ${absMems map (_.show) mkString ", "}")
+        val absMems = tp.possibleSamMethods
         if (absMems.size == 1)
           absMems.head.info match {
             case mt: MethodType if !mt.isParamDependent &&
diff --git a/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala b/compiler/src/dotty/tools/dotc/transform/ExpandSAMs.scala
@@ -46,7 +46,7 @@ class ExpandSAMs extends MiniPhase {
           tree
         case tpe =>
           val tpe1 = checkRefinements(tpe, fn)
-          val Seq(samDenot) = tpe1.abstractTermMembers.filter(!_.symbol.isSuperAccessor)
+          val Seq(samDenot) = tpe1.possibleSamMethods
           cpy.Block(tree)(stats,
               AnonClass(tpe1 :: Nil, fn.symbol.asTerm :: Nil, samDenot.symbol.asTerm.name :: Nil))
       }
diff --git a/project/Build.scala b/project/Build.scala
@@ -979,7 +979,7 @@ object Build {
           ++ (dir / "shared/src/test/scala/org/scalajs/testsuite/niobuffer" ** (("*.scala": FileFilter)  -- "ByteBufferTest.scala")).get
           ++ (dir / "shared/src/test/scala/org/scalajs/testsuite/niocharset" ** (("*.scala": FileFilter)  -- "BaseCharsetTest.scala" -- "Latin1Test.scala" -- "USASCIITest.scala" -- "UTF16Test.scala" -- "UTF8Test.scala")).get
           ++ (dir / "shared/src/test/scala/org/scalajs/testsuite/scalalib" ** (("*.scala": FileFilter)  -- "ArrayBuilderTest.scala" -- "ClassTagTest.scala" -- "EnumerationTest.scala" -- "RangesTest.scala" -- "SymbolTest.scala")).get
-          ++ (dir / "shared/src/test/require-sam" ** (("*.scala": FileFilter) -- "SAMTest.scala")).get
+          ++ (dir / "shared/src/test/require-sam" ** "*.scala").get
           ++ (dir / "shared/src/test/require-jdk8/org/scalajs/testsuite/compiler" ** (("*.scala": FileFilter) -- "DefaultMethodsTest.scala")).get
           ++ (dir / "shared/src/test/require-jdk8/org/scalajs/testsuite/javalib/lang" ** "*.scala").get
           ++ (dir / "shared/src/test/require-jdk8/org/scalajs/testsuite/javalib/util" ** (("*.scala": FileFilter) -- "CollectionsOnCopyOnWriteArrayListTestOnJDK8.scala")).get
diff --git a/tests/neg/i7359-b.check b/tests/neg/i7359-b.check
@@ -0,0 +1,4 @@
+-- [E105] Syntax Error: tests/neg/i7359-b.scala:3:6 --------------------------------------------------------------------
+3 |  def notifyAll(): Unit // error
+  |      ^
+  |      Traits cannot redefine final method notifyAll from class AnyRef.
diff --git a/tests/neg/i7359-b.scala b/tests/neg/i7359-b.scala
@@ -0,0 +1,3 @@
+trait SAMTrait
+  def first(): String
+  def notifyAll(): Unit // error
diff --git a/tests/neg/i7359-c.check b/tests/neg/i7359-c.check
@@ -0,0 +1,4 @@
+-- [E105] Syntax Error: tests/neg/i7359-c.scala:3:6 --------------------------------------------------------------------
+3 |  def wait(): Unit // error
+  |      ^
+  |      Traits cannot redefine final method wait from class AnyRef.
diff --git a/tests/neg/i7359-c.scala b/tests/neg/i7359-c.scala
@@ -0,0 +1,3 @@
+trait SAMTrait
+  def first(): String
+  def wait(): Unit // error
diff --git a/tests/neg/i7359-d.check b/tests/neg/i7359-d.check
@@ -0,0 +1,4 @@
+-- [E105] Syntax Error: tests/neg/i7359-d.scala:3:6 --------------------------------------------------------------------
+3 |  def wait(a: Long): Unit // error
+  |      ^
+  |      Traits cannot redefine final method wait from class AnyRef.
diff --git a/tests/neg/i7359-d.scala b/tests/neg/i7359-d.scala
@@ -0,0 +1,3 @@
+trait SAMTrait
+  def first(): String
+  def wait(a: Long): Unit // error
diff --git a/tests/neg/i7359-e.check b/tests/neg/i7359-e.check
@@ -0,0 +1,4 @@
+-- [E105] Syntax Error: tests/neg/i7359-e.scala:3:6 --------------------------------------------------------------------
+3 |  def wait(a: Long, n: Int): Unit // error
+  |      ^
+  |      Traits cannot redefine final method wait from class AnyRef.
diff --git a/tests/neg/i7359-e.scala b/tests/neg/i7359-e.scala
@@ -0,0 +1,3 @@
+trait SAMTrait
+  def first(): String
+  def wait(a: Long, n: Int): Unit // error
diff --git a/tests/neg/i7359-f.check b/tests/neg/i7359-f.check
@@ -0,0 +1,7 @@
+-- [E120] Duplicate Symbol Error: tests/neg/i7359-f.scala:1:6 ----------------------------------------------------------
+1 |trait SAMTrait // error
+  |      ^
+  |      Name clash between inherited members:
+  |      def equals: [T >: Boolean <: Boolean](obj: Any): T in trait SAMTrait at line 3 and
+  |      def equals(x$0: Any): Boolean in class Any
+  |      have the same type after erasure.
diff --git a/tests/neg/i7359-f.scala b/tests/neg/i7359-f.scala
@@ -0,0 +1,3 @@
+trait SAMTrait // error
+  def first(): String
+  def equals[T >: Boolean <: Boolean](obj: Any): T
diff --git a/tests/neg/i7359-g.check b/tests/neg/i7359-g.check
@@ -0,0 +1,5 @@
+-- [E007] Type Mismatch Error: tests/neg/i7359-g.scala:5:25 ------------------------------------------------------------
+5 |val m : SAMTrait = () => "Hello" // error
+  |                         ^^^^^^^
+  |                         Found:    () => String
+  |                         Required: SAMTrait
diff --git a/tests/neg/i7359-g.scala b/tests/neg/i7359-g.scala
@@ -0,0 +1,5 @@
+trait SAMTrait
+  def first(): String
+  def equals(obj: Int): Boolean
+
+val m : SAMTrait = () => "Hello" // error
diff --git a/tests/neg/i7359.check b/tests/neg/i7359.check
@@ -0,0 +1,4 @@
+-- [E105] Syntax Error: tests/neg/i7359.scala:3:6 ----------------------------------------------------------------------
+3 |  def notify(): Unit // error
+  |      ^
+  |      Traits cannot redefine final method notify from class AnyRef.
diff --git a/tests/neg/i7359.scala b/tests/neg/i7359.scala
@@ -0,0 +1,3 @@
+trait SAMTrait
+  def first(): String
+  def notify(): Unit // error
diff --git a/tests/pos/i7359.scala b/tests/pos/i7359.scala
@@ -0,0 +1,135 @@
+package test
+
+trait ObjectInterface
+  def equals(obj: Any): Boolean
+  def hashCode(): Int
+  def toString(): String
+
+trait SAMPlain
+  def first(): String
+
+trait SAMPlainWithOverriddenObjectMethods
+  def first(): String
+  def equals(obj: Any): Boolean
+  def hashCode(): Int
+  def toString(): String
+
+trait SAMPlainWithExtends extends ObjectInterface
+  def first(): String
+
+trait SAMPlainWithExtendsAndOverride extends ObjectInterface
+  def first(): String
+  override def equals(obj: Any): Boolean
+  override def hashCode(): Int
+  override def toString(): String
+
+trait SAMPlainCovariantOut[+O]
+  def first(): O
+
+trait SAMCovariantOut[+O]
+  def first(): O
+  def equals(obj: Any): Boolean
+  def hashCode(): Int
+  def toString(): String
+
+trait SAMCovariantOutExtends[+O] extends ObjectInterface
+  def first(): O
+
+trait SAMCovariantOutExtendsAndOverride[+O] extends ObjectInterface
+  def first(): O
+  override def equals(obj: Any): Boolean
+  override def hashCode(): Int
+  override def toString(): String
+
+trait SAMPlainContravariantIn[-I]
+  def first(in: I): Unit
+
+trait SAMContravariantIn[-I]
+  def first(in: I): Unit
+  def equals(obj: Any): Boolean
+  def hashCode(): Int
+  def toString(): String
+
+trait SAMContravariantInExtends[-I] extends ObjectInterface
+  def first(in: I): Unit
+
+trait SAMContravariantInExtendsAndOverride[-I] extends ObjectInterface
+  def first(in: I): Unit
+  override def equals(obj: Any): Boolean
+  override def hashCode(): Int
+  override def toString(): String
+
+trait SAMPlainInvariant[T]
+  def first(in: T): T
+
+trait SAMInvariant[T]
+  def first(in: T): T
+  def equals(obj: Any): Boolean
+  def hashCode(): Int
+  def toString(): String
+
+trait SAMInvariantExtends[T] extends ObjectInterface
+  def first(in: T): T
+
+trait SAMInvariantExtendsAndOverride[T] extends ObjectInterface
+  def first(in: T): T
+  override def equals(obj: Any): Boolean
+  override def hashCode(): Int
+  override def toString(): String
+
+trait SAMPlainInOut[-I, +O]
+  def first(in: I): O
+
+trait SAMInOut[-I, +O]
+  def first(in: I): O
+  def equals(obj: Any): Boolean
+  def hashCode(): Int
+  def toString(): String
+
+trait SAMInOutExtends[-I, +O] extends ObjectInterface
+  def first(in: I): O
+
+trait SAMInOutExtendsAndOverride[-I, +O] extends ObjectInterface
+  def first(in: I): O
+  override def equals(obj: Any): Boolean
+  override def hashCode(): Int
+  override def toString(): String
+
+type CustomString = String
+type CustomBoolean = Boolean
+type CustomInt = Int
+
+trait SAMWithCustomAliases
+  def first(): String
+  def equals(obj: Any): CustomBoolean
+  def hashCode(): CustomInt
+  def toString(): CustomString
+
+object Main
+  def main(args: Array[String]) =
+    val samPlain : SAMPlain = () => "Hello, World!"
+    val samPlainWithOverriddenObjectMethods: SAMPlainWithOverriddenObjectMethods = () => "Hello, World!"
+    val samPlainWithExtends : SAMPlainWithExtends = () => "Hello, World!"
+    val samPlainWithExtendsAndOverride : SAMPlainWithExtendsAndOverride = () => "Hello, World!"
+
+    val samPlainCovariantOut : SAMPlainCovariantOut[_] = () => "Hello, World!"
+    val samCovariantOut : SAMCovariantOut[_] = () => "Hello, World!"
+    val samCovariantOutExtends : SAMCovariantOutExtends[_] = () => "Hello, World!"
+    val samCovariantOutExtendsAndOverride : SAMCovariantOutExtendsAndOverride[_] = () => "Hello, World!"
+
+    val samPlainContravariantIn : SAMPlainContravariantIn[Int] = (x: Int) => ()
+    val samContravariantIn : SAMContravariantIn[Int] = (x: Int) => ()
+    val samContravariantInExtends : SAMContravariantInExtends[Int] = (x: Int) => ()
+    val samContravariantInExtendsAndOverride : SAMContravariantInExtendsAndOverride[Int] = (x: Int) => ()
+
+    val samPlainInvariant : SAMPlainInvariant[String] = (x: String) => x
+    val samInvariant : SAMInvariant[String] = (x: String) => x
+    val samInvariantExtends : SAMInvariantExtends[String] = (x: String) => x
+    val samInvariantExtendsAndOverride : SAMInvariantExtendsAndOverride[String] = (x: String) => x
+
+    val samPlainInOut : SAMPlainInOut[Int, String] = (x: Int) => x.toString
+    val samInOut : SAMInOut[Int, String] = (x: Int) => x.toString
+    val samInOutExtends : SAMInOutExtends[Int, String] = (x: Int) => x.toString
+    val samInOutExtendsAndOverride : SAMInOutExtendsAndOverride[Int, String] = (x: Int) => x.toString
+
+    val samWithCustomAliases : SAMWithCustomAliases = () => "Hello, World!"
diff --git a/tests/run/i7359.check b/tests/run/i7359.check
@@ -0,0 +1,6 @@
+o
+o
+()
+i
+10
+o
diff --git a/tests/run/i7359.scala b/tests/run/i7359.scala
@@ -0,0 +1,46 @@
+trait ObjectInterface
+  def equals(obj: Any): Boolean
+  def hashCode(): Int
+  def toString(): String
+
+trait SAMPlainWithExtends extends ObjectInterface
+  def first(): String
+
+trait SAMCovariantOutExtends[+O] extends ObjectInterface
+  def first(): O
+
+trait SAMContravariantInExtends[-I] extends ObjectInterface
+  def first(in: I): Unit
+
+trait SAMInvariantExtends[T] extends ObjectInterface
+  def first(in: T): T
+
+trait SAMInOutExtends[-I, +O] extends ObjectInterface
+  def first(in: I): O
+
+type CustomString = String
+type CustomBoolean = Boolean
+type CustomInt = Int
+
+trait SAMWithCustomAliases
+  def first(): String
+  def equals(obj: Any): CustomBoolean
+  def hashCode(): CustomInt
+  def toString(): CustomString
+
+object Test
+
+  def main(args: Array[String]): Unit =
+    val samPlainWithExtends : SAMPlainWithExtends = () => "o"
+    val samCovariantOutExtends : SAMCovariantOutExtends[_] = () => "o"
+    val samContravariantInExtends : SAMContravariantInExtends[Int] = (x: Int) => ()
+    val samInvariantExtends : SAMInvariantExtends[String] = (x: String) => x
+    val samInOutExtends : SAMInOutExtends[Int, String] = (x: Int) => x.toString
+    val samWithCustomAliases : SAMWithCustomAliases = () => "o"
+
+    println(samPlainWithExtends.first())
+    println(samCovariantOutExtends.first())
+    println(samContravariantInExtends.first(10))
+    println(samInvariantExtends.first("i"))
+    println(samInOutExtends.first(10))
+    println(samWithCustomAliases.first())

Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ class ExpandSAMs extends MiniPhase {`
`46`	`46`	`tree`
`47`	`47`	`case tpe =>`
`48`	`48`	`val tpe1 = checkRefinements(tpe, fn)`
`49`		`- val Seq(samDenot) = tpe1.abstractTermMembers.filter(!_.symbol.isSuperAccessor)`
	`49`	`+ val Seq(samDenot) = tpe1.possibleSamMethods`
`50`	`50`	`cpy.Block(tree)(stats,`
`51`	`51`	`AnonClass(tpe1 :: Nil, fn.symbol.asTerm :: Nil, samDenot.symbol.asTerm.name :: Nil))`
`52`	`52`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+trait SAMTrait`
	`2`	`+ def first(): String`
	`3`	`+ def notifyAll(): Unit // error`