From 7d9771d983afdec4344d35642bcdf6adecd58a4b Mon Sep 17 00:00:00 2001
From: Jan Chyb <48855024+jchyb@users.noreply.github.com>
Date: Thu, 5 Dec 2024 13:01:08 +0100
Subject: [PATCH] Do not bring forward symbols created in transform and backend
 phases (#21865)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 compiler/src/dotty/tools/dotc/core/Denotations.scala   | 10 ++++------
 .../src/dotty/tools/dotc/core/SymDenotations.scala     |  4 ----
 tests/pos-macros/i21844/Macro.scala                    |  6 ++++++
 tests/pos-macros/i21844/SubClass.scala                 |  3 +++
 tests/pos-macros/i21844/SuperClassWithLazyVal.scala    |  2 ++
 5 files changed, 15 insertions(+), 10 deletions(-)
 create mode 100644 tests/pos-macros/i21844/Macro.scala
 create mode 100644 tests/pos-macros/i21844/SubClass.scala
 create mode 100644 tests/pos-macros/i21844/SuperClassWithLazyVal.scala

diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala
index 816b28177333..85ff51bc19de 100644
--- a/compiler/src/dotty/tools/dotc/core/Denotations.scala
+++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala
@@ -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.*
@@ -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 {
@@ -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
     }
diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
index f54b8a62fa25..e14e5cf0a728 100644
--- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -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)
diff --git a/tests/pos-macros/i21844/Macro.scala b/tests/pos-macros/i21844/Macro.scala
new file mode 100644
index 000000000000..31a2c3a5e76f
--- /dev/null
+++ b/tests/pos-macros/i21844/Macro.scala
@@ -0,0 +1,6 @@
+import scala.quoted.*
+
+object Macro:
+  inline def foo = ${ fooImpl }
+  def fooImpl(using Quotes): Expr[Int] =
+    '{ 123 }
diff --git a/tests/pos-macros/i21844/SubClass.scala b/tests/pos-macros/i21844/SubClass.scala
new file mode 100644
index 000000000000..54b0d970e515
--- /dev/null
+++ b/tests/pos-macros/i21844/SubClass.scala
@@ -0,0 +1,3 @@
+class SubClass extends SuperClass
+object SubClass:
+  val foo: Int = Macro.foo
diff --git a/tests/pos-macros/i21844/SuperClassWithLazyVal.scala b/tests/pos-macros/i21844/SuperClassWithLazyVal.scala
new file mode 100644
index 000000000000..60c270f2b277
--- /dev/null
+++ b/tests/pos-macros/i21844/SuperClassWithLazyVal.scala
@@ -0,0 +1,2 @@
+class SuperClass:
+  lazy val xyz: Int = 123