From 040c0434d456dd75a174147d8a0c4cab37266ba6 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Thu, 17 Mar 2016 17:16:28 -0700 Subject: 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). --- spec/03-types.md | 2 +- .../scala/tools/nsc/typechecker/Typers.scala | 28 +++++++++++----------- test/files/neg/sammy_expected.check | 6 +++++ test/files/neg/sammy_expected.scala | 5 ++++ test/files/neg/sammy_overload.check | 7 ++++++ test/files/neg/sammy_overload.scala | 13 ++++++++++ test/files/pos/sam_ctor_arg.scala | 4 ---- test/files/pos/sam_infer_argtype_subtypes.scala | 6 ----- test/files/pos/sam_inferargs.scala | 6 ----- test/files/pos/sammy_ctor_arg.scala | 4 ++++ test/files/pos/sammy_infer_argtype_subtypes.scala | 6 +++++ test/files/pos/sammy_inferargs.scala | 6 +++++ test/files/run/sam_return.scala | 14 ----------- test/files/run/sammy_repeated.check | 1 - test/files/run/sammy_repeated.scala | 8 ------- test/files/run/sammy_return.scala | 14 +++++++++++ test/files/run/sammy_vararg_cbn.check | 1 + test/files/run/sammy_vararg_cbn.scala | 12 ++++++++++ 18 files changed, 89 insertions(+), 54 deletions(-) create mode 100644 test/files/neg/sammy_expected.check create mode 100644 test/files/neg/sammy_expected.scala create mode 100644 test/files/neg/sammy_overload.check create mode 100644 test/files/neg/sammy_overload.scala delete mode 100644 test/files/pos/sam_ctor_arg.scala delete mode 100644 test/files/pos/sam_infer_argtype_subtypes.scala delete mode 100644 test/files/pos/sam_inferargs.scala create mode 100644 test/files/pos/sammy_ctor_arg.scala create mode 100644 test/files/pos/sammy_infer_argtype_subtypes.scala create mode 100644 test/files/pos/sammy_inferargs.scala delete mode 100644 test/files/run/sam_return.scala delete mode 100644 test/files/run/sammy_repeated.check delete mode 100644 test/files/run/sammy_repeated.scala create mode 100644 test/files/run/sammy_return.scala create mode 100644 test/files/run/sammy_vararg_cbn.check create mode 100644 test/files/run/sammy_vararg_cbn.scala 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 { - * 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: ) => 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/sam_ctor_arg.scala deleted file mode 100644 index 3c556d59f0..0000000000 --- a/test/files/pos/sam_ctor_arg.scala +++ /dev/null @@ -1,4 +0,0 @@ -trait Fun[A, B] { def apply(a: A): B } -// can't do sam expansion until the sam body def is a static method in the sam class, and not a local method in a block' -class C(f: Fun[Int, String]) -class Test extends C(s => "a") \ No newline at end of file diff --git a/test/files/pos/sam_infer_argtype_subtypes.scala b/test/files/pos/sam_infer_argtype_subtypes.scala deleted file mode 100644 index 63966f879e..0000000000 --- a/test/files/pos/sam_infer_argtype_subtypes.scala +++ /dev/null @@ -1,6 +0,0 @@ -trait Fun[A, B] { def apply(a: A): B } - -class SamInferResult { - def foreach[U](f: Fun[String, U]): U = ??? - def foo = foreach(println) -} \ No newline at end of file diff --git a/test/files/pos/sam_inferargs.scala b/test/files/pos/sam_inferargs.scala deleted file mode 100644 index 10d9b4f0dd..0000000000 --- a/test/files/pos/sam_inferargs.scala +++ /dev/null @@ -1,6 +0,0 @@ -trait Proc { def apply(): Unit } -class Test { - val initCode = List[Proc]() - initCode foreach { proc => proc() } - -} diff --git a/test/files/pos/sammy_ctor_arg.scala b/test/files/pos/sammy_ctor_arg.scala new file mode 100644 index 0000000000..3c556d59f0 --- /dev/null +++ b/test/files/pos/sammy_ctor_arg.scala @@ -0,0 +1,4 @@ +trait Fun[A, B] { def apply(a: A): B } +// can't do sam expansion until the sam body def is a static method in the sam class, and not a local method in a block' +class C(f: Fun[Int, String]) +class Test extends C(s => "a") \ No newline at end of file diff --git a/test/files/pos/sammy_infer_argtype_subtypes.scala b/test/files/pos/sammy_infer_argtype_subtypes.scala new file mode 100644 index 0000000000..63966f879e --- /dev/null +++ b/test/files/pos/sammy_infer_argtype_subtypes.scala @@ -0,0 +1,6 @@ +trait Fun[A, B] { def apply(a: A): B } + +class SamInferResult { + def foreach[U](f: Fun[String, U]): U = ??? + def foo = foreach(println) +} \ No newline at end of file diff --git a/test/files/pos/sammy_inferargs.scala b/test/files/pos/sammy_inferargs.scala new file mode 100644 index 0000000000..10d9b4f0dd --- /dev/null +++ b/test/files/pos/sammy_inferargs.scala @@ -0,0 +1,6 @@ +trait Proc { def apply(): Unit } +class Test { + val initCode = List[Proc]() + initCode foreach { proc => proc() } + +} diff --git a/test/files/run/sam_return.scala b/test/files/run/sam_return.scala deleted file mode 100644 index e959619dd1..0000000000 --- a/test/files/run/sam_return.scala +++ /dev/null @@ -1,14 +0,0 @@ -trait Fun[A, B] { def apply(a: A): B } -class PF[A, B] { def runWith[U](action: Fun[B, U]): Fun[A, Boolean] = a => {action(a.asInstanceOf[B]); true} } - -class TO[A](x: A) { - def foreach[U](f: Fun[A, U]): U = f(x) - def collectFirst[B](pf: PF[A, B]): Option[B] = { - foreach(pf.runWith(b => return Some(b))) - None - } -} - -object Test extends App { - assert(new TO("a").collectFirst(new PF[String, String]).get == "a") -} \ No newline at end of file diff --git a/test/files/run/sammy_repeated.check b/test/files/run/sammy_repeated.check deleted file mode 100644 index 1cff0f067c..0000000000 --- a/test/files/run/sammy_repeated.check +++ /dev/null @@ -1 +0,0 @@ -WrappedArray(1) 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/sammy_return.scala b/test/files/run/sammy_return.scala new file mode 100644 index 0000000000..e959619dd1 --- /dev/null +++ b/test/files/run/sammy_return.scala @@ -0,0 +1,14 @@ +trait Fun[A, B] { def apply(a: A): B } +class PF[A, B] { def runWith[U](action: Fun[B, U]): Fun[A, Boolean] = a => {action(a.asInstanceOf[B]); true} } + +class TO[A](x: A) { + def foreach[U](f: Fun[A, U]): U = f(x) + def collectFirst[B](pf: PF[A, B]): Option[B] = { + foreach(pf.runWith(b => return Some(b))) + None + } +} + +object Test extends App { + assert(new TO("a").collectFirst(new PF[String, String]).get == "a") +} \ No newline at end of file diff --git a/test/files/run/sammy_vararg_cbn.check b/test/files/run/sammy_vararg_cbn.check new file mode 100644 index 0000000000..1cff0f067c --- /dev/null +++ b/test/files/run/sammy_vararg_cbn.check @@ -0,0 +1 @@ +WrappedArray(1) 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") +} -- cgit v1.2.3