Skip to content

Commit 979ee0f

Browse files
oderskyDarkDimius
authored andcommitted
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.
1 parent ebb3917 commit 979ee0f

File tree

3 files changed

+39
-10
lines changed

3 files changed

+39
-10
lines changed

src/dotty/tools/dotc/ast/tpd.scala

+5-2
Original file line numberDiff line numberDiff line change
@@ -578,8 +578,11 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
578578
def traverse(tree: Tree)(implicit ctx: Context) = tree match {
579579
case tree: DefTree =>
580580
val sym = tree.symbol
581-
if (sym.denot(ctx.withPhase(trans)).owner == from)
582-
sym.copySymDenotation(owner = to).installAfter(trans)
581+
if (sym.denot(ctx.withPhase(trans)).owner == from) {
582+
val d = sym.copySymDenotation(owner = to)
583+
d.installAfter(trans)
584+
d.transformAfter(trans, d => if (d.owner eq from) d.copySymDenotation(owner = to) else d)
585+
}
583586
if (sym.isWeakOwner) traverseChildren(tree)
584587
case _ =>
585588
traverseChildren(tree)

src/dotty/tools/dotc/core/Denotations.scala

+28-8
Original file line numberDiff line numberDiff line change
@@ -620,14 +620,9 @@ object Denotations {
620620
// println(s"installing $this after $phase/${phase.id}, valid = ${current.validFor}")
621621
// printPeriods(current)
622622
this.validFor = Period(ctx.runId, targetId, current.validFor.lastPhaseId)
623-
if (current.validFor.firstPhaseId == targetId) {
624-
// replace current with this denotation
625-
var prev = current
626-
while (prev.nextInRun ne current) prev = prev.nextInRun
627-
prev.nextInRun = this
628-
this.nextInRun = current.nextInRun
629-
current.validFor = Nowhere
630-
} else {
623+
if (current.validFor.firstPhaseId == targetId)
624+
replaceDenotation(current)
625+
else {
631626
// insert this denotation after current
632627
current.validFor = Period(ctx.runId, current.validFor.firstPhaseId, targetId - 1)
633628
this.nextInRun = current.nextInRun
@@ -637,6 +632,31 @@ object Denotations {
637632
}
638633
}
639634

635+
/** Apply a transformation `f` to all denotations in this group that start at or after
636+
* given phase. Denotations are replaced while keeping the same validity periods.
637+
*/
638+
protected def transformAfter(phase: DenotTransformer, f: SymDenotation => SymDenotation)(implicit ctx: Context): Unit = {
639+
var current = symbol.current
640+
while (current.validFor.firstPhaseId < phase.id && (current ne symbol.current))
641+
current = current.nextInRun
642+
while (current.validFor.firstPhaseId >= phase.id) {
643+
val current1: SingleDenotation = f(current.asSymDenotation)
644+
if (current1 ne current) {
645+
current1.validFor = current.validFor
646+
current1.replaceDenotation(current)
647+
}
648+
current = current.nextInRun
649+
}
650+
}
651+
652+
private def replaceDenotation(current: SingleDenotation): Unit = {
653+
var prev = current
654+
while (prev.nextInRun ne current) prev = prev.nextInRun
655+
prev.nextInRun = this
656+
this.nextInRun = current.nextInRun
657+
current.validFor = Nowhere
658+
}
659+
640660
def staleSymbolError(implicit ctx: Context) = {
641661
def ownerMsg = this match {
642662
case denot: SymDenotation => s"in ${denot.owner}"

src/dotty/tools/dotc/core/SymDenotations.scala

+6
Original file line numberDiff line numberDiff line change
@@ -1043,6 +1043,12 @@ object SymDenotations {
10431043
/** Install this denotation as the result of the given denotation transformer. */
10441044
override def installAfter(phase: DenotTransformer)(implicit ctx: Context): Unit =
10451045
super.installAfter(phase)
1046+
1047+
/** Apply a transformation `f` to all denotations in this group that start at or after
1048+
* given phase. Denotations are replaced while keeping the same validity periods.
1049+
*/
1050+
override def transformAfter(phase: DenotTransformer, f: SymDenotation => SymDenotation)(implicit ctx: Context): Unit =
1051+
super.transformAfter(phase, f)
10461052
}
10471053

10481054
/** The contents of a class definition during a period

0 commit comments

Comments
 (0)