From a5363c4e08d01c32501036af6aeeb8a6c07e0d3b Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Mon, 19 Dec 2016 13:25:28 +0100 Subject: [PATCH 1/3] Fix #1812, Symbols.mapSymbols shouldn't replace denotations It will use lazy types instead. The current version transforms a type, with a context that has denotations that may be forcefully replaced by mapSymbols. Types created during this transformation may cache denots, that are-to-be replaced. This is very problematic as this method is called from TreeTypeMap.withMappedSyms in a fixed-point cycle, creating new symbols on every iteration. Those cached denotations could make types keep symbols from previous iterations indefinitely. The changed version does not transform the types eagerly, and instead makes them lazy. Assuming there are no cycles, this should ensure correct ordering. Unfortunatelly, at this point in the compiler we basically always touch everything, and we can't even transform the info of denotation without this denotations info. We basically have a chicked&egg problem here. To solve it, I use the same trick as used by other lazy types by assigning an approximation of future type first. This allows to pass the tests and makes dotty more robust, but I suspect this isn't a complete fix and new similar bugs may arrive. --- .../src/dotty/tools/dotc/core/Symbols.scala | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index cfd85c49cd0e..c475e99f533b 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -313,10 +313,7 @@ trait Symbols { this: Context => newNakedSymbol[original.ThisName](original.coord) } val ttmap1 = ttmap.withSubstitution(originals, copies) - (originals, copies).zipped foreach {(original, copy) => - copy.denot = original.denot // preliminary denotation, so that we can access symbols in subsequent transform - } - (originals, copies).zipped foreach {(original, copy) => + (originals, copies).zipped foreach { (original, copy) => val odenot = original.denot val oinfo = original.info match { case ClassInfo(pre, _, parents, decls, selfInfo) => @@ -324,14 +321,26 @@ trait Symbols { this: Context => ClassInfo(pre, copy.asClass, parents, decls.cloneScope, selfInfo) case oinfo => oinfo } + + val completer = new LazyType { + def complete(denot: SymDenotation)(implicit ctx: Context): Unit = { + denot.info = oinfo // needed as otherwise we won't be able to go from Sym -> parents & etc + // Note that this is a hack, but hack commonly used in Dotty + // The same thing is done by other completers all the time + denot.info = ttmap1.mapType(oinfo) + } + } + copy.denot = odenot.copySymDenotation( symbol = copy, owner = ttmap1.mapOwner(odenot.owner), - initFlags = odenot.flags &~ Frozen | Fresh, - info = ttmap1.mapType(oinfo), + initFlags = odenot.flags &~ (Frozen | Touched) | Fresh, + info = completer, privateWithin = ttmap1.mapOwner(odenot.privateWithin), // since this refers to outer symbols, need not include copies (from->to) in ownermap here. annotations = odenot.annotations.mapConserve(ttmap1.apply)) + } + copies } From 3270404d2a6ea5fc96a3ba475e9a79fb977f3a32 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Mon, 19 Dec 2016 13:46:50 +0100 Subject: [PATCH 2/3] Add tests verifying that i1812 stays fixed. --- tests/pos/i1812.scala | 12 ++++++++++++ tests/pos/i1812b.scala | 11 +++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/pos/i1812.scala create mode 100644 tests/pos/i1812b.scala diff --git a/tests/pos/i1812.scala b/tests/pos/i1812.scala new file mode 100644 index 000000000000..653274883cf1 --- /dev/null +++ b/tests/pos/i1812.scala @@ -0,0 +1,12 @@ +class FF[R] { + def compose(): R = ??? +} + +class Test(x: Int) extends AnyVal { + def method: Unit = { + class Bla + class Foo extends FF[Bla] { + override def compose() = super[FF].compose() + } + } +} diff --git a/tests/pos/i1812b.scala b/tests/pos/i1812b.scala new file mode 100644 index 000000000000..492c545f154f --- /dev/null +++ b/tests/pos/i1812b.scala @@ -0,0 +1,11 @@ +class FF[R] { + def compose(): R = ??? +} + +class Test(x: Int) extends AnyVal { + def method: Unit = { + class Bla{ def bar:a.S = ???} + trait TRT{ type S} + val a: TRT = ??? + } +} From a7d17e286760879a35dfaeeadca1dbad2aa85dfc Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Mon, 19 Dec 2016 17:12:12 +0100 Subject: [PATCH 3/3] Fix #1812, Symbols.mapSymbols shouldn't force annotation transformation. Reasoning similar to one in the previous commit also applies to annotations. --- compiler/src/dotty/tools/dotc/core/Symbols.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index c475e99f533b..e0117300a9f8 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -328,6 +328,7 @@ trait Symbols { this: Context => // Note that this is a hack, but hack commonly used in Dotty // The same thing is done by other completers all the time denot.info = ttmap1.mapType(oinfo) + denot.annotations = odenot.annotations.mapConserve(ttmap1.apply) } } @@ -337,7 +338,7 @@ trait Symbols { this: Context => initFlags = odenot.flags &~ (Frozen | Touched) | Fresh, info = completer, privateWithin = ttmap1.mapOwner(odenot.privateWithin), // since this refers to outer symbols, need not include copies (from->to) in ownermap here. - annotations = odenot.annotations.mapConserve(ttmap1.apply)) + annotations = odenot.annotations) }