From c166db97515387ed1c516874a861e25133ea0ea0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 18 Jul 2019 18:20:36 +0200 Subject: [PATCH 01/18] Implement memoization Implement `memo(...)` function which caches its argument on first evaluation and re-uses the cached value afterwards. The cache is placed next to the method enclosing the memo(...) call. `memo` is a member of package `compiletime`. --- .../dotty/tools/dotc/core/Definitions.scala | 1 + .../src/dotty/tools/dotc/core/NameKinds.scala | 4 ++ .../src/dotty/tools/dotc/typer/Inliner.scala | 47 ++++++++++++++++++- .../src/dotty/tools/dotc/typer/Typer.scala | 3 ++ .../tools/dotc/util/FreshNameCreator.scala | 11 ++++- library/src/scala/compiletime/package.scala | 2 + tests/run/memoTest.scala | 5 +- 7 files changed, 69 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 60df6c4e5fe1..5afa0c8b9f22 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -237,6 +237,7 @@ class Definitions { @threadUnsafe lazy val Compiletime_constValue : SymbolPerRun = perRunSym(CompiletimePackageObject.requiredMethodRef("constValue")) @threadUnsafe lazy val Compiletime_constValueOpt: SymbolPerRun = perRunSym(CompiletimePackageObject.requiredMethodRef("constValueOpt")) @threadUnsafe lazy val Compiletime_code : SymbolPerRun = perRunSym(CompiletimePackageObject.requiredMethodRef("code")) + @threadUnsafe lazy val Compiletime_memo : SymbolPerRun = perRunSym(CompiletimePackageObject.requiredMethodRef("memo")) /** The `scalaShadowing` package is used to safely modify classes and * objects in scala so that they can be used from dotty. They will diff --git a/compiler/src/dotty/tools/dotc/core/NameKinds.scala b/compiler/src/dotty/tools/dotc/core/NameKinds.scala index ec0a6be801b3..5ae5aee962c2 100644 --- a/compiler/src/dotty/tools/dotc/core/NameKinds.scala +++ b/compiler/src/dotty/tools/dotc/core/NameKinds.scala @@ -213,6 +213,9 @@ object NameKinds { safePrefix + info.num } + def currentCount(prefix: TermName = EmptyTermName) given (ctx: Context): Int = + ctx.freshNames.currentCount(prefix, this) + /** Generate fresh unique term name of this kind with given prefix name */ def fresh(prefix: TermName = EmptyTermName)(implicit ctx: Context): TermName = ctx.freshNames.newName(prefix, this) @@ -296,6 +299,7 @@ object NameKinds { val UniqueInlineName: UniqueNameKind = new UniqueNameKind("$i") val InlineScrutineeName: UniqueNameKind = new UniqueNameKind("$scrutinee") val InlineBinderName: UniqueNameKind = new UniqueNameKind("$elem") + val MemoCacheName: UniqueNameKind = new UniqueNameKind("memo$") /** A kind of unique extension methods; Unlike other unique names, these can be * unmangled. diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index bf282476389a..d386258fe003 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -15,7 +15,7 @@ import StdNames._ import transform.SymUtils._ import Contexts.Context import Names.{Name, TermName} -import NameKinds.{InlineAccessorName, InlineBinderName, InlineScrutineeName} +import NameKinds.{InlineAccessorName, InlineBinderName, InlineScrutineeName, MemoCacheName} import ProtoTypes.selectionProto import SymDenotations.SymDenotation import Inferencing.fullyDefinedType @@ -188,6 +188,19 @@ object Inliner { if (callSym.is(Macro)) ref(callSym.topLevelClass.owner).select(callSym.topLevelClass.name).withSpan(pos.span) else Ident(callSym.topLevelClass.typeRef).withSpan(pos.span) } + + /** For every occurrence of a memo cache symbol `memo$N` of type `T_N` in `tree`, + * an assignment `val memo$N: T_N = null` + */ + def memoCacheDefs(tree: Tree) given Context: Set[ValDef] = { + val memoCacheSyms = tree.deepFold[Set[TermSymbol]](Set.empty) { + (syms, t) => t match { + case Assign(lhs, _) if lhs.symbol.name.is(MemoCacheName) => syms + lhs.symbol.asTerm + case _ => syms + } + } + memoCacheSyms.map(ValDef(_, Literal(Constant(null)))) + } } /** Produces an inlined version of `call` via its `inlined` method. @@ -392,6 +405,36 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { case _ => EmptyTree } + /** The expansion of `memo(op)` where `op: T` is: + * + * { if (memo$N == null) memo$N = op; $memo.asInstanceOf[T] } + * + * This creates as a side effect a memo cache symbol $memo$N` of type `T | Null`. + * TODO: Restrict this to non-null types, once nullability checking is in. + */ + def memoized: Tree = { + val currentOwner = ctx.owner.skipWeakOwner + if (currentOwner.isRealMethod) { + val cacheOwner = ctx.owner.effectiveOwner + val argType = callTypeArgs.head.tpe + val memoVar = ctx.newSymbol( + owner = cacheOwner, + name = MemoCacheName.fresh(), + flags = + if (cacheOwner.isTerm) Synthetic | Mutable + else Synthetic | Mutable | Private | Local, + info = OrType(argType, defn.NullType), + coord = call.span) + val cond = If( + ref(memoVar).select(defn.Any_==).appliedTo(Literal(Constant(null))), + ref(memoVar).becomes(callValueArgss.head.head), + Literal(Constant(()))) + val expr = ref(memoVar).cast(argType) + Block(cond :: Nil, expr) + } + else errorTree(call, em"""memo(...) outside method""") + } + /** The Inlined node representing the inlined call */ def inlined(sourcePos: SourcePosition): Tree = { @@ -408,6 +451,8 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { else New(defn.SomeClass.typeRef.appliedTo(constVal.tpe), constVal :: Nil) ) } + else if (inlinedMethod == defn.Compiletime_memo) + return memoized // Compute bindings for all parameters, appending them to bindingsBuf computeParamBindings(inlinedMethod.info, callTypeArgs, callValueArgss) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 00289794ee73..6f33c8e5e369 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2122,6 +2122,7 @@ class Typer extends Namer case Some(xtree) => traverse(xtree :: rest) case none => + val memoCacheCount = MemoCacheName.currentCount() typed(mdef) match { case mdef1: DefDef if Inliner.hasBodyToInline(mdef1.symbol) => buf += inlineExpansion(mdef1) @@ -2132,6 +2133,8 @@ class Typer extends Namer mdef match { case mdef: untpd.TypeDef if mdef.mods.isEnumClass => enumContexts(mdef1.symbol) = ctx + case _: untpd.DefDef if MemoCacheName.currentCount() != memoCacheCount => + buf ++= Inliner.memoCacheDefs(mdef1) case _ => } if (!mdef1.isEmpty) // clashing synthetic case methods are converted to empty trees diff --git a/compiler/src/dotty/tools/dotc/util/FreshNameCreator.scala b/compiler/src/dotty/tools/dotc/util/FreshNameCreator.scala index f3375028c95f..0c79f926fcc6 100644 --- a/compiler/src/dotty/tools/dotc/util/FreshNameCreator.scala +++ b/compiler/src/dotty/tools/dotc/util/FreshNameCreator.scala @@ -9,20 +9,27 @@ import core.StdNames.str abstract class FreshNameCreator { def newName(prefix: TermName, unique: UniqueNameKind): TermName + def currentCount(prefix: TermName, unique: UniqueNameKind): Int } object FreshNameCreator { class Default extends FreshNameCreator { - protected var counter: Int = 0 protected val counters: mutable.Map[String, Int] = mutable.AnyRefMap() withDefaultValue 0 + private def keyFor(prefix: TermName, unique: UniqueNameKind) = + str.sanitize(prefix.toString) + unique.separator + + /** The current counter for the given combination of `prefix` and `unique` */ + def currentCount(prefix: TermName, unique: UniqueNameKind): Int = + counters(keyFor(prefix, unique)) + /** * Create a fresh name with the given prefix. It is guaranteed * that the returned name has never been returned by a previous * call to this function (provided the prefix does not end in a digit). */ def newName(prefix: TermName, unique: UniqueNameKind): TermName = { - val key = str.sanitize(prefix.toString) + unique.separator + val key = keyFor(prefix, unique) counters(key) += 1 prefix.derived(unique.NumberedInfo(counters(key))) } diff --git a/library/src/scala/compiletime/package.scala b/library/src/scala/compiletime/package.scala index 7152c2b110bd..cfe87d785218 100644 --- a/library/src/scala/compiletime/package.scala +++ b/library/src/scala/compiletime/package.scala @@ -38,4 +38,6 @@ package object compiletime { inline def constValue[T]: T = ??? type S[X <: Int] <: Int + + inline def memo[T](op: => T): T = ??? } diff --git a/tests/run/memoTest.scala b/tests/run/memoTest.scala index 460991eeaa59..4e39612f5194 100644 --- a/tests/run/memoTest.scala +++ b/tests/run/memoTest.scala @@ -1,4 +1,5 @@ object Test extends App { + import compiletime.memo var opCache: Int | Null = null @@ -7,6 +8,8 @@ object Test extends App { opCache.asInstanceOf[Int] + 1 } - assert(foo(1) + foo(2) == 4) + def bar(x: Int) = memo(x * x) + 1 + assert(foo(1) + foo(2) == 4) + assert(bar(1) + bar(2) == 4) } \ No newline at end of file From 9a7124f9e26c99e7d5c50fb80490fda794200e2a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 18 Jul 2019 22:57:35 +0200 Subject: [PATCH 02/18] Set position of memo cache ValDefs Maybe we should always set the positions of these generated definitions automatically from the symbol's span. It's an easy trap to fall into. --- compiler/src/dotty/tools/dotc/typer/Inliner.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index d386258fe003..5779a6bcf1eb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -199,7 +199,7 @@ object Inliner { case _ => syms } } - memoCacheSyms.map(ValDef(_, Literal(Constant(null)))) + memoCacheSyms.map(sym => ValDef(sym, Literal(Constant(null))).withSpan(sym.span)) } } @@ -425,11 +425,12 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { else Synthetic | Mutable | Private | Local, info = OrType(argType, defn.NullType), coord = call.span) + val memoRef = ref(memoVar).withSpan(call.span) val cond = If( - ref(memoVar).select(defn.Any_==).appliedTo(Literal(Constant(null))), - ref(memoVar).becomes(callValueArgss.head.head), + memoRef.select(defn.Any_==).appliedTo(Literal(Constant(null))), + memoRef.becomes(callValueArgss.head.head), Literal(Constant(()))) - val expr = ref(memoVar).cast(argType) + val expr = memoRef.cast(argType) Block(cond :: Nil, expr) } else errorTree(call, em"""memo(...) outside method""") From 4cc12e2dd4b465ff743981b74e4ac1dd1b9f0638 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 18 Jul 2019 22:58:45 +0200 Subject: [PATCH 03/18] Don't exit with StaleSymbol errors when testing pickling. Stale symbol errors can happen when inspecting symbols while comparing the trees before and after pickling. --- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index b40468716616..58854adf8ec5 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -102,8 +102,12 @@ trait SymDenotations { this: Context => } } - /** Configurable: Accept stale symbol with warning if in IDE */ - def staleOK: Boolean = Config.ignoreStaleInIDE && mode.is(Mode.Interactive) + /** Configurable: Accept stale symbol with warning if in IDE + * Always accept stale symbols when testing pickling. + */ + def staleOK: Boolean = + Config.ignoreStaleInIDE && mode.is(Mode.Interactive) || + settings.YtestPickler.value /** Possibly accept stale symbol with warning if in IDE */ def acceptStale(denot: SingleDenotation): Boolean = From 3404445e6fa45c0d394cccfd5423a2caf73a6caa Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 18 Jul 2019 23:37:07 +0200 Subject: [PATCH 04/18] Expand test --- tests/run/memoTest.scala | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/run/memoTest.scala b/tests/run/memoTest.scala index 4e39612f5194..87873c75731b 100644 --- a/tests/run/memoTest.scala +++ b/tests/run/memoTest.scala @@ -12,4 +12,24 @@ object Test extends App { assert(foo(1) + foo(2) == 4) assert(bar(1) + bar(2) == 4) + + class Context(val n: Int) + def f(c: Context): Context = { + println("computing f") + Context(c.n + 1) + } + given as Context(0) + + locally { + given as Context given (c: Context) = memo(f(c)) + println(the[Context].n) + println(the[Context].n) + } + + val ctx = f(the[Context]) + locally { + given as Context = ctx + println(the[Context].n) + println(the[Context].n) + } } \ No newline at end of file From 4005c430bef5407bab8b89b6b5b2ea20bd87a96a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 19 Jul 2019 16:09:34 +0200 Subject: [PATCH 05/18] Add docs --- docs/docs/reference/metaprogramming/inline.md | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/docs/docs/reference/metaprogramming/inline.md b/docs/docs/reference/metaprogramming/inline.md index 9b12b9e02d30..bed0c0e5b25f 100644 --- a/docs/docs/reference/metaprogramming/inline.md +++ b/docs/docs/reference/metaprogramming/inline.md @@ -406,6 +406,72 @@ inline def fail(p1: => Any) = { fail(indentity("foo")) // error: failed on: indentity("foo") ``` +#### `memo` + +The `memo` method is used to avoid repeated evaluation of subcomputations. +Example: +``` +class C(x: T) { + def costly(x: T): Int = ??? + def f(y: Int) = memo(costly(x)) * y +} +``` +Let's assume that `costly` is a pure function that is expensive to compute. If `f` was defined +like this: +``` + def f(y: Int) = costly(x) * y +``` +the `costly(x)` subexpression would be recomputed each time `f` was called, even though +its result is the same each time. With the addition of `memo(...)` the subexpression +in the parentheses is computed only the first time and is cached for subsequent recalculuations. +The memoized program expands to the following code: +``` +class C(x: T) { + def costly(x: T): Int = ??? + private[this] var memo$1: T | Null = null + def f(y: Int) = { + if (memo$1 == null) memo$1 = costly(x) + memo$1.asInstanceOf[T] + } * y +} +``` +The fine-print behind this expansion is: + + - The caching variable is placed next to the enclosing method (`f` in this case). + - Its type is the union of the type of the cached expression and `Null`. + - Its inital value is `null`. + - A `memo(op)` call is expanded to code that tests whether the cached variable is + null, in which case it reassignes the variable with the result of evaluating `op`. + The value of `memo(op)` is the value of the cached variable after this conditional assignment. + +In simple scenarios the call to `memo` is equivalent to using `lazy val`. For instance +the example program above could be simulated like this: +``` +class C(x: T) { + def costly(x: T): Int = ??? + @threadunsafe private[this] lazy val cached = costly(x) + def f(y: Int) = cached * y +} +``` +The advantage of using `memo` over lazy vals is that it's more concise. But `memo` could also be +used in scenarios where lazy vals are not suitable. For instance, let's assume +that the methods in class `C` above also need a given `Context` parameter. +``` +class C(x: T) { + def costly(x: T) given Context: Int = ??? + def f(y: Int) given (c: Context) = memo(costly(x) given c) * y +} +``` +Now, we cannot simply pull out the computation `costly(x) given c` into a lazy val since +it depends on the parameter `c` which is only available inside `f`. On the other hand, +it's much harder to argue that the `memo` solution is correct. One possible scenario +is that we fully intend to capture and reuse only the first computation of `costly(x)`. +Another possible scenario is that we do want `memo` to be semantically invisible, used +for optimization only, but that we convince ourselves that `costly(x) given c` would return +the same value no matter what context `c` is passed to `f`. That's a much harder argument +to make, but sometimes we can derive this from the global architecture of the system we are +dealing with. + ## Implicit Matches It is foreseen that many areas of typelevel programming can be done with rewrite From 1d710996ccb3b8a7edc9a1615dfcf41532aad196 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 20 Jul 2019 17:20:20 +0200 Subject: [PATCH 06/18] Add neg test --- tests/neg/memoTest.scala | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 tests/neg/memoTest.scala diff --git a/tests/neg/memoTest.scala b/tests/neg/memoTest.scala new file mode 100644 index 000000000000..952767b051e8 --- /dev/null +++ b/tests/neg/memoTest.scala @@ -0,0 +1,4 @@ +object Test { + import compiletime.memo + val a = memo(1) // error: memo(...) outside method +} \ No newline at end of file From dfd2af300b0c9f4c007610f306f80cd987e1bab9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 25 Jul 2019 18:52:23 +0200 Subject: [PATCH 07/18] Add missing def in doc --- docs/docs/reference/metaprogramming/inline.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/docs/reference/metaprogramming/inline.md b/docs/docs/reference/metaprogramming/inline.md index bed0c0e5b25f..e61c477ddc99 100644 --- a/docs/docs/reference/metaprogramming/inline.md +++ b/docs/docs/reference/metaprogramming/inline.md @@ -411,6 +411,7 @@ fail(indentity("foo")) // error: failed on: indentity("foo") The `memo` method is used to avoid repeated evaluation of subcomputations. Example: ``` +type T = ... class C(x: T) { def costly(x: T): Int = ??? def f(y: Int) = memo(costly(x)) * y From 5ec66c388cca1816825f781adc4fd16ba819e194 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 25 Jul 2019 19:16:57 +0200 Subject: [PATCH 08/18] Allow `memo` in traits --- compiler/src/dotty/tools/dotc/typer/Inliner.scala | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 5779a6bcf1eb..79284e342d49 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -16,6 +16,7 @@ import transform.SymUtils._ import Contexts.Context import Names.{Name, TermName} import NameKinds.{InlineAccessorName, InlineBinderName, InlineScrutineeName, MemoCacheName} +import NameOps._ import ProtoTypes.selectionProto import SymDenotations.SymDenotation import Inferencing.fullyDefinedType @@ -407,7 +408,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { /** The expansion of `memo(op)` where `op: T` is: * - * { if (memo$N == null) memo$N = op; $memo.asInstanceOf[T] } + * { if (memo$N == null) memo$N_=(op); $memo.asInstanceOf[T] } * * This creates as a side effect a memo cache symbol $memo$N` of type `T | Null`. * TODO: Restrict this to non-null types, once nullability checking is in. @@ -425,10 +426,17 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { else Synthetic | Mutable | Private | Local, info = OrType(argType, defn.NullType), coord = call.span) + val memoSetter = ctx.newSymbol( + owner = cacheOwner, + name = memoVar.name.setterName, + flags = memoVar.flags | Method | Accessor, + info = MethodType(argType :: Nil, defn.UnitType), + coord = call.span + ) val memoRef = ref(memoVar).withSpan(call.span) val cond = If( memoRef.select(defn.Any_==).appliedTo(Literal(Constant(null))), - memoRef.becomes(callValueArgss.head.head), + ref(memoSetter).withSpan(call.span).becomes(callValueArgss.head.head), Literal(Constant(()))) val expr = memoRef.cast(argType) Block(cond :: Nil, expr) From c1cb31d4938f52e0d730112b21703868327fb5ae Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jul 2019 08:55:12 +0200 Subject: [PATCH 09/18] Make memo work in traits ... and also make it work on several nested levels together. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 14 +++---- .../src/dotty/tools/dotc/core/NameKinds.scala | 2 +- .../src/dotty/tools/dotc/core/StdNames.scala | 1 + .../src/dotty/tools/dotc/core/Symbols.scala | 7 ++-- .../src/dotty/tools/dotc/typer/Inliner.scala | 41 ++++++++++++------- .../src/dotty/tools/dotc/typer/Typer.scala | 11 +++-- tests/run/memoTest.check | 7 ++++ tests/run/memoTest.scala | 16 ++++++++ 8 files changed, 65 insertions(+), 34 deletions(-) create mode 100644 tests/run/memoTest.check diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 17873cf62ecb..52bc778d8952 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -144,6 +144,9 @@ object desugar { // ----- Desugar methods ------------------------------------------------- + def setterNeeded(flags: FlagSet, owner: Symbol) given Context = + flags.is(Mutable) && owner.isClass && (!flags.isAllOf(PrivateLocal) || owner.is(Trait)) + /** var x: Int = expr * ==> * def x: Int = expr @@ -151,14 +154,7 @@ object desugar { */ def valDef(vdef0: ValDef)(implicit ctx: Context): Tree = { val vdef @ ValDef(name, tpt, rhs) = transformQuotedPatternName(vdef0) - val mods = vdef.mods - val setterNeeded = - mods.is(Mutable) && ctx.owner.isClass && (!mods.isAllOf(PrivateLocal) || ctx.owner.is(Trait)) - if (setterNeeded) { - // TODO: copy of vdef as getter needed? - // val getter = ValDef(mods, name, tpt, rhs) withPos vdef.pos? - // right now vdef maps via expandedTree to a thicket which concerns itself. - // I don't see a problem with that but if there is one we can avoid it by making a copy here. + if (setterNeeded(vdef.mods.flags, ctx.owner)) { val setterParam = makeSyntheticParameter(tpt = SetterParamTree().watching(vdef)) // The rhs gets filled in later, when field is generated and getter has parameters (see Memoize miniphase) val setterRhs = if (vdef.rhs.isEmpty) EmptyTree else unitLiteral @@ -168,7 +164,7 @@ object desugar { vparamss = (setterParam :: Nil) :: Nil, tpt = TypeTree(defn.UnitType), rhs = setterRhs - ).withMods((mods | Accessor) &~ (CaseAccessor | GivenOrImplicit | Lazy)) + ).withMods((vdef.mods | Accessor) &~ (CaseAccessor | GivenOrImplicit | Lazy)) Thicket(vdef, setter) } else vdef diff --git a/compiler/src/dotty/tools/dotc/core/NameKinds.scala b/compiler/src/dotty/tools/dotc/core/NameKinds.scala index 5ae5aee962c2..fca24d032fdb 100644 --- a/compiler/src/dotty/tools/dotc/core/NameKinds.scala +++ b/compiler/src/dotty/tools/dotc/core/NameKinds.scala @@ -299,7 +299,7 @@ object NameKinds { val UniqueInlineName: UniqueNameKind = new UniqueNameKind("$i") val InlineScrutineeName: UniqueNameKind = new UniqueNameKind("$scrutinee") val InlineBinderName: UniqueNameKind = new UniqueNameKind("$elem") - val MemoCacheName: UniqueNameKind = new UniqueNameKind("memo$") + val MemoCacheName: UniqueNameKind = new UniqueNameKind("$cache") /** A kind of unique extension methods; Unlike other unique names, these can be * unmangled. diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index e331c3a84dc1..f7dd2da26ace 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -482,6 +482,7 @@ object StdNames { val materializeClassTag: N = "materializeClassTag" val materializeWeakTypeTag: N = "materializeWeakTypeTag" val materializeTypeTag: N = "materializeTypeTag" + val memo: N = "memo" val mirror : N = "mirror" val moduleClass : N = "moduleClass" val name: N = "name" diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index 24d2c188a419..18610a450257 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -553,9 +553,10 @@ object Symbols { /** This symbol entered into owner's scope (owner must be a class). */ final def entered(implicit ctx: Context): this.type = { - assert(this.owner.isClass, s"symbol ($this) entered the scope of non-class owner ${this.owner}") // !!! DEBUG - this.owner.asClass.enter(this) - if (this.is(Module)) this.owner.asClass.enter(this.moduleClass) + if (this.owner.isClass) { + this.owner.asClass.enter(this) + if (this.is(Module)) this.owner.asClass.enter(this.moduleClass) + } this } diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 79284e342d49..5434d09236cf 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -193,14 +193,22 @@ object Inliner { /** For every occurrence of a memo cache symbol `memo$N` of type `T_N` in `tree`, * an assignment `val memo$N: T_N = null` */ - def memoCacheDefs(tree: Tree) given Context: Set[ValDef] = { - val memoCacheSyms = tree.deepFold[Set[TermSymbol]](Set.empty) { - (syms, t) => t match { - case Assign(lhs, _) if lhs.symbol.name.is(MemoCacheName) => syms + lhs.symbol.asTerm - case _ => syms + def memoCacheDefs(tree: Tree) given Context: List[ValOrDefDef] = { + object memoRefs extends TreeTraverser { + val syms = new mutable.LinkedHashSet[TermSymbol] + def traverse(tree: Tree) given Context = tree match { + case tree: RefTree if tree.symbol.name.is(MemoCacheName) => + syms += tree.symbol.asTerm + case _: DefDef => + // don't traverse deeper; nested memo caches go next to nested method + case _ => + traverseChildren(tree) } } - memoCacheSyms.map(sym => ValDef(sym, Literal(Constant(null))).withSpan(sym.span)) + memoRefs.traverse(tree) + for sym <- memoRefs.syms.toList yield + (if (sym.isSetter) DefDef(sym, _ => Literal(Constant(()))) + else ValDef(sym, Literal(Constant(null)))).withSpan(sym.span) } } @@ -420,19 +428,22 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { val argType = callTypeArgs.head.tpe val memoVar = ctx.newSymbol( owner = cacheOwner, - name = MemoCacheName.fresh(), + name = MemoCacheName.fresh(nme.memo), flags = if (cacheOwner.isTerm) Synthetic | Mutable else Synthetic | Mutable | Private | Local, info = OrType(argType, defn.NullType), - coord = call.span) - val memoSetter = ctx.newSymbol( - owner = cacheOwner, - name = memoVar.name.setterName, - flags = memoVar.flags | Method | Accessor, - info = MethodType(argType :: Nil, defn.UnitType), - coord = call.span - ) + coord = call.span).entered + val memoSetter = + if (desugar.setterNeeded(memoVar.flags, cacheOwner)) + ctx.newSymbol( + owner = cacheOwner, + name = memoVar.name.setterName, + flags = memoVar.flags | Method | Accessor, + info = MethodType(argType :: Nil, defn.UnitType), + coord = call.span + ).entered + else memoVar val memoRef = ref(memoVar).withSpan(call.span) val cond = If( memoRef.select(defn.Any_==).appliedTo(Literal(Constant(null))), diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 6f33c8e5e369..6b90ec74e79f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2122,19 +2122,18 @@ class Typer extends Namer case Some(xtree) => traverse(xtree :: rest) case none => - val memoCacheCount = MemoCacheName.currentCount() + val memoCacheCount = MemoCacheName.currentCount(nme.memo) typed(mdef) match { case mdef1: DefDef if Inliner.hasBodyToInline(mdef1.symbol) => buf += inlineExpansion(mdef1) // replace body with expansion, because it will be used as inlined body // from separately compiled files - the original BodyAnnotation is not kept. case mdef1 => - import untpd.modsDeco - mdef match { - case mdef: untpd.TypeDef if mdef.mods.isEnumClass => + mdef1 match { + case mdef1: TypeDef if mdef1.symbol.flags.is(Enum, butNot = Case) => enumContexts(mdef1.symbol) = ctx - case _: untpd.DefDef if MemoCacheName.currentCount() != memoCacheCount => - buf ++= Inliner.memoCacheDefs(mdef1) + case mdef1: DefDef if MemoCacheName.currentCount(nme.memo) != memoCacheCount => + buf ++= Inliner.memoCacheDefs(mdef1.rhs) case _ => } if (!mdef1.isEmpty) // clashing synthetic case methods are converted to empty trees diff --git a/tests/run/memoTest.check b/tests/run/memoTest.check new file mode 100644 index 000000000000..bcc6c67fe293 --- /dev/null +++ b/tests/run/memoTest.check @@ -0,0 +1,7 @@ +computing inner +computing f +1 +1 +computing f +1 +1 diff --git a/tests/run/memoTest.scala b/tests/run/memoTest.scala index 87873c75731b..a265a816525d 100644 --- a/tests/run/memoTest.scala +++ b/tests/run/memoTest.scala @@ -13,6 +13,22 @@ object Test extends App { assert(foo(1) + foo(2) == 4) assert(bar(1) + bar(2) == 4) + trait T { + def x: Int + def y: Int = memo { + def inner = memo { + println("computing inner"); + x * x + } + inner + inner + } + } + val t = new T { + def x = 3 + assert(y == 18) + } + assert(t.y == 18) + class Context(val n: Int) def f(c: Context): Context = { println("computing f") From 814c1f0fe8edece480def8ef51a429f8cf7b3989 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jul 2019 09:17:48 +0200 Subject: [PATCH 10/18] Streamline Typer#traverse --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 6b90ec74e79f..325409d02372 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2128,16 +2128,15 @@ class Typer extends Namer buf += inlineExpansion(mdef1) // replace body with expansion, because it will be used as inlined body // from separately compiled files - the original BodyAnnotation is not kept. + case mdef1: DefDef if MemoCacheName.currentCount(nme.memo) != memoCacheCount => + buf ++= Inliner.memoCacheDefs(mdef1.rhs) += mdef1 + case mdef1: TypeDef if mdef1.symbol.is(Enum, butNot = Case) => + enumContexts(mdef1.symbol) = ctx + buf += mdef1 + case EmptyTree => + // clashing synthetic case methods are converted to empty trees, drop them here case mdef1 => - mdef1 match { - case mdef1: TypeDef if mdef1.symbol.flags.is(Enum, butNot = Case) => - enumContexts(mdef1.symbol) = ctx - case mdef1: DefDef if MemoCacheName.currentCount(nme.memo) != memoCacheCount => - buf ++= Inliner.memoCacheDefs(mdef1.rhs) - case _ => - } - if (!mdef1.isEmpty) // clashing synthetic case methods are converted to empty trees - buf += mdef1 + buf += mdef1 } traverse(rest) } From f93b49da903db5694de91c1735c15225044a20ce Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jul 2019 09:26:47 +0200 Subject: [PATCH 11/18] Make enteredAfter also work for term members Restricting `entered` and `enteredAfter` to class members is more a trap to fall into than a helpful check. --- .../src/dotty/tools/dotc/core/Symbols.scala | 18 ++++++++++-------- .../dotc/transform/CacheAliasImplicits.scala | 2 +- .../tools/dotc/transform/HoistSuperArgs.scala | 6 +++--- .../tools/dotc/transform/LambdaLift.scala | 5 +++-- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index 18610a450257..3aaf8cb80838 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -567,14 +567,16 @@ object Symbols { */ def enteredAfter(phase: DenotTransformer)(implicit ctx: Context): this.type = if (ctx.phaseId != phase.next.id) enteredAfter(phase)(ctx.withPhase(phase.next)) - else { - if (this.owner.is(Package)) { - denot.validFor |= InitialPeriod - if (this.is(Module)) this.moduleClass.validFor |= InitialPeriod - } - else this.owner.asClass.ensureFreshScopeAfter(phase) - assert(isPrivate || phase.changesMembers, i"$this entered in ${this.owner} at undeclared phase $phase") - entered + else this.owner match { + case owner: ClassSymbol => + if (owner.is(Package)) { + denot.validFor |= InitialPeriod + if (this.is(Module)) this.moduleClass.validFor |= InitialPeriod + } + else owner.ensureFreshScopeAfter(phase) + assert(isPrivate || phase.changesMembers, i"$this entered in $owner at undeclared phase $phase") + entered + case _ => this } /** Remove symbol from scope of owning class */ diff --git a/compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala b/compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala index 29eae5107ec7..9cf5fd216388 100644 --- a/compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala +++ b/compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala @@ -82,7 +82,7 @@ class CacheAliasImplicits extends MiniPhase with IdentityDenotTransformer { this val cacheFlags = if (ctx.owner.isClass) Private | Local | Mutable else Mutable val cacheSym = ctx.newSymbol(ctx.owner, CacheName(tree.name), cacheFlags, rhsType, coord = sym.coord) - if (ctx.owner.isClass) cacheSym.enteredAfter(thisPhase) + .enteredAfter(thisPhase) val cacheDef = ValDef(cacheSym, tpd.defaultValue(rhsType)) val cachingDef = cpy.DefDef(tree)(rhs = Block( diff --git a/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala b/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala index 302d2330d056..d72f0895ca49 100644 --- a/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala +++ b/compiler/src/dotty/tools/dotc/transform/HoistSuperArgs.scala @@ -91,13 +91,13 @@ class HoistSuperArgs extends MiniPhase with IdentityDenotTransformer { thisPhase val argTypeWrtConstr = argType.subst(origParams, allParamRefs(constr.info)) // argType with references to paramRefs of the primary constructor instead of // local parameter accessors - val meth = ctx.newSymbol( + ctx.newSymbol( owner = methOwner, name = SuperArgName.fresh(cls.name.toTermName), flags = Synthetic | Private | Method | staticFlag, info = replaceResult(constr.info, argTypeWrtConstr), - coord = constr.coord) - if (methOwner.isClass) meth.enteredAfter(thisPhase) else meth + coord = constr.coord + ).enteredAfter(thisPhase) } /** Type of a reference implies that it needs to be hoisted */ diff --git a/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala b/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala index 4bd73238797b..548f594865b1 100644 --- a/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala +++ b/compiler/src/dotty/tools/dotc/transform/LambdaLift.scala @@ -298,8 +298,9 @@ object LambdaLift { proxyMap(owner) = { for (fv <- freeValues.toList) yield { val proxyName = newName(fv) - val proxy = ctx.newSymbol(owner, proxyName.asTermName, newFlags, fv.info, coord = fv.coord) - if (owner.isClass) proxy.enteredAfter(thisPhase) + val proxy = + ctx.newSymbol(owner, proxyName.asTermName, newFlags, fv.info, coord = fv.coord) + .enteredAfter(thisPhase) (fv, proxy) } }.toMap From ffec99c72442fae084baaa162d91b4718d7faaaa Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jul 2019 14:26:13 +0200 Subject: [PATCH 12/18] Fix owner of memo cache --- compiler/src/dotty/tools/dotc/typer/Inliner.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala index 5434d09236cf..6dafc0fa6932 100644 --- a/compiler/src/dotty/tools/dotc/typer/Inliner.scala +++ b/compiler/src/dotty/tools/dotc/typer/Inliner.scala @@ -424,7 +424,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) { def memoized: Tree = { val currentOwner = ctx.owner.skipWeakOwner if (currentOwner.isRealMethod) { - val cacheOwner = ctx.owner.effectiveOwner + val cacheOwner = currentOwner.owner val argType = callTypeArgs.head.tpe val memoVar = ctx.newSymbol( owner = cacheOwner, From 5f6a37f08a8395da0a3918849d419b5ca4609d07 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jul 2019 11:27:43 +0200 Subject: [PATCH 13/18] Fix #6909: Use memo to cache given aliases --- compiler/src/dotty/tools/dotc/Compiler.scala | 1 - .../src/dotty/tools/dotc/ast/Desugar.scala | 6 +- .../src/dotty/tools/dotc/core/NameKinds.scala | 1 - .../src/dotty/tools/dotc/core/NameTags.scala | 2 - .../dotty/tools/dotc/parsing/Parsers.scala | 10 +- .../dotc/transform/CacheAliasImplicits.scala | 104 ------------------ .../dotc/typer/MemoizeGivenAliases.scala | 59 ++++++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 4 +- 8 files changed, 70 insertions(+), 117 deletions(-) delete mode 100644 compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala create mode 100644 compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 56e71c29110b..945171da45d7 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -63,7 +63,6 @@ class Compiler { new ExpandSAMs, // Expand single abstract method closures to anonymous classes new ProtectedAccessors, // Add accessors for protected members new ExtensionMethods, // Expand methods of value classes with extension methods - new CacheAliasImplicits, // Cache RHS of parameterless alias implicits new ShortcutImplicits, // Allow implicit functions without creating closures new ByNameClosures, // Expand arguments to by-name parameters to closures new HoistSuperArgs, // Hoist complex arguments of supercalls to enclosing scope diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 52bc778d8952..457fddd7ca36 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -887,7 +887,7 @@ object desugar { /** The normalized name of `mdef`. This means * 1. Check that the name does not redefine a Scala core class. * If it does redefine, issue an error and return a mangled name instead of the original one. - * 2. If the name is missing (this can be the case for instance definitions), invent one instead. + * 2. If the name is missing (this can be the case for given instance definitions), invent one instead. */ def normalizeName(mdef: MemberDef, impl: Tree)(implicit ctx: Context): Name = { var name = mdef.name @@ -900,7 +900,7 @@ object desugar { name } - /** Invent a name for an anonymous instance with template `impl`. + /** Invent a name for an anonymous given instance with template `impl`. */ private def inventName(impl: Tree)(implicit ctx: Context): String = impl match { case impl: Template => @@ -912,7 +912,7 @@ object desugar { case Some(DefDef(name, _, (vparam :: _) :: _, _, _)) => s"${name}_of_${inventTypeName(vparam.tpt)}" case _ => - ctx.error(i"anonymous instance must have `for` part or must define at least one extension method", impl.sourcePos) + ctx.error(i"anonymous given must have `as` part or must define at least one extension method", impl.sourcePos) nme.ERROR.toString } else diff --git a/compiler/src/dotty/tools/dotc/core/NameKinds.scala b/compiler/src/dotty/tools/dotc/core/NameKinds.scala index fca24d032fdb..3c78d1311574 100644 --- a/compiler/src/dotty/tools/dotc/core/NameKinds.scala +++ b/compiler/src/dotty/tools/dotc/core/NameKinds.scala @@ -363,7 +363,6 @@ object NameKinds { val InlineAccessorName: PrefixNameKind = new PrefixNameKind(INLINEACCESSOR, "inline$") val AvoidClashName: SuffixNameKind = new SuffixNameKind(AVOIDCLASH, "$_avoid_name_clash_$") - val CacheName = new SuffixNameKind(CACHE, "$_cache") val DirectMethodName: SuffixNameKind = new SuffixNameKind(DIRECT, "$direct") { override def definesNewName = true } val FieldName: SuffixNameKind = new SuffixNameKind(FIELD, "$$local") { override def mkString(underlying: TermName, info: ThisInfo) = underlying.toString diff --git a/compiler/src/dotty/tools/dotc/core/NameTags.scala b/compiler/src/dotty/tools/dotc/core/NameTags.scala index fe351ccc61e7..f5c2923ffb11 100644 --- a/compiler/src/dotty/tools/dotc/core/NameTags.scala +++ b/compiler/src/dotty/tools/dotc/core/NameTags.scala @@ -36,8 +36,6 @@ object NameTags extends TastyFormat.NameTags { final val IMPLMETH = 32 // Used to define methods in implementation classes // (can probably be removed). - final val CACHE = 33 // Used as a cache for the rhs of an alias implicit. - def nameTagToString(tag: Int): String = tag match { case UTF8 => "UTF8" case QUALIFIED => "QUALIFIED" diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 28f7ac1f32fa..230f675de2cf 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -2334,7 +2334,7 @@ object Parsers { * given C ... * we know that `given` must start a parameter list. It cannot be a new given` definition. */ - def followingIsInstanceDef = + def followingIsGivenDef = (ofClass || ofInstance) && { val lookahead = in.lookaheadScanner // skips newline on startup lookahead.nextToken() // skip the `given` @@ -2362,7 +2362,7 @@ object Parsers { var initialMods = EmptyModifiers val isNewLine = in.token == NEWLINE newLineOptWhenFollowedBy(LPAREN) - if (in.token == NEWLINE && in.next.token == GIVEN && !followingIsInstanceDef) + if (in.token == NEWLINE && in.next.token == GIVEN && !followingIsGivenDef) in.nextToken() if (in.token == GIVEN) { in.nextToken() @@ -2765,7 +2765,7 @@ object Parsers { case ENUM => enumDef(start, posMods(start, mods | Enum)) case IMPLIED | GIVEN => - instanceDef(in.token == GIVEN, start, mods, atSpan(in.skipToken()) { Mod.Given() }) + givenDef(in.token == GIVEN, start, mods, atSpan(in.skipToken()) { Mod.Given() }) case _ => syntaxErrorOrIncomplete(ExpectedStartOfTopLevelDefinition()) EmptyTree @@ -2861,7 +2861,7 @@ object Parsers { * GivenBody ::= [‘as ConstrApp {‘,’ ConstrApp }] {GivenParamClause} [TemplateBody] * | ‘as’ Type {GivenParamClause} ‘=’ Expr */ - def instanceDef(newStyle: Boolean, start: Offset, mods: Modifiers, instanceMod: Mod) = atSpan(start, nameStart) { + def givenDef(newStyle: Boolean, start: Offset, mods: Modifiers, instanceMod: Mod) = atSpan(start, nameStart) { var mods1 = addMod(mods, instanceMod) val name = if (isIdent && (!newStyle || in.name != nme.as)) ident() else EmptyTermName val tparams = typeParamClauseOpt(ParamOwner.Def) @@ -3178,7 +3178,7 @@ object Parsers { val mods = modifiers(closureMods) mods.mods match { case givenMod :: Nil if !isBindingIntro => - stats += instanceDef(true, start, EmptyModifiers, Mod.Given().withSpan(givenMod.span)) + stats += givenDef(true, start, EmptyModifiers, Mod.Given().withSpan(givenMod.span)) case _ => stats += implicitClosure(in.offset, Location.InBlock, mods) } diff --git a/compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala b/compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala deleted file mode 100644 index 9cf5fd216388..000000000000 --- a/compiler/src/dotty/tools/dotc/transform/CacheAliasImplicits.scala +++ /dev/null @@ -1,104 +0,0 @@ -package dotty.tools.dotc -package transform - -import MegaPhase._ -import core.DenotTransformers.{IdentityDenotTransformer} -import core.Symbols._ -import core.Contexts._ -import core.Types._ -import core.Flags._ -import core.StdNames.nme -import core.NameKinds.CacheName -import core.Constants.Constant -import core.Decorators._ -import core.TypeErasure.erasure -import ast.tpd - -object CacheAliasImplicits { - val name: String = "cacheAliasImplicits" - - /** Flags that disable caching */ - val NoCacheFlags = - StableRealizable | // It's a simple forwarder, leave it as one - Exported // Export forwarders are never cached -} - -/** This phase ensures that the right hand side of parameterless alias implicits - * is cached. It applies to all alias implicits that have neither type parameters - * nor a given clause. Example: The alias - * - * implicit a for TC = rhs - * - * is expanded before this phase - * - * implicit def a: TC = rhs - * - * It is then expanded further as follows: - * - * 1. If `rhs` is a simple name `x` (possibly with a `this.` prefix), leave the definition as is. - * 2. Otherwise, if `rhs` is a pure path, replace the definition with - * - * implicit val a: TC = rhs - * - * 3. Otherwise, if `TC` is a reference type, replace the definition with - * - * private[this] var a$_cache: TC = null - * implicit def a: TC = { if (a$_cache == null) a$_cache = rhs; a$_cache } - * - * 4. Otherwise `TC` is a value type. Replace the definition with - * - * lazy implicit val a: TC = rhs - */ -class CacheAliasImplicits extends MiniPhase with IdentityDenotTransformer { thisPhase => - import tpd._ - - override def phaseName: String = CacheAliasImplicits.name - - override def transformDefDef(tree: DefDef)(implicit ctx: Context): Tree = { - val sym = tree.symbol - sym.info match { - case ExprType(rhsType) if sym.is(Given, butNot = CacheAliasImplicits.NoCacheFlags) => - // If rhs is a simple TermRef, leave a def. - tree.rhs.tpe match { - case TermRef(pre, _) => - pre match { - case NoPrefix => return tree - case pre: ThisType if pre.cls == ctx.owner.enclosingClass => return tree - case _ => - } - case _ => - } - def makeVal(additionalFlags: FlagSet) = { - sym.copySymDenotation( - initFlags = sym.flags &~ Method | additionalFlags, - info = rhsType) - .installAfter(thisPhase) - cpy.ValDef(tree)(tree.name, tree.tpt, tree.rhs) - } - if (isPurePath(tree.rhs)) makeVal(EmptyFlags) - else if (rhsType.classSymbol.isValueClass || - !erasure(rhsType).typeSymbol.derivesFrom(defn.ObjectClass)) makeVal(Lazy) - else { - val cacheFlags = if (ctx.owner.isClass) Private | Local | Mutable else Mutable - val cacheSym = - ctx.newSymbol(ctx.owner, CacheName(tree.name), cacheFlags, rhsType, coord = sym.coord) - .enteredAfter(thisPhase) - val cacheDef = ValDef(cacheSym, tpd.defaultValue(rhsType)) - val cachingDef = cpy.DefDef(tree)(rhs = - Block( - If( - ref(cacheSym).select(defn.Any_==).appliedTo(nullLiteral), - Assign(ref(cacheSym), tree.rhs), - unitLiteral) :: Nil, - ref(cacheSym) - ) - ) - Thicket(cacheDef, cachingDef) - } - case _ => tree - } - } -} - - - diff --git a/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala b/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala new file mode 100644 index 000000000000..88c353db0873 --- /dev/null +++ b/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala @@ -0,0 +1,59 @@ +package dotty.tools.dotc +package typer + +import core.Symbols._ +import core.Contexts._ +import core.Types._ +import core.Flags._ +import core.Decorators._ +import ast.{untpd, tpd} + +object MemoizeGivenAliases { + + /** Flags that disable caching */ + val NoCacheFlags = + StableRealizable | // It's a simple forwarder, leave it as one + Exported // Export forwarders are never cached +} + +trait MemoizeGivenAliases { this: Typer => + import tpd._ + + /** Ensure that the right hand side of a parameterless given alias + * is cached. This applies to all given aliases that have neither type parameters + * nor a given clause. Example: The given alias + * + * given a as TC = rhs + * + * is desugared to + * + * given def a: TC = rhs + * + * It is then expanded as follows: + * + * 1. If `rhs` is a simple name `x` (possibly with a `this.` prefix), leave the definition as is. + * 2. Otherwise, wrap `rhs` in a call to `scala.compiletime.memo`. + */ + def memoizeGivenAlias(rhs: Tree, meth: Symbol) given Context : Tree = meth.info match { + case ExprType(rhsType) if meth.is(Given, butNot = MemoizeGivenAliases.NoCacheFlags) => + // If rhs is a simple stable TermRef, leave as is. + val needsMemo = rhs.tpe match { + case rhsTpe @ TermRef(pre, _) if rhsTpe.isStable => + pre match { + case NoPrefix => false + case pre: ThisType => pre.cls != meth.owner.enclosingClass + } + case _ => true + } + if (needsMemo) { + val memoized = + untpd.Apply(untpd.ref(defn.Compiletime_memo.termRef), untpd.TypedSplice(rhs) :: Nil) + typed(memoized, rhsType) + } + else rhs + case _ => rhs + } +} + + + diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 325409d02372..73c135bf20bf 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -88,6 +88,7 @@ class Typer extends Namer with Dynamic with Checking with QuotesAndSplices + with MemoizeGivenAliases with Deriving { import Typer._ @@ -1538,7 +1539,8 @@ class Typer extends Namer } if (sym.isInlineMethod) rhsCtx.addMode(Mode.InlineableBody) - val rhs1 = typedExpr(ddef.rhs, tpt1.tpe.widenExpr)(rhsCtx) + val rhs0 = typedExpr(ddef.rhs, tpt1.tpe.widenExpr)(rhsCtx) + val rhs1 = memoizeGivenAlias(rhs0, sym) if (sym.isInlineMethod) { PrepareInlineable.checkInlineMacro(sym, rhs1, ddef.sourcePos) From a4df82058806cb7bff43b9580107824f06241f56 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jul 2019 11:35:18 +0200 Subject: [PATCH 14/18] Update docs The new implementation does not optimize given aliases with pure paths as right hand sides to be vals anymore. The reason is that even a pure path might be expensive to compute so we might want the caching. And caching is cheap, so there's little downside. Therefore, it's attractive to go with the simpler two way choice: leave RHS as is, or wrap it in a memo. --- .../reference/contextual/relationship-implicits.md | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/docs/docs/reference/contextual/relationship-implicits.md b/docs/docs/reference/contextual/relationship-implicits.md index 95b1f160ed0f..956a5865cd0c 100644 --- a/docs/docs/reference/contextual/relationship-implicits.md +++ b/docs/docs/reference/contextual/relationship-implicits.md @@ -28,18 +28,12 @@ Given instances can be mapped to combinations of implicit objects, classes and i class ListOrd[T](implicit ord: Ord[T]) extends Ord[List[T]] { ... } final implicit def ListOrd[T](implicit ord: Ord[T]): ListOrd[T] = new ListOrd[T] ``` - 3. Alias givens map to implicit methods. If an alias has neither type parameters nor a given clause, its right-hand side is cached in a variable. There are two cases that can be optimized: - - - If the right hand side is a simple reference, we can - use a forwarder to that reference without caching it. - - If the right hand side is more complex, but still known to be pure, we can - create a `val` that computes it ahead of time. + 3. Alias givens map to implicit methods. If an alias has neither type parameters nor a given clause, its right-hand side is cached in a variable, unless the right hand side is a simple reference in which case we can use a forwarder to that reference without caching it. Examples: ```scala given global as ExecutionContext = new ForkJoinContext() - given config as Config = default.config val ctx: Context given as Context = ctx @@ -52,8 +46,6 @@ Given instances can be mapped to combinations of implicit objects, classes and i global$cache } - final implicit val config: Config = default.config - final implicit def Context_given = ctx ``` From 74d8c8e2bd5f42a51d1e220ad8af14743b680bbf Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jul 2019 14:03:53 +0200 Subject: [PATCH 15/18] Fix memoizeGivenAliases --- .../src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala b/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala index 88c353db0873..fd2ad3af6690 100644 --- a/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala +++ b/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala @@ -42,13 +42,13 @@ trait MemoizeGivenAliases { this: Typer => pre match { case NoPrefix => false case pre: ThisType => pre.cls != meth.owner.enclosingClass + case _ => true } case _ => true } if (needsMemo) { - val memoized = - untpd.Apply(untpd.ref(defn.Compiletime_memo.termRef), untpd.TypedSplice(rhs) :: Nil) - typed(memoized, rhsType) + val memoized = ref(defn.Compiletime_memo).appliedToType(rhsType).appliedTo(rhs) + adapt(memoized, rhsType) } else rhs case _ => rhs From b73e372d75233162c21c8c45d1d6f0765ba0feee Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 26 Jul 2019 17:19:47 +0200 Subject: [PATCH 16/18] Don't flag memo cache names as race-prone in reentrancy checking --- compiler/src/dotty/tools/dotc/transform/CheckReentrant.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckReentrant.scala b/compiler/src/dotty/tools/dotc/transform/CheckReentrant.scala index 9b950869f7d5..52b46df685e1 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckReentrant.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckReentrant.scala @@ -7,6 +7,7 @@ import Flags._ import Contexts.Context import Symbols._ import Decorators._ +import NameKinds.MemoCacheName /** A no-op transform that checks whether the compiled sources are re-entrant. @@ -44,6 +45,7 @@ class CheckReentrant extends MiniPhase { def isIgnored(sym: Symbol)(implicit ctx: Context): Boolean = sym.hasAnnotation(sharableAnnot()) || sym.hasAnnotation(unsharedAnnot()) || + sym.name.is(MemoCacheName) || sym.owner == defn.EnumValuesClass // enum values are initialized eagerly before use // in the long run, we should make them vals From c619dd7abf3826bb983d642295a37d8072e9407b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 27 Jul 2019 11:28:42 +0200 Subject: [PATCH 17/18] Don't cache given instances that are macros. An inline given instance with a splice as right hand side should not be cached. --- .../src/dotty/tools/dotc/ast/TreeInfo.scala | 12 ++++++++- .../dotc/typer/MemoizeGivenAliases.scala | 26 ++++++++++--------- .../contextual/relationship-implicits.md | 2 +- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala index e655927c2eb4..60be301cc41c 100644 --- a/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala +++ b/compiler/src/dotty/tools/dotc/ast/TreeInfo.scala @@ -3,7 +3,7 @@ package dotc package ast import core._ -import Flags._, Trees._, Types._, Contexts._ +import Flags._, Trees._, Types._, Contexts._, Constants.Constant import Names._, StdNames._, NameOps._, Symbols._ import typer.ConstFold import reporting.trace @@ -856,6 +856,16 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => case _ => None } } + + object SplicedRHS { + /** Extracts splices, possibly wrapped in blocks or type ascriptions */ + def unapply(tree: tpd.Tree)(implicit ctx: Context): Option[tpd.Tree] = tree match { + case Block(stat :: Nil, Literal(Constant(()))) => unapply(stat) + case Block(Nil, expr) => unapply(expr) + case Typed(expr, _) => unapply(expr) + case _ => Spliced.unapply(tree) + } + } } object TreeInfo { diff --git a/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala b/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala index fd2ad3af6690..2d1718cba53f 100644 --- a/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala +++ b/compiler/src/dotty/tools/dotc/typer/MemoizeGivenAliases.scala @@ -31,29 +31,31 @@ trait MemoizeGivenAliases { this: Typer => * * It is then expanded as follows: * - * 1. If `rhs` is a simple name `x` (possibly with a `this.` prefix), leave the definition as is. + * 1. If `rhs` is a simple name `x` (possibly with a `this.` prefix), + * or `rhs` is a splice, leave the definition as is. * 2. Otherwise, wrap `rhs` in a call to `scala.compiletime.memo`. */ def memoizeGivenAlias(rhs: Tree, meth: Symbol) given Context : Tree = meth.info match { case ExprType(rhsType) if meth.is(Given, butNot = MemoizeGivenAliases.NoCacheFlags) => // If rhs is a simple stable TermRef, leave as is. - val needsMemo = rhs.tpe match { - case rhsTpe @ TermRef(pre, _) if rhsTpe.isStable => - pre match { - case NoPrefix => false - case pre: ThisType => pre.cls != meth.owner.enclosingClass - case _ => true - } - case _ => true + val needsMemo = rhs match { + case SplicedRHS(_) => false + case _ => rhs.tpe match { + case rhsTpe @ TermRef(pre, _) if rhsTpe.isStable => + pre match { + case NoPrefix => false + case pre: ThisType => pre.cls != meth.owner.enclosingClass + case _ => true + } + case _ => true + } } if (needsMemo) { val memoized = ref(defn.Compiletime_memo).appliedToType(rhsType).appliedTo(rhs) - adapt(memoized, rhsType) + adapt(memoized, rhsType) // this will do the inlining of `memo` } else rhs case _ => rhs } } - - diff --git a/docs/docs/reference/contextual/relationship-implicits.md b/docs/docs/reference/contextual/relationship-implicits.md index 956a5865cd0c..62e7edb1de1c 100644 --- a/docs/docs/reference/contextual/relationship-implicits.md +++ b/docs/docs/reference/contextual/relationship-implicits.md @@ -28,7 +28,7 @@ Given instances can be mapped to combinations of implicit objects, classes and i class ListOrd[T](implicit ord: Ord[T]) extends Ord[List[T]] { ... } final implicit def ListOrd[T](implicit ord: Ord[T]): ListOrd[T] = new ListOrd[T] ``` - 3. Alias givens map to implicit methods. If an alias has neither type parameters nor a given clause, its right-hand side is cached in a variable, unless the right hand side is a simple reference in which case we can use a forwarder to that reference without caching it. + 3. Alias givens map to implicit methods. If an alias is not a macro, and has neither type parameters nor a given clause, its right-hand side is cached in a variable, unless the right hand side is a simple reference in which case we can use a forwarder to that reference without caching it. Examples: From 68862b1bd2b3f41faf93f958676d9d100974d29d Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 28 Jul 2019 15:48:07 +0200 Subject: [PATCH 18/18] Add test --- tests/pos/i6909.scala | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/pos/i6909.scala diff --git a/tests/pos/i6909.scala b/tests/pos/i6909.scala new file mode 100644 index 000000000000..dd1f8e91d8da --- /dev/null +++ b/tests/pos/i6909.scala @@ -0,0 +1,14 @@ +import scala.compiletime.memo +trait Foo[A] + + +trait Qux { + private[this] var x: Int | Null = null + def f = { + if (x == null) x = 22 + x.asInstanceOf[Int] + } + def g = memo(new Foo[Int] {}) +// + //given as Foo[Int] = new Foo[Int] {} +} \ No newline at end of file