From c8764bac0f555b2d3c9f42403d1ae216960d28ad Mon Sep 17 00:00:00 2001
From: Guillaume Martres <smarter@ubuntu.com>
Date: Tue, 14 May 2024 22:23:10 +0200
Subject: [PATCH] Avoid forcing whole package when using `-experimental`

In https://github.com/scala/scala3/pull/19807, the behavior of `-experimental`
was changed to mark all top-level definitions as experimental. To do so, the
implementation traverses the whole package and checks every symbol to see if it
should be transformed or not. The problem was that the first check we do is
`sym.isExperimental` which ends up forcing the symbol.

Besides being a performance issue, this could also lead to a crash if the
current package is the empty package, because we could end up forcing the magic
`module-info.class` that Java modules place there. For some reason, this appear
to only happen when building with sbt, hence the additional scripted test.

This PR fixes this issue by reordering the checks (and adding a preliminary
`isDefinedInCurrentRun` check for good measure). We should also investigate
whether we can avoid creating a symbol for `module-info.class`, but this PR is
intentionally minimal so we can backport it to 3.5.0-RC2 without risks.
---
 compiler/src/dotty/tools/dotc/typer/Checking.scala | 3 ++-
 sbt-test/java-compat/moduleInfo/A.scala            | 2 ++
 sbt-test/java-compat/moduleInfo/build.sbt          | 5 +++++
 sbt-test/java-compat/moduleInfo/test               | 1 +
 4 files changed, 10 insertions(+), 1 deletion(-)
 create mode 100644 sbt-test/java-compat/moduleInfo/A.scala
 create mode 100644 sbt-test/java-compat/moduleInfo/build.sbt
 create mode 100644 sbt-test/java-compat/moduleInfo/test

diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala
index 073055ba5b58..1f82b9ddc084 100644
--- a/compiler/src/dotty/tools/dotc/typer/Checking.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala
@@ -806,10 +806,11 @@ object Checking {
   def checkAndAdaptExperimentalImports(trees: List[Tree])(using Context): Unit =
     def nonExperimentalTopLevelDefs(pack: Symbol): Iterator[Symbol] =
       def isNonExperimentalTopLevelDefinition(sym: Symbol) =
-        !sym.isExperimental
+        sym.isDefinedInCurrentRun
         && sym.source == ctx.compilationUnit.source
         && !sym.isConstructor // not constructor of package object
         && !sym.is(Package) && !sym.name.isPackageObjectName
+        && !sym.isExperimental
 
       pack.info.decls.toList.iterator.flatMap: sym =>
         if sym.isClass && (sym.is(Package) || sym.isPackageObject) then
diff --git a/sbt-test/java-compat/moduleInfo/A.scala b/sbt-test/java-compat/moduleInfo/A.scala
new file mode 100644
index 000000000000..4b46ae7047d6
--- /dev/null
+++ b/sbt-test/java-compat/moduleInfo/A.scala
@@ -0,0 +1,2 @@
+// Previously, we crashed trying to parse module-info.class in the empty package.
+class A
diff --git a/sbt-test/java-compat/moduleInfo/build.sbt b/sbt-test/java-compat/moduleInfo/build.sbt
new file mode 100644
index 000000000000..a0308b6cb83a
--- /dev/null
+++ b/sbt-test/java-compat/moduleInfo/build.sbt
@@ -0,0 +1,5 @@
+scalaVersion := sys.props("plugin.scalaVersion")
+
+scalacOptions ++= Seq(
+  "-experimental"
+)
diff --git a/sbt-test/java-compat/moduleInfo/test b/sbt-test/java-compat/moduleInfo/test
new file mode 100644
index 000000000000..5df2af1f3956
--- /dev/null
+++ b/sbt-test/java-compat/moduleInfo/test
@@ -0,0 +1 @@
+> compile