Skip to content

Commit

Permalink
Do not bring forward symbols created in transform and backend phases (s…
Browse files Browse the repository at this point in the history
…cala#21865)

In the i21844 issue minimization, first the `Macro.scala` and
`SuperClassWIthLazyVal.scala` compilation units are compiled, then in
the next run `SubClass.scala` is compiled. While compiling
`SuperClassWIthLazyVal.scala`, in the `LazyVals` transform phase, the
lzyINIT initialization fields are created. In the next run, while
compiling `SubClass.scala`, in the `GenBCode` phase, the compiler would
try to access the lzyINIT symbol, leading to a stale symbols crash.
While that symbol was a part of the SuperClass, it by design is not
generated for the Subclass - if we were to completely split those files
into 2 separate compilations, that symbol would be created only for the
classfile, but it would not be included in tasty, so the second
compilation would not try to access it.

This observation inspires the proposed fix, where if the symbol was
first created in or after the first transform phase (so after the
pickler phases), we do not try to bring forward this denotation, instead
returning NoDenotation.

In this PR, we also remove the fix proposed in i21559, as it is no
longer necessary with the newly added condition. There, since one of the
problematic Symbols created in `LazyVals` was moved elsewhere in
`MoveStatics`, we checked if that symbol could be found in a companion
object. I was not able to create any reproduction where a user defined
static would produce this problem, so I believe it’s safe to remove
that.
  • Loading branch information
jchyb authored Dec 5, 2024
1 parent eb575a8 commit 7d9771d
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 10 deletions.
10 changes: 4 additions & 6 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package dotty.tools
package dotc
package core

import SymDenotations.{ SymDenotation, ClassDenotation, NoDenotation, LazyType, stillValid, movedToCompanionClass, acceptStale, traceInvalid }
import SymDenotations.{ SymDenotation, ClassDenotation, NoDenotation, LazyType, stillValid, acceptStale, traceInvalid }
import Contexts.*
import Names.*
import NameKinds.*
Expand Down Expand Up @@ -742,6 +742,8 @@ object Denotations {
* the old version otherwise.
* - If the symbol did not have a denotation that was defined at the current phase
* return a NoDenotation instead.
* - If the symbol was first defined in one of the transform phases (after pickling), it should not
* be visible in new runs, so also return a NoDenotation.
*/
private def bringForward()(using Context): SingleDenotation = {
this match {
Expand All @@ -755,11 +757,7 @@ object Denotations {
}
if (!symbol.exists) return updateValidity()
if (!coveredInterval.containsPhaseId(ctx.phaseId)) return NoDenotation
// Moved to a companion class, likely at a later phase (in MoveStatics)
this match {
case symd: SymDenotation if movedToCompanionClass(symd) => return NoDenotation
case _ =>
}
if (coveredInterval.firstPhaseId >= Phases.firstTransformPhase.id) return NoDenotation
if (ctx.debug) traceInvalid(this)
staleSymbolError
}
Expand Down
4 changes: 0 additions & 4 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2680,10 +2680,6 @@ object SymDenotations {
stillValidInOwner(denot)
}

def movedToCompanionClass(denot: SymDenotation)(using Context): Boolean =
val ownerCompanion = denot.maybeOwner.companionClass
stillValid(ownerCompanion) && ownerCompanion.unforcedDecls.contains(denot.name, denot.symbol)

private[SymDenotations] def stillValidInOwner(denot: SymDenotation)(using Context): Boolean = try
val owner = denot.maybeOwner.denot
stillValid(owner)
Expand Down
6 changes: 6 additions & 0 deletions tests/pos-macros/i21844/Macro.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import scala.quoted.*

object Macro:
inline def foo = ${ fooImpl }
def fooImpl(using Quotes): Expr[Int] =
'{ 123 }
3 changes: 3 additions & 0 deletions tests/pos-macros/i21844/SubClass.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class SubClass extends SuperClass
object SubClass:
val foo: Int = Macro.foo
2 changes: 2 additions & 0 deletions tests/pos-macros/i21844/SuperClassWithLazyVal.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class SuperClass:
lazy val xyz: Int = 123

0 comments on commit 7d9771d

Please sign in to comment.