Skip to content

Commit de4d420

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 704726f commit de4d420

File tree

3 files changed

+39
-10
lines changed

3 files changed

+39
-10
lines changed

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

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

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

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -618,14 +618,9 @@ object Denotations {
618618
// println(s"installing $this after $phase/${phase.id}, valid = ${current.validFor}")
619619
// printPeriods(current)
620620
this.validFor = Period(ctx.runId, targetId, current.validFor.lastPhaseId)
621-
if (current.validFor.firstPhaseId == targetId) {
622-
// replace current with this denotation
623-
var prev = current
624-
while (prev.nextInRun ne current) prev = prev.nextInRun
625-
prev.nextInRun = this
626-
this.nextInRun = current.nextInRun
627-
current.validFor = Nowhere
628-
} else {
621+
if (current.validFor.firstPhaseId == targetId)
622+
replaceDenotation(current)
623+
else {
629624
// insert this denotation after current
630625
current.validFor = Period(ctx.runId, current.validFor.firstPhaseId, targetId - 1)
631626
this.nextInRun = current.nextInRun
@@ -635,6 +630,31 @@ object Denotations {
635630
}
636631
}
637632

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

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,12 @@ object SymDenotations {
10391039
/** Install this denotation as the result of the given denotation transformer. */
10401040
override def installAfter(phase: DenotTransformer)(implicit ctx: Context): Unit =
10411041
super.installAfter(phase)
1042+
1043+
/** Apply a transformation `f` to all denotations in this group that start at or after
1044+
* given phase. Denotations are replaced while keeping the same validity periods.
1045+
*/
1046+
override def transformAfter(phase: DenotTransformer, f: SymDenotation => SymDenotation)(implicit ctx: Context): Unit =
1047+
super.transformAfter(phase, f)
10421048
}
10431049

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

0 commit comments

Comments
 (0)