From fb5ed85793747fa1d2b4ced8d41bb2f8c2a02e53 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 15:07:55 +0200 Subject: [PATCH 1/9] Always do eta expansion if arity >= 1 Implements another part of #2570 --- .../src/dotty/tools/dotc/typer/Typer.scala | 8 ++++++-- tests/pos/i2570.scala | 19 ++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 378cd97cf7cd..9478c9178bd8 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2006,8 +2006,12 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit // prioritize method parameter types as parameter types of the eta-expanded closure 0 else defn.functionArity(ptNorm) - else if (pt eq AnyFunctionProto) wtp.paramInfos.length - else -1 + else { + val nparams = wtp.paramInfos.length + if (nparams > 0 || pt.eq(AnyFunctionProto)) nparams + else -1 // no eta expansion in this case + } + if (arity >= 0 && !tree.symbol.isConstructor) typed(etaExpand(tree, wtp, arity), pt) else if (wtp.paramInfos.isEmpty) diff --git a/tests/pos/i2570.scala b/tests/pos/i2570.scala index 1fa9a07567e3..350a4daf04e8 100644 --- a/tests/pos/i2570.scala +++ b/tests/pos/i2570.scala @@ -21,9 +21,18 @@ object Test { def sum2(x: Int, ys: Int*) = (x /: ys)(_ + _) val h1: ((Int, Seq[Int]) => Int) = sum2 -// Not yet: -// val h1 = repeat -// val h2: (String, Int, Int) = h1 -// val h3 = sum -// val h4: (Int, => Int) = h3 + val h3 = repeat + val h4: ((String, Int, Int) => String) = h3 + val h5 = sum + val h6: ((Int, => Int) => Int) = h5 + + class A + class B + class C + implicit object b extends B + + def foo(x: A)(implicit bla: B): C = ??? + + val h7 = foo + val h8: A => C = h7 } From 1ee79345f45ef5d47437b488a20f90393dff46c2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 16:34:52 +0200 Subject: [PATCH 2/9] Don't eta expand in Pattern mode. --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9478c9178bd8..1c94ef11ea22 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2012,7 +2012,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit else -1 // no eta expansion in this case } - if (arity >= 0 && !tree.symbol.isConstructor) + if (arity >= 0 && !tree.symbol.isConstructor && !ctx.mode.is(Mode.Pattern)) typed(etaExpand(tree, wtp, arity), pt) else if (wtp.paramInfos.isEmpty) adaptInterpolated(tpd.Apply(tree, Nil), pt, EmptyTree) From 87c5ac099ca953661b1fea44dfed465e51975c2a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 16:35:01 +0200 Subject: [PATCH 3/9] Fix tests. --- tests/neg/i1503.scala | 2 +- tests/neg/i1716.scala | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/neg/i1503.scala b/tests/neg/i1503.scala index 8e5dc53c6813..8433d6d35853 100644 --- a/tests/neg/i1503.scala +++ b/tests/neg/i1503.scala @@ -9,6 +9,6 @@ object Test { def main(args: Array[String]) = { (if (cond) foo1 else bar1)() // error: Unit does not take parameters - (if (cond) foo2 else bar2)(22) // error: missing arguments // error: missing arguments + (if (cond) foo2 else bar2)(22) // OK after changes to eta expansion } } diff --git a/tests/neg/i1716.scala b/tests/neg/i1716.scala index 1a3fd71d0a77..fab79c833e4c 100644 --- a/tests/neg/i1716.scala +++ b/tests/neg/i1716.scala @@ -1,9 +1,10 @@ object Fail { def f(m: Option[Int]): Unit = { m match { - case x @ Some[_] => // error + case x @ Some[_] => // error: unbound wildcard type case _ => } } - Some[_] // error + Some[_] // error: unbound wildcard type + } From 7e2627c460893c3a27f0e75d394dde05a33ab25f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 22:22:30 +0200 Subject: [PATCH 4/9] Avoid over-eager eta-expansion For neg/typedapply.scala we had an infinite sequence of eta-expansions, followed by `.apply` selections. --- compiler/src/dotty/tools/dotc/ast/TreeInfo.scala | 13 +++++++++++++ compiler/src/dotty/tools/dotc/typer/Typer.scala | 10 +++++++--- tests/neg/typedapply.scala | 12 +++++++++++- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index f3bce4000079..6458fb11d516 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -280,6 +280,13 @@ trait UntypedTreeInfo extends TreeInfo[Untyped] { self: Trees.Instance[Untyped] case _ => false } + /** Is tree a Function or Closure node, possibly preceded by definitions? (currently unused) */ + def isClosure(tree: Tree)(implicit ctx: Context): Boolean = unsplice(tree) match { + case _: Function | _: Closure => true + case Block(_, expr) => isClosure(expr) + case _ => false + } + /** Is `tree` an implicit function or closure, possibly nested in a block? */ def isImplicitClosure(tree: Tree)(implicit ctx: Context): Boolean = unsplice(tree) match { case Function((param: untpd.ValDef) :: _, _) => param.mods.is(Implicit) @@ -470,6 +477,12 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => case _ => false } + /** Is tree a compiler-generated `.apply` node? */ + def isSyntheticApply(tree: Tree): Boolean = tree match { + case Select(qual, nme.apply) => tree.pos.end == qual.pos.end + case _ => false + } + /** Strips layers of `.asInstanceOf[T]` / `_.$asInstanceOf[T]()` from an expression */ def stripCast(tree: Tree)(implicit ctx: Context): Tree = { def isCast(sel: Tree) = sel.symbol == defn.Any_asInstanceOf diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 1c94ef11ea22..a3a988db4fc1 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2012,12 +2012,16 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit else -1 // no eta expansion in this case } - if (arity >= 0 && !tree.symbol.isConstructor && !ctx.mode.is(Mode.Pattern)) + if (wtp.isImplicit) + err.typeMismatch(tree, pt) + else if (arity >= 0 && + !tree.symbol.isConstructor && + !ctx.mode.is(Mode.Pattern) && + !isApplyProto(pt) && + !isSyntheticApply(tree)) typed(etaExpand(tree, wtp, arity), pt) else if (wtp.paramInfos.isEmpty) adaptInterpolated(tpd.Apply(tree, Nil), pt, EmptyTree) - else if (wtp.isImplicit) - err.typeMismatch(tree, pt) else missingArgs case _ => diff --git a/tests/neg/typedapply.scala b/tests/neg/typedapply.scala index 706b05da3592..bddbe73e54c1 100644 --- a/tests/neg/typedapply.scala +++ b/tests/neg/typedapply.scala @@ -6,8 +6,18 @@ object typedapply { foo[Int, String, String](1, "abc") // error: wrong number of type parameters + def fuz(x: Int, y: String) = (x, y) + + val x1 = fuz.curried // OK + val x2: Int => String => (Int, String) = x1 + def bar(x: Int) = x - bar[Int](1) // error: does not take parameters + bar[Int](1) // error: does not take type parameters + + bar.baz // error: baz is not a member of Int => Int + } + + From 8b9b68c4c7cdd92508afa6fb2dbe79f700e7cc8b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 6 Jun 2017 22:23:07 +0200 Subject: [PATCH 5/9] Fix error message "Expected:" always came out empty: `[]`. --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 3ecdbdb85746..b3bd9e43cfe3 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -668,8 +668,7 @@ object messages { private val msgPrefix = if (actualCount > expectedCount) "Too many" else "Not enough" //TODO add def simpleParamName to ParamInfo - private val expectedArgString = fntpe - .widen.typeParams + private val expectedArgString = expectedArgs .map(_.paramName.unexpandedName.show) .mkString("[", ", ", "]") From cc88c5dcd0edf686804a3de738c1302b8aec5b1a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 7 Jun 2017 13:59:07 +0200 Subject: [PATCH 6/9] Fine-tune condition when to eta expand --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 4 +++- .../dotty/tools/dotc/core/Definitions.scala | 4 ++-- .../src/dotty/tools/dotc/typer/Typer.scala | 24 ++++++++++++------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 6458fb11d516..0af2f9744b13 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -477,7 +477,9 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => case _ => false } - /** Is tree a compiler-generated `.apply` node? */ + /** Is tree a compiler-generated `.apply` node that refers to the + * apply of a function class? (implicit functions are excluded) + */ def isSyntheticApply(tree: Tree): Boolean = tree match { case Select(qual, nme.apply) => tree.pos.end == qual.pos.end case _ => false diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 5e58e60c3603..e063c047167a 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -809,8 +809,8 @@ class Definitions { */ def erasedFunctionClass(cls: Symbol): Symbol = { val arity = scalaClassName(cls).functionArity - if (arity > 22) defn.FunctionXXLClass - else if (arity >= 0) defn.FunctionClass(arity) + if (arity > 22) FunctionXXLClass + else if (arity >= 0) FunctionClass(arity) else NoSymbol } diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index a3a988db4fc1..29c02ee13e22 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2011,17 +2011,25 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit if (nparams > 0 || pt.eq(AnyFunctionProto)) nparams else -1 // no eta expansion in this case } - - if (wtp.isImplicit) - err.typeMismatch(tree, pt) - else if (arity >= 0 && - !tree.symbol.isConstructor && - !ctx.mode.is(Mode.Pattern) && - !isApplyProto(pt) && - !isSyntheticApply(tree)) + def isExpandableApply = + defn.isImplicitFunctionClass(tree.symbol.maybeOwner) && defn.isFunctionType(ptNorm) + + // Reasons NOT to eta expand: + // - we reference a constructor + // - we are in a patterm + // - the current tree is a synthetic non-implicit apply (eta-expasion would simply undo that) + // - the current tree is a synthetic implicit apply and the expected + // type is a function type (this rule is needed so we can pass an implicit function + // to a regular function type) + if (arity >= 0 && + !tree.symbol.isConstructor && + !ctx.mode.is(Mode.Pattern) && + !(isSyntheticApply(tree) && !isExpandableApply)) typed(etaExpand(tree, wtp, arity), pt) else if (wtp.paramInfos.isEmpty) adaptInterpolated(tpd.Apply(tree, Nil), pt, EmptyTree) + else if (wtp.isImplicit) + err.typeMismatch(tree, pt) else missingArgs case _ => From 08442eb5756405ceeb535281708d026d6d1f2d52 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 7 Jun 2017 14:25:00 +0200 Subject: [PATCH 7/9] Better documentation of when to eta expand --- .../src/dotty/tools/dotc/typer/Typer.scala | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 29c02ee13e22..67397d850935 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2011,16 +2011,39 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit if (nparams > 0 || pt.eq(AnyFunctionProto)) nparams else -1 // no eta expansion in this case } + + /** A synthetic apply should be eta-expanded if it is the apply of an implicit function + * class, and the expected type is a function type. This rule is needed so we can pass + * an implicit function to a regular function type. So the following is OK + * + * val f: implicit A => B = ??? + * val g: A => B = f + * + * and the last line expands to + * + * val g: A => B = (x$0: A) => f.apply(x$0) + * + * One could be tempted not to eta expand the rhs, but that would violate the invariant + * that expressions of implicit function types are always implicit closures, which is + * exploited by ShortcutImplicits. + * + * On the other hand, the following would give an error if there is no implicit + * instance of A available. + * + * val x: AnyRef = f + * + * That's intentional, we want to fail here, otherwise some unsuccesful implicit searches + * would go undetected. + * + * Examples for these cases are found in run/implicitFuns.scala and neg/i2006.scala. + */ def isExpandableApply = defn.isImplicitFunctionClass(tree.symbol.maybeOwner) && defn.isFunctionType(ptNorm) // Reasons NOT to eta expand: // - we reference a constructor // - we are in a patterm - // - the current tree is a synthetic non-implicit apply (eta-expasion would simply undo that) - // - the current tree is a synthetic implicit apply and the expected - // type is a function type (this rule is needed so we can pass an implicit function - // to a regular function type) + // - the current tree is a synthetic apply which is not expandable (eta-expasion would simply undo that) if (arity >= 0 && !tree.symbol.isConstructor && !ctx.mode.is(Mode.Pattern) && From 81f36c9ffae404a0be1ca2cbc684b80acf0e025c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 14 Jun 2017 16:56:04 +0200 Subject: [PATCH 8/9] Drop unused method in TreeInfo I overlooked that we actually have a `closure` extractor, which would do the job of `isClosure`, if we ever needed it. --- compiler/src/dotty/tools/dotc/ast/TreeInfo.scala | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 0af2f9744b13..06eace243685 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -280,13 +280,6 @@ trait UntypedTreeInfo extends TreeInfo[Untyped] { self: Trees.Instance[Untyped] case _ => false } - /** Is tree a Function or Closure node, possibly preceded by definitions? (currently unused) */ - def isClosure(tree: Tree)(implicit ctx: Context): Boolean = unsplice(tree) match { - case _: Function | _: Closure => true - case Block(_, expr) => isClosure(expr) - case _ => false - } - /** Is `tree` an implicit function or closure, possibly nested in a block? */ def isImplicitClosure(tree: Tree)(implicit ctx: Context): Boolean = unsplice(tree) match { case Function((param: untpd.ValDef) :: _, _) => param.mods.is(Implicit) @@ -717,7 +710,7 @@ object TreeInfo { /* def isAbsTypeDef(tree: Tree) = tree match { case TypeDef(_, _, _, TypeBoundsTree(_, _)) => true - case TypeDef(_, _, _, rhs) => rhs.tpe.isInstanceOf[TypeBounds] + case TypeDef(_, _, _, rhs) => rhs.tpe. isInstanceOf[TypeBounds] case _ => false } From ff32e6eaf9c14db51435c1e7ed852514827d8a59 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 14 Jun 2017 16:59:35 +0200 Subject: [PATCH 9/9] Drop false comment --- compiler/src/dotty/tools/dotc/ast/TreeInfo.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 06eace243685..cee5ce6df905 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -471,7 +471,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => } /** Is tree a compiler-generated `.apply` node that refers to the - * apply of a function class? (implicit functions are excluded) + * apply of a function class? */ def isSyntheticApply(tree: Tree): Boolean = tree match { case Select(qual, nme.apply) => tree.pos.end == qual.pos.end @@ -710,7 +710,7 @@ object TreeInfo { /* def isAbsTypeDef(tree: Tree) = tree match { case TypeDef(_, _, _, TypeBoundsTree(_, _)) => true - case TypeDef(_, _, _, rhs) => rhs.tpe. isInstanceOf[TypeBounds] + case TypeDef(_, _, _, rhs) => rhs.tpe.isInstanceOf[TypeBounds] case _ => false }