diff options
author | Martin Odersky <odersky@gmail.com> | 2015-04-23 15:15:20 +0200 |
---|---|---|
committer | Dmitry Petrashko <dmitry.petrashko@gmail.com> | 2015-04-30 14:26:05 +0200 |
commit | 979ee0ff41c2a957b22e886ea43ad4f070a9777d (patch) | |
tree | 8c0d3ff5f9a338d747547e69d4e5f291ec673dcb | |
parent | ebb3917fe33041586160fd1ff4b904ac65aeed5c (diff) | |
download | dotty-979ee0ff41c2a957b22e886ea43ad4f070a9777d.tar.gz dotty-979ee0ff41c2a957b22e886ea43ad4f070a9777d.tar.bz2 dotty-979ee0ff41c2a957b22e886ea43ad4f070a9777d.zip |
Fix changeOwnerAfter by adding transformDenotations method.
With the previous commit, we get a bad owner for the "typedArgs" var
in `dotc.typer.Applications`. What happens is the following (@DarkDimius
figured it out):
Here's the code in question:
val result = {
var typedArg = ...
(code that captures typedArg)
}
There's an interplay between 3 mini-phases, which run in interleaved succession in the same
group:
Memoize
CapturedVars
Constructors
The following events happen in the order they are written:
1. typedArg is noted to be captured, so prepareValDef in CapturedVars
installs a new denotation, valid after CapturedVars, with a Ref type.
2. Memoize in transformDefDef creates a field for `result` and changes
the owner of all definitions in the right-hand side to the field,
using `changeOwnerAfter`. This gives `typedArg` a new denotation
with the owner being the field `result$local` and a validity period
from Memoize + 1 to CapturedVars + 1 (because CapturedVars has already
installed a new denotation).
3. Constructors runs intoConstructor which changes the owner again. All code
with the field as current owner becomes owned by the constructor. But
unfortunately `typedArg` is owned by the getter `result`, because that's
the denotation installed by the preceding phase, CapturedVars. So its
owner stays the `getter` even though its definition is now part of the
constructor. Boom, -Ycheck fails.
The fix applied here adds a new method `transformAfter` which can transform
all future denotations of a symbol. `changeOwnerAfter` uses this method to become
insensitive to the order in which denotations are installed.
Morale: State is hard.
-rw-r--r-- | src/dotty/tools/dotc/ast/tpd.scala | 7 | ||||
-rw-r--r-- | src/dotty/tools/dotc/core/Denotations.scala | 36 | ||||
-rw-r--r-- | src/dotty/tools/dotc/core/SymDenotations.scala | 6 |
3 files changed, 39 insertions, 10 deletions
diff --git a/src/dotty/tools/dotc/ast/tpd.scala b/src/dotty/tools/dotc/ast/tpd.scala index 3bcfc2dda..56d916bdd 100644 --- a/src/dotty/tools/dotc/ast/tpd.scala +++ b/src/dotty/tools/dotc/ast/tpd.scala @@ -578,8 +578,11 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { def traverse(tree: Tree)(implicit ctx: Context) = tree match { case tree: DefTree => val sym = tree.symbol - if (sym.denot(ctx.withPhase(trans)).owner == from) - sym.copySymDenotation(owner = to).installAfter(trans) + if (sym.denot(ctx.withPhase(trans)).owner == from) { + val d = sym.copySymDenotation(owner = to) + d.installAfter(trans) + d.transformAfter(trans, d => if (d.owner eq from) d.copySymDenotation(owner = to) else d) + } if (sym.isWeakOwner) traverseChildren(tree) case _ => traverseChildren(tree) diff --git a/src/dotty/tools/dotc/core/Denotations.scala b/src/dotty/tools/dotc/core/Denotations.scala index a30cff714..aa1442769 100644 --- a/src/dotty/tools/dotc/core/Denotations.scala +++ b/src/dotty/tools/dotc/core/Denotations.scala @@ -620,14 +620,9 @@ object Denotations { // println(s"installing $this after $phase/${phase.id}, valid = ${current.validFor}") // printPeriods(current) this.validFor = Period(ctx.runId, targetId, current.validFor.lastPhaseId) - if (current.validFor.firstPhaseId == targetId) { - // replace current with this denotation - var prev = current - while (prev.nextInRun ne current) prev = prev.nextInRun - prev.nextInRun = this - this.nextInRun = current.nextInRun - current.validFor = Nowhere - } else { + if (current.validFor.firstPhaseId == targetId) + replaceDenotation(current) + else { // insert this denotation after current current.validFor = Period(ctx.runId, current.validFor.firstPhaseId, targetId - 1) this.nextInRun = current.nextInRun @@ -637,6 +632,31 @@ object Denotations { } } + /** Apply a transformation `f` to all denotations in this group that start at or after + * given phase. Denotations are replaced while keeping the same validity periods. + */ + protected def transformAfter(phase: DenotTransformer, f: SymDenotation => SymDenotation)(implicit ctx: Context): Unit = { + var current = symbol.current + while (current.validFor.firstPhaseId < phase.id && (current ne symbol.current)) + current = current.nextInRun + while (current.validFor.firstPhaseId >= phase.id) { + val current1: SingleDenotation = f(current.asSymDenotation) + if (current1 ne current) { + current1.validFor = current.validFor + current1.replaceDenotation(current) + } + current = current.nextInRun + } + } + + private def replaceDenotation(current: SingleDenotation): Unit = { + var prev = current + while (prev.nextInRun ne current) prev = prev.nextInRun + prev.nextInRun = this + this.nextInRun = current.nextInRun + current.validFor = Nowhere + } + def staleSymbolError(implicit ctx: Context) = { def ownerMsg = this match { case denot: SymDenotation => s"in ${denot.owner}" diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala index 83499ca7b..bcd46810e 100644 --- a/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1043,6 +1043,12 @@ object SymDenotations { /** Install this denotation as the result of the given denotation transformer. */ override def installAfter(phase: DenotTransformer)(implicit ctx: Context): Unit = super.installAfter(phase) + + /** Apply a transformation `f` to all denotations in this group that start at or after + * given phase. Denotations are replaced while keeping the same validity periods. + */ + override def transformAfter(phase: DenotTransformer, f: SymDenotation => SymDenotation)(implicit ctx: Context): Unit = + super.transformAfter(phase, f) } /** The contents of a class definition during a period |