From b89d90026c03aef0d7029ac9adbdb0f8f84fac46 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 20 Nov 2018 15:42:07 +0100 Subject: [PATCH 1/4] Fix #5386: Normalize unary operator expressions A unary operator expression `pre.op` where `op` is one of `+`, `-`, `~`, `!` that has a constant type `ConstantType(v)` but that is not a constant expression (i.e. `pre` has side-effects) is translated to { pre; v } This avoids the situation where we have a Select node which does not have a symbol. --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 20 +++++++++++++-- tests/pos/i5386.scala | 25 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 tests/pos/i5386.scala diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 30eb2b16ba5e..5f57c05d86b5 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -449,7 +449,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => def isIdempotentRef(tree: Tree)(implicit ctx: Context): Boolean = refPurity(tree) >= Idempotent - /** If `tree` is a constant expression, its value as a Literal, + /** (1) If `tree` is a constant expression, its value as a Literal, * or `tree` itself otherwise. * * Note: Demanding idempotency instead of purity in literalize is strictly speaking too loose. @@ -485,11 +485,27 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => * Ident * Select * TypeApply + * + * (2) A unary operator expression `pre.op` where `op` is one of `+`, `-`, `~`, `!` + * that has a constant type `ConstantType(v)` but that is not a constant expression + * (i.e. `pre` has side-effects) is translated to + * + * { pre; v } + * + * This avoids the situation where we have a Select node that does not have a symbol. */ def constToLiteral(tree: Tree)(implicit ctx: Context): Tree = { val tree1 = ConstFold(tree) tree1.tpe.widenTermRefExpr match { - case ConstantType(value) if isIdempotentExpr(tree1) => Literal(value) + case ConstantType(value) => + if (isIdempotentExpr(tree1)) Literal(value) + else tree1 match { + case Select(qual, _) if tree1.tpe.isInstanceOf[ConstantType] => + // it's a unary operator; Simplify `pre.op` to `{ pre; v }` where `v` is the value of `pre.op` + Block(qual :: Nil, Literal(value)) + case _ => + tree1 + } case _ => tree1 } } diff --git a/tests/pos/i5386.scala b/tests/pos/i5386.scala new file mode 100644 index 000000000000..5a744d584a37 --- /dev/null +++ b/tests/pos/i5386.scala @@ -0,0 +1,25 @@ +object Test { + ~{ + println("!") + 1 + } + + +{ + println("!") + 1 + } + + -{ + println("!") + 1 + } + + !{ + println("!") + true + } + + { println("1"); 1 } + { println("2"); 2} + + !(try true finally{()}) +} \ No newline at end of file From b6c2e30cb45a35ed18e45ea0d310f365a389e2c2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 20 Nov 2018 16:15:23 +0100 Subject: [PATCH 2/4] Clarification --- 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 5f57c05d86b5..2c0be4f27f2b 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -486,7 +486,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => * Select * TypeApply * - * (2) A unary operator expression `pre.op` where `op` is one of `+`, `-`, `~`, `!` + * (2) A primitive unary operator expression `pre.op` where `op` is one of `+`, `-`, `~`, `!` * that has a constant type `ConstantType(v)` but that is not a constant expression * (i.e. `pre` has side-effects) is translated to * @@ -501,7 +501,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => if (isIdempotentExpr(tree1)) Literal(value) else tree1 match { case Select(qual, _) if tree1.tpe.isInstanceOf[ConstantType] => - // it's a unary operator; Simplify `pre.op` to `{ pre; v }` where `v` is the value of `pre.op` + // it's a primitive unary operator; Simplify `pre.op` to `{ pre; v }` where `v` is the value of `pre.op` Block(qual :: Nil, Literal(value)) case _ => tree1 From ebb228fd9c59d994b951f61f84737fe030dc823f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 20 Nov 2018 16:52:53 +0100 Subject: [PATCH 3/4] Add test with user-defined constant unary operators Add test showing that user-defined unary operators do not cause a problem, even if they have constant types. --- tests/pos/i5386.scala | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/pos/i5386.scala b/tests/pos/i5386.scala index 5a744d584a37..1805c8470224 100644 --- a/tests/pos/i5386.scala +++ b/tests/pos/i5386.scala @@ -1,4 +1,5 @@ object Test { + ~{ println("!") 1 @@ -22,4 +23,12 @@ object Test { { println("1"); 1 } + { println("2"); 2} !(try true finally{()}) + + + class C { + def foo: 1 = 1 + } + + { println("!"); new C }.foo + } \ No newline at end of file From 67012361b9bb99d21d95f8aa7aa11d7267056147 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 21 Nov 2018 21:20:54 +0100 Subject: [PATCH 4/4] Check test output --- tests/run/i5386.check | 8 ++++++++ tests/{pos => run}/i5386.scala | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 tests/run/i5386.check rename tests/{pos => run}/i5386.scala (80%) diff --git a/tests/run/i5386.check b/tests/run/i5386.check new file mode 100644 index 000000000000..343e36be2164 --- /dev/null +++ b/tests/run/i5386.check @@ -0,0 +1,8 @@ +! +! +! +! +1 +2 +! +1 diff --git a/tests/pos/i5386.scala b/tests/run/i5386.scala similarity index 80% rename from tests/pos/i5386.scala rename to tests/run/i5386.scala index 1805c8470224..eff28a2601f9 100644 --- a/tests/pos/i5386.scala +++ b/tests/run/i5386.scala @@ -1,4 +1,4 @@ -object Test { +object Test extends App { ~{ println("!") @@ -26,7 +26,7 @@ object Test { class C { - def foo: 1 = 1 + def foo: 1 = { println("1"); 1 } } { println("!"); new C }.foo