From 0124dcac815bdbb382cd924505e5b0251b4e3015 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 15 Jan 2018 18:08:53 +0100 Subject: [PATCH 1/5] Fix #2808: Modify lifting of infix operations Lift the left hand side of a right-associative infix operation only if the operation takes a by-value argument. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 55 ++++++++++--------- .../src/dotty/tools/dotc/typer/Typer.scala | 25 +++++++++ tests/run/i2808.check | 1 + tests/run/i2808.scala | 14 +++++ 4 files changed, 68 insertions(+), 27 deletions(-) create mode 100644 tests/run/i2808.check create mode 100644 tests/run/i2808.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 88c024dae041..88803165f8d2 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -725,6 +725,31 @@ object desugar { tree } + /** Translate infix operation expression + * + * l op r ==> l.op(r) if op is left-associative + * ==> r.op(l) if op is right-associative + */ + def binop(left: Tree, op: Ident, right: Tree)(implicit ctx: Context): Apply = { + def assignToNamedArg(arg: Tree) = arg match { + case Assign(Ident(name), rhs) => cpy.NamedArg(arg)(name, rhs) + case _ => arg + } + def makeOp(fn: Tree, arg: Tree, selectPos: Position) = { + val args: List[Tree] = arg match { + case Parens(arg) => assignToNamedArg(arg) :: Nil + case Tuple(args) => args.mapConserve(assignToNamedArg) + case _ => arg :: Nil + } + Apply(Select(fn, op.name).withPos(selectPos), args) + } + + if (isLeftAssoc(op.name)) + makeOp(left, right, Position(left.pos.start, op.pos.end, op.pos.start)) + else + makeOp(right, left, Position(op.pos.start, right.pos.end)) + } + /** Make closure corresponding to function. * params => body * ==> @@ -832,30 +857,6 @@ object desugar { Block(ldef, call) } - /** Translate infix operation expression left op right - */ - def makeBinop(left: Tree, op: Ident, right: Tree): Tree = { - def assignToNamedArg(arg: Tree) = arg match { - case Assign(Ident(name), rhs) => cpy.NamedArg(arg)(name, rhs) - case _ => arg - } - if (isLeftAssoc(op.name)) { - val args: List[Tree] = right match { - case Parens(arg) => assignToNamedArg(arg) :: Nil - case Tuple(args) => args mapConserve assignToNamedArg - case _ => right :: Nil - } - val selectPos = Position(left.pos.start, op.pos.end, op.pos.start) - Apply(Select(left, op.name).withPos(selectPos), args) - } else { - val x = UniqueName.fresh() - val selectPos = Position(op.pos.start, right.pos.end, op.pos.start) - new InfixOpBlock( - ValDef(x, TypeTree(), left).withMods(synthetic), - Apply(Select(right, op.name).withPos(selectPos), Ident(x).withPos(left.pos))) - } - } - /** Create tree for for-comprehension `` or * `` where mapName and flatMapName are chosen * corresponding to whether this is a for-do or a for-yield. @@ -1066,10 +1067,10 @@ object desugar { if (!op.isBackquoted && op.name == tpnme.raw.AMP) AndTypeTree(l, r) // l & r else if (!op.isBackquoted && op.name == tpnme.raw.BAR) OrTypeTree(l, r) // l | r else AppliedTypeTree(op, l :: r :: Nil) // op[l, r] - else if (ctx.mode is Mode.Pattern) + else { + assert(ctx.mode is Mode.Pattern) // expressions are handled separately by `binop` Apply(op, l :: r :: Nil) // op(l, r) - else // l.op(r), or val x = r; l.op(x), plus handle named args specially - makeBinop(l, op, r) + } case PostfixOp(t, op) => if ((ctx.mode is Mode.Type) && !op.isBackquoted && op.name == tpnme.raw.STAR) { val seqType = if (ctx.compilationUnit.isJava) defn.ArrayType else defn.SeqType diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 8335c85ca55f..39b8b4e31b65 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1679,6 +1679,30 @@ class Typer extends Namer res } + /** Translate infix operation expression `l op r` to + * + * l.op(r) if `op` is left-associative + * { val x = l; r.op(l) } if `op` is right-associative call-by-value and `l` is impure + * r.op(l) if `op` is right-associative call-by-name or `l` is pure + */ + def typedInfixOp(tree: untpd.InfixOp, pt: Type)(implicit ctx: Context): Tree = { + val untpd.InfixOp(l, op, r) = tree + val app = typedApply(desugar.binop(l, op, r), pt) + if (untpd.isLeftAssoc(op.name)) app + else { + val defs = new mutable.ListBuffer[Tree] + def lift(app: Tree): Tree = (app: @unchecked) match { + case Apply(fn, args) => + tpd.cpy.Apply(app)(fn, LiftImpure.liftArgs(defs, fn.tpe, args)) + case Assign(lhs, rhs) => + tpd.cpy.Assign(app)(lhs, lift(rhs)) + case Block(stats, expr) => + tpd.cpy.Block(app)(stats, lift(expr)) + } + Applications.wrapDefs(defs, lift(app)) + } + } + /** Retrieve symbol attached to given tree */ protected def retrieveSym(tree: untpd.Tree)(implicit ctx: Context) = tree.removeAttachment(SymOfTree) match { case Some(sym) => @@ -1765,6 +1789,7 @@ class Typer extends Namer case tree: untpd.TypedSplice => typedTypedSplice(tree) case tree: untpd.UnApply => typedUnApply(tree, pt) case tree: untpd.DependentTypeTree => typed(untpd.TypeTree().withPos(tree.pos), pt) + case tree: untpd.InfixOp if ctx.mode.isExpr => typedInfixOp(tree, pt) case tree @ untpd.PostfixOp(qual, Ident(nme.WILDCARD)) => typedAsFunction(tree, pt) case untpd.EmptyTree => tpd.EmptyTree case _ => typedUnadapted(desugar(tree), pt) diff --git a/tests/run/i2808.check b/tests/run/i2808.check new file mode 100644 index 000000000000..257cc5642cb1 --- /dev/null +++ b/tests/run/i2808.check @@ -0,0 +1 @@ +foo diff --git a/tests/run/i2808.scala b/tests/run/i2808.scala new file mode 100644 index 000000000000..e2a13536f271 --- /dev/null +++ b/tests/run/i2808.scala @@ -0,0 +1,14 @@ +class C { + def m1_:(f: Int) = () + def m2_:(f: => Int) = () +} + +object Test { + def foo() = { println("foo") ; 5 } + + def main(args: Array[String]): Unit = { + val c = new C + foo() m1_: c + foo() m2_: c + } +} From 1a1e0aea7c5f435a57253b57712121d7d88db206 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 15 Jan 2018 20:47:01 +0100 Subject: [PATCH 2/5] Fix repl test --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 8 ++++---- compiler/test-resources/repl/errmsgs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 88803165f8d2..d88b0df2bede 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -731,10 +731,10 @@ object desugar { * ==> r.op(l) if op is right-associative */ def binop(left: Tree, op: Ident, right: Tree)(implicit ctx: Context): Apply = { - def assignToNamedArg(arg: Tree) = arg match { - case Assign(Ident(name), rhs) => cpy.NamedArg(arg)(name, rhs) - case _ => arg - } + def assignToNamedArg(arg: Tree) = arg match { + case Assign(Ident(name), rhs) => cpy.NamedArg(arg)(name, rhs) + case _ => arg + } def makeOp(fn: Tree, arg: Tree, selectPos: Position) = { val args: List[Tree] = arg match { case Parens(arg) => assignToNamedArg(arg) :: Nil diff --git a/compiler/test-resources/repl/errmsgs b/compiler/test-resources/repl/errmsgs index 711f5657babc..dbf6779d56b7 100644 --- a/compiler/test-resources/repl/errmsgs +++ b/compiler/test-resources/repl/errmsgs @@ -57,7 +57,7 @@ scala> class Foo() { def bar: Int = 1 }; val foo = new Foo(); foo.barr scala> val x: List[Int] = "foo" :: List(1) 1 | val x: List[Int] = "foo" :: List(1) | ^^^^^ - | found: String($1$) + | found: String("foo") | required: Int | scala> { def f: Int = g; val x: Int = 1; def g: Int = 5; } From a027c09aa38868ba4a86ab2598e72868d82aa943 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 15 Jan 2018 21:08:17 +0100 Subject: [PATCH 3/5] Survive erroneous infix operations Also, more tests --- .../src/dotty/tools/dotc/typer/Typer.scala | 3 ++- tests/neg/i2808.scala | 7 ++++++ tests/run/i2808.scala | 24 +++++++++++++++---- 3 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 tests/neg/i2808.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 39b8b4e31b65..9839e2420df2 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1693,7 +1693,8 @@ class Typer extends Namer val defs = new mutable.ListBuffer[Tree] def lift(app: Tree): Tree = (app: @unchecked) match { case Apply(fn, args) => - tpd.cpy.Apply(app)(fn, LiftImpure.liftArgs(defs, fn.tpe, args)) + if (app.tpe.isError) app + else tpd.cpy.Apply(app)(fn, LiftImpure.liftArgs(defs, fn.tpe, args)) case Assign(lhs, rhs) => tpd.cpy.Assign(app)(lhs, lift(rhs)) case Block(stats, expr) => diff --git a/tests/neg/i2808.scala b/tests/neg/i2808.scala new file mode 100644 index 000000000000..884e404b5efb --- /dev/null +++ b/tests/neg/i2808.scala @@ -0,0 +1,7 @@ +class C {} + +object Test { + def foo1() = { println("foo1") ; 5 } + val c = new C + foo1() m1_: c // error +} diff --git a/tests/run/i2808.scala b/tests/run/i2808.scala index e2a13536f271..00e7a0365894 100644 --- a/tests/run/i2808.scala +++ b/tests/run/i2808.scala @@ -1,14 +1,30 @@ class C { def m1_:(f: Int) = () def m2_:(f: => Int) = () + def m3_:(f: Int)(implicit c: C) = () + def m4_:(f: => Int)(implicit c: C) = () + } object Test { - def foo() = { println("foo") ; 5 } + def foo1() = { println("foo1") ; 5 } + def foo2() = { println("foo2") ; 5 } + def foo3() = { println("foo3") ; 5 } + def foo4() = { println("foo4") ; 5 } def main(args: Array[String]): Unit = { - val c = new C - foo() m1_: c - foo() m2_: c + implicit val c = new C + foo1() m1_: c // foo1 + foo2() m2_: c + foo3() m3_: c // foo3 + foo4() m4_: c + + def bar() = { println("bar"); c } + foo1() m1_: bar() // foo1 + // bar + foo2() m2_: bar() // bar + foo3() m3_: bar() // foo3 + // bar + foo4() m4_: bar() // bar } } From 2ce92c3f52527d1916e1f8baec119de03fc0d8fc Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 15 Jan 2018 21:58:17 +0100 Subject: [PATCH 4/5] Update check file --- tests/run/i2808.check | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/run/i2808.check b/tests/run/i2808.check index 257cc5642cb1..0d82bc33b098 100644 --- a/tests/run/i2808.check +++ b/tests/run/i2808.check @@ -1 +1,8 @@ -foo +foo1 +foo3 +foo1 +bar +bar +foo3 +bar +bar From 355a8c75b26ed00033f280999e8594acd452efa3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 3 Feb 2018 14:05:51 +0100 Subject: [PATCH 5/5] Remove InfixOpBlock And add a comment on possibly ode types returned from typedApply --- compiler/src/dotty/tools/dotc/ast/untpd.scala | 10 ---------- compiler/src/dotty/tools/dotc/typer/Applications.scala | 4 ++++ compiler/src/dotty/tools/dotc/typer/Typer.scala | 7 +------ 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala index 7452fa9179e7..22fd83688eaf 100644 --- a/compiler/src/dotty/tools/dotc/ast/untpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/untpd.scala @@ -96,16 +96,6 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo { override def isEmpty = true } - /** A block arising from a right-associative infix operation, where, e.g. - * - * a +: b - * - * is expanded to - * - * { val x = a; b.+:(x) } - */ - class InfixOpBlock(leftOperand: Tree, rightOp: Tree) extends Block(leftOperand :: Nil, rightOp) - /** A block generated by the XML parser, only treated specially by * `Positioned#checkPos` */ class XMLBlock(stats: List[Tree], expr: Tree) extends Block(stats, expr) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 0b5c3f5cb3c0..1cd77d4d8517 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -665,6 +665,10 @@ trait Applications extends Compatibility { self: Typer with Dynamic => def argCtx(app: untpd.Tree)(implicit ctx: Context): Context = if (untpd.isSelfConstrCall(app)) ctx.thisCallArgContext else ctx + /** Typecheck application. Result could be an `Apply` node, + * or, if application is an operator assignment, also an `Assign` or + * Block node. + */ def typedApply(tree: untpd.Apply, pt: Type)(implicit ctx: Context): Tree = { def realApply(implicit ctx: Context): Tree = track("realApply") { diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9839e2420df2..abd10c299991 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -664,12 +664,7 @@ class Typer extends Namer def typedBlock(tree: untpd.Block, pt: Type)(implicit ctx: Context) = track("typedBlock") { val (exprCtx, stats1) = typedBlockStats(tree.stats) - val ept = - if (tree.isInstanceOf[untpd.InfixOpBlock]) - // Right-binding infix operations are expanded to InfixBlocks, which may be followed by arguments. - // Example: `(a /: bs)(op)` expands to `{ val x = a; bs./:(x) } (op)` where `{...}` is an InfixBlock. - pt - else pt.notApplied + val ept = pt.notApplied val expr1 = typedExpr(tree.expr, ept)(exprCtx) ensureNoLocalRefs( cpy.Block(tree)(stats1, expr1).withType(expr1.tpe), pt, localSyms(stats1))