From e2d0659569ce537bc849a4607db3b687bfc334f8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 8 May 2020 18:14:07 +0200 Subject: [PATCH 1/5] Fix #8881: Lift lets over applications This is needed to correctly deal with bunched arguments arisign from IFTs. It's necessary in particular for let-bindings introduced by the compiler, e.g. when passing default araguments. User-defined blocks are not the problem since they get already eta expanded in typer. --- compiler/src/dotty/tools/dotc/Compiler.scala | 1 + .../tools/dotc/transform/LetOverApply.scala | 37 +++++++++++++++++++ tests/pos/i8881.scala | 5 +++ 3 files changed, 43 insertions(+) create mode 100644 compiler/src/dotty/tools/dotc/transform/LetOverApply.scala create mode 100644 tests/pos/i8881.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 49c28f1ea030..a8bb0b72c10d 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -93,6 +93,7 @@ class Compiler { new FunctionXXLForwarders, // Add forwarders for FunctionXXL apply method new ParamForwarding, // Add forwarders for aliases of superclass parameters new TupleOptimizations, // Optimize generic operations on tuples + new LetOverApply, // Lift blocks from receivers of applications new ArrayConstructors) :: // Intercept creation of (non-generic) arrays and intrinsify. List(new Erasure) :: // Rewrite types to JVM model, erasing all type parameters, abstract types and refinements. List(new ElimErasedValueType, // Expand erased value types to their underlying implmementation types diff --git a/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala b/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala new file mode 100644 index 000000000000..94c47411f103 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala @@ -0,0 +1,37 @@ +package dotty.tools +package dotc +package transform + +import core._ +import MegaPhase._ +import Contexts.Context +import Symbols._, Decorators._, Types._ +import ast.Trees._ +import dotty.tools.dotc.ast.tpd + + +import scala.collection.immutable.:: + + +/** Rewrite `{ stats; expr}.f(args) }` to `{ stats; expr.f(args) }` before + * proceeding, but leave closures alone. This is necessary to be able to + * collapse applies of IFTs. + */ +class LetOverApply extends MiniPhase: + import ast.tpd._ + + override def phaseName: String = "letOverApply" + + override def transformApply(tree: tpd.Apply)(using Context): tpd.Tree = + tree.fun match + case Select(blk @ Block(stats, expr), name) if !expr.isInstanceOf[Closure] => + cpy.Block(blk)(stats, + cpy.Apply(tree)( + cpy.Select(tree.fun)(expr, name), tree.args)) + case Block(stats, expr) => + cpy.Block(tree.fun)(stats, + cpy.Apply(tree)(expr, tree.args)) + case _ => + tree + +end LetOverApply diff --git a/tests/pos/i8881.scala b/tests/pos/i8881.scala new file mode 100644 index 000000000000..ce3820aa5d6d --- /dev/null +++ b/tests/pos/i8881.scala @@ -0,0 +1,5 @@ +object Crash { + def f(a: String, b: String, c: Int = 0): Int ?=> String = "" + given Int = ??? + f(b = "b", a = "a") +} \ No newline at end of file From 7c8370127e4cbafd3b9de8491dc3785c81b86f08 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 8 May 2020 18:21:45 +0200 Subject: [PATCH 2/5] Drop redundant code --- .../src/dotty/tools/dotc/transform/LetOverApply.scala | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala b/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala index 94c47411f103..305622e3e289 100644 --- a/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala +++ b/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala @@ -3,15 +3,9 @@ package dotc package transform import core._ +import Contexts.Context, Symbols._, Decorators._ import MegaPhase._ -import Contexts.Context -import Symbols._, Decorators._, Types._ import ast.Trees._ -import dotty.tools.dotc.ast.tpd - - -import scala.collection.immutable.:: - /** Rewrite `{ stats; expr}.f(args) }` to `{ stats; expr.f(args) }` before * proceeding, but leave closures alone. This is necessary to be able to @@ -22,7 +16,7 @@ class LetOverApply extends MiniPhase: override def phaseName: String = "letOverApply" - override def transformApply(tree: tpd.Apply)(using Context): tpd.Tree = + override def transformApply(tree: Apply)(using Context): Tree = tree.fun match case Select(blk @ Block(stats, expr), name) if !expr.isInstanceOf[Closure] => cpy.Block(blk)(stats, From 3e1a0615fc0b93f09b7b862a8eb5a1ec0ce84621 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 8 May 2020 18:26:32 +0200 Subject: [PATCH 3/5] Drop redundant code in erasure --- compiler/src/dotty/tools/dotc/transform/Erasure.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Erasure.scala b/compiler/src/dotty/tools/dotc/transform/Erasure.scala index 24ab78f4df7a..a127551dc45f 100644 --- a/compiler/src/dotty/tools/dotc/transform/Erasure.scala +++ b/compiler/src/dotty/tools/dotc/transform/Erasure.scala @@ -749,14 +749,13 @@ object Erasure { val origFunType = origFun.tpe.widen(using preErasureCtx) val ownArgs = if origFunType.isErasedMethod then Nil else args val fun1 = typedExpr(fun, AnyFunctionProto) - val fun1core = stripBlock(fun1) fun1.tpe.widen match case mt: MethodType => val (xmt, // A method type like `mt` but with bunched arguments expanded to individual ones bunchArgs, // whether arguments are bunched outers) = // the outer reference parameter(s) - if fun1core.isInstanceOf[Apply] then - (mt, fun1core.removeAttachment(BunchedArgs).isDefined, Nil) + if fun1.isInstanceOf[Apply] then + (mt, fun1.removeAttachment(BunchedArgs).isDefined, Nil) else val xmt = expandedMethodType(mt, origFun) (xmt, xmt ne mt, outer.args(origFun)) From 7ce3d5b3c850045aa7f12f77e3f11f8827381f7f Mon Sep 17 00:00:00 2001 From: odersky Date: Fri, 8 May 2020 21:55:14 +0200 Subject: [PATCH 4/5] Update compiler/src/dotty/tools/dotc/transform/LetOverApply.scala Co-authored-by: Guillaume Martres --- compiler/src/dotty/tools/dotc/transform/LetOverApply.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala b/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala index 305622e3e289..d0765ff01cb7 100644 --- a/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala +++ b/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala @@ -9,7 +9,7 @@ import ast.Trees._ /** Rewrite `{ stats; expr}.f(args) }` to `{ stats; expr.f(args) }` before * proceeding, but leave closures alone. This is necessary to be able to - * collapse applies of IFTs. + * collapse applies of IFTs (this is done in Erasure). */ class LetOverApply extends MiniPhase: import ast.tpd._ From 40a294692f063c4811a8b3e5491687111b92677d Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 11 May 2020 15:30:17 +0200 Subject: [PATCH 5/5] Update compiler/src/dotty/tools/dotc/transform/LetOverApply.scala Co-authored-by: Guillaume Martres --- compiler/src/dotty/tools/dotc/transform/LetOverApply.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala b/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala index d0765ff01cb7..2279302ef16e 100644 --- a/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala +++ b/compiler/src/dotty/tools/dotc/transform/LetOverApply.scala @@ -7,8 +7,9 @@ import Contexts.Context, Symbols._, Decorators._ import MegaPhase._ import ast.Trees._ -/** Rewrite `{ stats; expr}.f(args) }` to `{ stats; expr.f(args) }` before - * proceeding, but leave closures alone. This is necessary to be able to +/** Rewrite `{ stats; expr}.f(args)` to `{ stats; expr.f(args) }` and + * `{ stats; expr }(args)` to `{ stats; expr(args) }` before proceeding, + * but leave closures alone. This is necessary to be able to * collapse applies of IFTs (this is done in Erasure). */ class LetOverApply extends MiniPhase: