More fixes based on feedback by Lukas

Crucially, the fully-defined expected type must be checked for
conformance to the original expected type!!

The logic in adaptToSam that checks whether pt is fully defined
probably needs some more thought. See pos/t8310 for a good test
case. Argument type checking is a challenge, as we first check
against a lenient pt (this lenient expected type has wildcards,
and thus is not fully defined, but we should still consider sam
adaptation a success even if we end up with wildcards for some
unknown type parameters, they should be determined later).

13 files changed, 58 insertions, 23 deletions

diff --git a/spec/03-types.md b/spec/03-types.md
index 16c48bcf6c..e2a6523dff 100644
--- a/spec/03-types.md
+++ b/spec/03-types.md
@@ -986,7 +986,7 @@ def foo(x: ToString): Unit
 trait ToString { def convert(x: Int): String }
 ```
 
-The application `foo(_.toString)` [resolves](06-expressions.html#overloading-resolution) to the first overload,
+The application `foo((x: Int) => x.toString)` [resolves](06-expressions.html#overloading-resolution) to the first overload,
 as it's more specific:
   - `Int => String` is compatible to `ToString` -- when expecting a value of type `ToString`, you may pass a function literal from `Int` to `String`, as it will be SAM-converted to said function;
   - `ToString` is not compatible to `Int => String` -- when expecting a function from `Int` to `String`, you may not pass a `ToString`.
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index f581eab00f..4f006fe9a9 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -2726,7 +2726,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       *
       * ```
       *   new S {
-      *    <static> def apply$body(p1: T1, ..., pN: TN): T = body
+      *    def apply$body(p1: T1, ..., pN: TN): T = body
       *    def apply(p1: T1', ..., pN: TN'): T' = apply$body(p1,..., pN)
       *   }
       * ```
@@ -2737,27 +2737,28 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
       * The types T1' ... TN' and T' are derived from the method signature of the sam method,
       * as seen from the fully defined `samClassTpFullyDefined`.
       *
-      * The function's body is put in a static method in the class definition to enforce scoping.
+      * The function's body is put in a (static) method in the class definition to enforce scoping.
       * S's members should not be in scope in `body`. (Putting it in the block outside the class runs into implementation problems described below)
       *
       * The restriction on implicit arguments (neither S's constructor, nor sam may take an implicit argument list),
-      * is largely to keep the implementation of type inference (the computation of `samClassTpFullyDefined`) simple.
+      * is to keep the implementation of type inference (the computation of `samClassTpFullyDefined`) simple.
+      *
+      * Impl notes:
+      *   - `fun` has a FunctionType, but the expected type `pt` is some SAM type -- let's remedy that
+      *   - `fun` is fully attributed, so we'll have to wrangle some symbols into shape (owner change, vparam syms)
+      *   - after experimentation, it works best to type check function literals fully first and then adapt to a sam type,
+      *     as opposed to a sam-specific code paths earlier on in type checking (in typedFunction).
+      *     For one, we want to emit the same bytecode regardless of whether the expected
+      *     function type is a built-in FunctionN or some SAM type
       *
       */
     def adaptToSAM(sam: Symbol, fun: Function, pt: Type, mode: Mode): Tree = {
-      // `fun` has a FunctionType, but the expected type `pt` is some SAM type -- let's remedy that
-      // `fun` is fully attributed, so we'll have to wrangle some symbols into shape (owner change, vparam syms)
-      // I tried very hard to leave `fun` untyped and rework everything into the right shape and type check once,
-      // but couldn't make it work due to retyping that happens down the line
-      // (implicit conversion retries/retypechecking, CBN transform, super call arg context nesting weirdness)
-
-      def funTpMatchesExpected(pt: Type): Boolean = isFullyDefined(pt) && {
-        // what's the signature of the method that we should actually be overriding?
+      def fullyDefinedMeetsExpectedFunTp(pt: Type): Boolean = isFullyDefined(pt) && {
         val samMethType = pt memberInfo sam
         fun.tpe <:< functionType(samMethType.paramTypes, samMethType.resultType)
       }
 
-      if (funTpMatchesExpected(pt)) fun.setType(pt)
+      if (fullyDefinedMeetsExpectedFunTp(pt)) fun.setType(pt)
       else try {
         val samClassSym = pt.typeSymbol
 
@@ -2786,9 +2787,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
 
         debuglog(s"sam infer: $pt --> ${appliedType(samTyCon, targs)} by ${fun.tpe} <:< $samInfoWithTVars --> $targs for $tparams")
 
-        // a fully defined samClassTp
         val ptFullyDefined = appliedType(samTyCon, targs)
-        if (funTpMatchesExpected(ptFullyDefined)) {
+        if (ptFullyDefined <:< pt && fullyDefinedMeetsExpectedFunTp(ptFullyDefined)) {
           debuglog(s"sam fully defined expected type: $ptFullyDefined from $pt for ${fun.tpe}")
           fun.setType(ptFullyDefined)
         } else {
diff --git a/test/files/neg/sammy_expected.check b/test/files/neg/sammy_expected.check
new file mode 100644
index 0000000000..3b76aabdd2
--- /dev/null
+++ b/test/files/neg/sammy_expected.check
@@ -0,0 +1,6 @@
+sammy_expected.scala:4: error: type mismatch;
+ found   : String => Int
+ required: F[Object,Int]
+  def wrong: F[Object, Int] = (x: String) => 1
+                                          ^
+one error found
diff --git a/test/files/neg/sammy_expected.scala b/test/files/neg/sammy_expected.scala
new file mode 100644
index 0000000000..8fc1f66ff7
--- /dev/null
+++ b/test/files/neg/sammy_expected.scala
@@ -0,0 +1,5 @@
+trait F[A, B] { def apply(x: A): B }
+
+class MustMeetExpected {
+  def wrong: F[Object, Int] = (x: String) => 1
+}
\ No newline at end of file
diff --git a/test/files/neg/sammy_overload.check b/test/files/neg/sammy_overload.check
new file mode 100644
index 0000000000..903d7c88f4
--- /dev/null
+++ b/test/files/neg/sammy_overload.check
@@ -0,0 +1,7 @@
+sammy_overload.scala:11: error: missing parameter type for expanded function ((x$1: <error>) => x$1.toString)
+  O.m(_.toString) // error expected: eta-conversion breaks down due to overloading
+      ^
+sammy_overload.scala:12: error: missing parameter type
+  O.m(x => x) // error expected: needs param type
+      ^
+two errors found
diff --git a/test/files/neg/sammy_overload.scala b/test/files/neg/sammy_overload.scala
new file mode 100644
index 0000000000..91c52cf96c
--- /dev/null
+++ b/test/files/neg/sammy_overload.scala
@@ -0,0 +1,13 @@
+trait ToString { def convert(x: Int): String }
+
+class ExplicitSamType {
+  object O {
+    def m(x: Int => String): Int = 0
+    def m(x: ToString): Int = 1
+  }
+
+  O.m((x: Int) => x.toString) // ok, function type takes precedence
+
+  O.m(_.toString) // error expected: eta-conversion breaks down due to overloading
+  O.m(x => x) // error expected: needs param type
+}
diff --git a/test/files/pos/sam_ctor_arg.scala b/test/files/pos/sammy_ctor_arg.scala
index 3c556d59f0..3c556d59f0 100644
--- a/test/files/pos/sam_ctor_arg.scala
+++ b/test/files/pos/sammy_ctor_arg.scala
diff --git a/test/files/pos/sam_infer_argtype_subtypes.scala b/test/files/pos/sammy_infer_argtype_subtypes.scala
index 63966f879e..63966f879e 100644
--- a/test/files/pos/sam_infer_argtype_subtypes.scala
+++ b/test/files/pos/sammy_infer_argtype_subtypes.scala
diff --git a/test/files/pos/sam_inferargs.scala b/test/files/pos/sammy_inferargs.scala
index 10d9b4f0dd..10d9b4f0dd 100644
--- a/test/files/pos/sam_inferargs.scala
+++ b/test/files/pos/sammy_inferargs.scala
diff --git a/test/files/run/sammy_repeated.scala b/test/files/run/sammy_repeated.scala
deleted file mode 100644
index c24dc41909..0000000000
--- a/test/files/run/sammy_repeated.scala
+++ /dev/null
@@ -1,8 +0,0 @@
-trait RepeatedSink { def accept(a: Any*): Unit }
-
-object Test {
-  def main(args: Array[String]): Unit = {
-    val f: RepeatedSink = (a) => println(a)
-    f.accept(1)
-  }
-}
\ No newline at end of file
diff --git a/test/files/run/sam_return.scala b/test/files/run/sammy_return.scala
index e959619dd1..e959619dd1 100644
--- a/test/files/run/sam_return.scala
+++ b/test/files/run/sammy_return.scala
diff --git a/test/files/run/sammy_repeated.check b/test/files/run/sammy_vararg_cbn.check
index 1cff0f067c..1cff0f067c 100644
--- a/test/files/run/sammy_repeated.check
+++ b/test/files/run/sammy_vararg_cbn.check
diff --git a/test/files/run/sammy_vararg_cbn.scala b/test/files/run/sammy_vararg_cbn.scala
new file mode 100644
index 0000000000..e5b49498ea
--- /dev/null
+++ b/test/files/run/sammy_vararg_cbn.scala
@@ -0,0 +1,12 @@
+trait SamRepeated { def accept(a: Any*): Unit }
+trait SamByName { def accept(a: => Any): (Any, Any) }
+
+object Test extends App {
+  val rep: SamRepeated = (a) => println(a)
+  rep.accept(1)
+
+  val nam: SamByName = (a) => (a, a)
+  var v = 0
+  assert(nam.accept({v += 1; v}) == (1, 2))
+  assert(v == 2, "by name arg should be evaluated twice")
+}