From 89e84d231ec1f75ed3dd402f8361d0c20c9eb118 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 25 Apr 2019 21:36:02 +0200 Subject: [PATCH 1/5] Fix #6341: Revise purity checking There is some confusion on terms that needs to be fixed. Step 1: Make `SimplyPure apply to paths only. The only point where it is used is in eta expansion to answer the question whether in a partial application `f(e, _)` the expression `e` should be lifted out, giving `{ val x = e; y => f(x, y) }` instead of `y => f(e, y)`. With the change, closures and new expressions are never simply pure, so are always lifted out. This can reduce the number of allocations, if the lambda is applied several times. --- compiler/src/dotty/tools/dotc/ast/TreeInfo.scala | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 796c9ad91bd2..fef2f377646a 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -367,7 +367,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => } /** The purity level of this expression. - * @return SimplyPure if expression has no side effects and cannot contain local definitions + * @return SimplyPure if expression has no side effects is a path * Pure if expression has no side effects * Idempotent if running the expression a second time has no side effects * Impure otherwise @@ -381,16 +381,15 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => case EmptyTree | This(_) | Super(_, _) - | Literal(_) - | Closure(_, _, _) => + | Literal(_) => SimplyPure case Ident(_) => refPurity(tree) case Select(qual, _) => if (tree.symbol.is(Erased)) Pure else refPurity(tree).min(exprPurity(qual)) - case New(_) => - SimplyPure + case New(_) | Closure(_, _, _) => + Pure case TypeApply(fn, _) => if (fn.symbol.is(Erased)) Pure else exprPurity(fn) case Apply(fn, args) => From ae72183e8ac88f285f26225e214068e599be35a6 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 25 Apr 2019 22:26:45 +0200 Subject: [PATCH 2/5] Sharpen condition in Inliner --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 52 +++++++++++-------- .../dotty/tools/dotc/typer/EtaExpansion.scala | 10 ++-- .../src/dotty/tools/dotc/typer/Inliner.scala | 2 +- tests/run/i6341.scala | 7 +++ 4 files changed, 43 insertions(+), 28 deletions(-) create mode 100644 tests/run/i6341.scala diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index fef2f377646a..1cbb87b53ceb 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -367,13 +367,19 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => } /** The purity level of this expression. - * @return SimplyPure if expression has no side effects is a path + * @return A possibly combination of + * + * Path if expression is at least idempotent and is a path + * * Pure if expression has no side effects * Idempotent if running the expression a second time has no side effects - * Impure otherwise * - * Note that purity and idempotency are different. References to modules and lazy - * vals are impure (side-effecting) both because side-effecting code may be executed and because the first reference + * Pure implies Idempotent. + * Impure designates the empty combination. + * + * Note that purity and idempotency are treated differently. + * References to modules and lazy vals are impure (side-effecting) both because + * side-effecting code may be executed and because the first reference * takes a different code path than all to follow; but they are idempotent * because running the expression a second time gives the cached result. */ @@ -382,12 +388,12 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => | This(_) | Super(_, _) | Literal(_) => - SimplyPure + PurePath case Ident(_) => refPurity(tree) case Select(qual, _) => if (tree.symbol.is(Erased)) Pure - else refPurity(tree).min(exprPurity(qual)) + else refPurity(tree) `min` exprPurity(qual) case New(_) | Closure(_, _, _) => Pure case TypeApply(fn, _) => @@ -415,37 +421,38 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => Impure } - private def minOf(l0: PurityLevel, ls: List[PurityLevel]) = (l0 /: ls)(_ min _) + private def minOf(l0: PurityLevel, ls: List[PurityLevel]) = (l0 /: ls)(_ `min` _) - def isSimplyPure(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) == SimplyPure + def isPurePath(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) == PurePath def isPureExpr(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) >= Pure def isIdempotentExpr(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) >= Idempotent + def isIdempotentPath(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) >= IdempotentPath def isPureBinding(tree: Tree)(implicit ctx: Context): Boolean = statPurity(tree) >= Pure /** The purity level of this reference. * @return - * SimplyPure if reference is (nonlazy and stable) or to a parameterized function - * Idempotent if reference is lazy and stable - * Impure otherwise + * PurePath if reference is (nonlazy and stable) or to a parameterized function + * IdempotentPath if reference is lazy and stable + * Impure otherwise * @DarkDimius: need to make sure that lazy accessor methods have Lazy and Stable * flags set. */ def refPurity(tree: Tree)(implicit ctx: Context): PurityLevel = { val sym = tree.symbol if (!tree.hasType) Impure - else if (!tree.tpe.widen.isParameterless || sym.isEffectivelyErased) SimplyPure + else if (!tree.tpe.widen.isParameterless || sym.isEffectivelyErased) PurePath else if (!sym.isStableMember) Impure else if (sym.is(Module)) - if (sym.moduleClass.isNoInitsClass) Pure else Idempotent - else if (sym.is(Lazy)) Idempotent - else SimplyPure + if (sym.moduleClass.isNoInitsClass) PurePath else IdempotentPath + else if (sym.is(Lazy)) IdempotentPath + else PurePath } def isPureRef(tree: Tree)(implicit ctx: Context): Boolean = - refPurity(tree) == SimplyPure + refPurity(tree) == PurePath def isIdempotentRef(tree: Tree)(implicit ctx: Context): Boolean = - refPurity(tree) >= Idempotent + refPurity(tree) >= IdempotentPath /** (1) If `tree` is a constant expression, its value as a Literal, * or `tree` itself otherwise. @@ -840,12 +847,15 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => object TreeInfo { class PurityLevel(val x: Int) extends AnyVal { - def >= (that: PurityLevel): Boolean = x >= that.x - def min(that: PurityLevel): PurityLevel = new PurityLevel(x min that.x) + def >= (that: PurityLevel): Boolean = (x & that.x) == that.x + def min(that: PurityLevel): PurityLevel = new PurityLevel(x & that.x) } - val SimplyPure: PurityLevel = new PurityLevel(3) - val Pure: PurityLevel = new PurityLevel(2) + val Path: PurityLevel = new PurityLevel(4) + val Pure: PurityLevel = new PurityLevel(3) val Idempotent: PurityLevel = new PurityLevel(1) val Impure: PurityLevel = new PurityLevel(0) + + val PurePath: PurityLevel = new PurityLevel(7) + val IdempotentPath: PurityLevel = new PurityLevel(5) } diff --git a/compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala b/compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala index 0156e0c6f340..2322aa026346 100644 --- a/compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala +++ b/compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala @@ -121,12 +121,10 @@ abstract class Lifter { * val x0 = pre * x0.f(...) * - * unless `pre` is a `New` or `pre` is idempotent. + * unless `pre` is idempotent. */ - def liftPrefix(defs: mutable.ListBuffer[Tree], tree: Tree)(implicit ctx: Context): Tree = tree match { - case New(_) => tree - case _ => if (isIdempotentExpr(tree)) tree else lift(defs, tree) - } + def liftPrefix(defs: mutable.ListBuffer[Tree], tree: Tree)(implicit ctx: Context): Tree = + if (isIdempotentExpr(tree)) tree else lift(defs, tree) } /** No lifting at all */ @@ -142,7 +140,7 @@ object LiftImpure extends LiftImpure /** Lift all impure or complex arguments */ class LiftComplex extends Lifter { - def noLift(expr: tpd.Tree)(implicit ctx: Context): Boolean = tpd.isSimplyPure(expr) + def noLift(expr: tpd.Tree)(implicit ctx: Context): Boolean = tpd.isPurePath(expr) override def exprLifter: Lifter = LiftToDefs } object LiftComplex extends LiftComplex diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 6abc4d3b9449..c0a685d191d4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -285,7 +285,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { (tp.paramNames, tp.paramInfos, argss.head).zipped.foreach { (name, paramtp, arg) => paramSpan(name) = arg.span paramBinding(name) = arg.tpe.dealias match { - case _: SingletonType if isIdempotentExpr(arg) => arg.tpe + case _: SingletonType if isIdempotentPath(arg) => arg.tpe case _ => paramBindingDef(name, paramtp, arg, bindingsBuf).symbol.termRef } } diff --git a/tests/run/i6341.scala b/tests/run/i6341.scala new file mode 100644 index 000000000000..1389e44cdea8 --- /dev/null +++ b/tests/run/i6341.scala @@ -0,0 +1,7 @@ +object Test extends App { + class Config(val t1: Int) + + inline def m(t2:Int) = t2 + + m(new Config(3).t1) +} \ No newline at end of file From e6b2db5f9b3e711b01d347cac221ed0476fa4de2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Apr 2019 14:00:42 +0200 Subject: [PATCH 3/5] Refine purity checking for constants A pure expression such as `(a + b).c` of constant type is now characterized again as a pure path (since it will be eventually replaced by that constant, which is a path). This fixes instability problems in pickling and improves inlining. For instance `assert(0 == 0)` is reduced to `()` again with this change. --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 1cbb87b53ceb..edfa712c27ae 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -423,10 +423,21 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => private def minOf(l0: PurityLevel, ls: List[PurityLevel]) = (l0 /: ls)(_ `min` _) - def isPurePath(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) == PurePath - def isPureExpr(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) >= Pure - def isIdempotentExpr(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) >= Idempotent - def isIdempotentPath(tree: Tree)(implicit ctx: Context): Boolean = exprPurity(tree) >= IdempotentPath + def isPurePath(tree: Tree)(implicit ctx: Context): Boolean = tree.tpe match { + case tpe: ConstantType => exprPurity(tree) >= Pure + case _ => exprPurity(tree) == PurePath + } + + def isPureExpr(tree: Tree)(implicit ctx: Context): Boolean = + exprPurity(tree) >= Pure + + def isIdempotentPath(tree: Tree)(implicit ctx: Context): Boolean = tree.tpe match { + case tpe: ConstantType => exprPurity(tree) >= Idempotent + case _ => exprPurity(tree) >= IdempotentPath + } + + def isIdempotentExpr(tree: Tree)(implicit ctx: Context): Boolean = + exprPurity(tree) >= Idempotent def isPureBinding(tree: Tree)(implicit ctx: Context): Boolean = statPurity(tree) >= Pure From 5a2e79fa02debbfd77a45df3d59894ba23511123 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 27 Apr 2019 14:36:52 +0200 Subject: [PATCH 4/5] Address review comments --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index edfa712c27ae..1f62b255b8ed 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -366,7 +366,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => // But if we do that the repl/vars test break. Need to figure out why that's the case. } - /** The purity level of this expression. + /** The purity level of this expression. See docs for PurityLevel for what that means * @return A possibly combination of * * Path if expression is at least idempotent and is a path @@ -857,16 +857,29 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => } object TreeInfo { + /** A purity level is represented as a bitset (expressed as an Int) */ class PurityLevel(val x: Int) extends AnyVal { + /** `this` contains the bits of `that` */ def >= (that: PurityLevel): Boolean = (x & that.x) == that.x + + /** The intersection of the bits of `this` and `that` */ def min(that: PurityLevel): PurityLevel = new PurityLevel(x & that.x) } + /** An expression is a stable path. Requires that expression is at least idempotent */ val Path: PurityLevel = new PurityLevel(4) + + /** The expression has no side effects */ val Pure: PurityLevel = new PurityLevel(3) + + /** Running the expression a second time has no side effects. Implied by `Pure`. */ val Idempotent: PurityLevel = new PurityLevel(1) + val Impure: PurityLevel = new PurityLevel(0) - val PurePath: PurityLevel = new PurityLevel(7) - val IdempotentPath: PurityLevel = new PurityLevel(5) + /** A stable path that is evaluated without side effects */ + val PurePath: PurityLevel = new PurityLevel(Pure.x | Path.x) + + /** A stable path that is also idempotent */ + val IdempotentPath: PurityLevel = new PurityLevel(Idempotent.x | Path.x) } From b9d8a2ac1f2ab5fbcaa84405376b7039490ffe19 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 27 Apr 2019 15:29:05 +0200 Subject: [PATCH 5/5] Drop redundant comment --- compiler/src/dotty/tools/dotc/ast/TreeInfo.scala | 9 --------- 1 file changed, 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index 1f62b255b8ed..f6622eedd04b 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -367,15 +367,6 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => } /** The purity level of this expression. See docs for PurityLevel for what that means - * @return A possibly combination of - * - * Path if expression is at least idempotent and is a path - * - * Pure if expression has no side effects - * Idempotent if running the expression a second time has no side effects - * - * Pure implies Idempotent. - * Impure designates the empty combination. * * Note that purity and idempotency are treated differently. * References to modules and lazy vals are impure (side-effecting) both because