Detect macro dependencies that are missing from the classloader (scala#20139)

So the situation is basically that `DFiant` and `html.scala` projects do
not work "out of the box" with pipelining, and will need to tune their
builds if they want some pipelining.

However, the compiler reports an error that is not helpful to the user,
so in this PR we report a better one.

Previously, it was assumed that a missing class (that is valid in
current run)
during macro evaluation was due to the symbol being defined in the same
project.
If this condition is met, then compilation is suspended.

This assumption breaks when the symbol comes from the classpath, but
without a
corresponding class file, leading a situation where the same file is
always suspended,
until it is the only one left, leading to the "cyclic macro
dependencies" error.
In this case we should assume that the class file will never become
available because class path entries
are supposed to be immutable. Therefore we should not suspend in this
case.

This commit therefore detects this situation. Instead of suspending the
unit,
the compiler aborts the macro expansion, reporting an error that
the user will have to deal with - likely by changing the build
definition/

In the end, users will see a more actionable error.

Note that sbt already automatically disables pipelining on projects that
define macros, but this is not useful if the macro itself depends on
upstream projects that do not define macros. This is probably a hard
problem to detect automatically - so this is good compromise.

We also fix `-Xprint-suspension`, which appeared to swallow a lot of
diagnostic information.
Also make `-Yno-suspended-units` behave better.

fixes scala#20119

Author: bishabosha

compiler/src/dotty/tools/dotc/CompilationUnit.scala

@@ -90,22 +90,19 @@ class CompilationUnit protected (val source: SourceFile, val info: CompilationUn
   /** Suspends the compilation unit by thowing a SuspendException
    *  and recording the suspended compilation unit
    */
-  def suspend()(using Context): Nothing =
+  def suspend(hint: => String)(using Context): Nothing =
     assert(isSuspendable)
     // Clear references to symbols that may become stale. No need to call
     // `depRecorder.sendToZinc()` since all compilation phases will be rerun
     // when this unit is unsuspended.
     depRecorder.clear()
     if !suspended then
-      if ctx.settings.YnoSuspendedUnits.value then
-        report.error(i"Compilation unit suspended $this (-Yno-suspended-units is set)")
-      else
-        if (ctx.settings.XprintSuspension.value)
-          report.echo(i"suspended: $this")
-        suspended = true
-        ctx.run.nn.suspendedUnits += this
-        if ctx.phase == Phases.inliningPhase then
-          suspendedAtInliningPhase = true
+      suspended = true
+      ctx.run.nn.suspendedUnits += this
+      if ctx.settings.XprintSuspension.value then
+        ctx.run.nn.suspendedHints += (this -> hint)
+      if ctx.phase == Phases.inliningPhase then
+        suspendedAtInliningPhase = true
     throw CompilationUnit.SuspendException()
 
   private var myAssignmentSpans: Map[Int, List[Span]] | Null = null
@@ -123,7 +120,7 @@ class CompilationUnit protected (val source: SourceFile, val info: CompilationUn
 
   override def isJava: Boolean = false
 
-  override def suspend()(using Context): Nothing =
+  override def suspend(hint: => String)(using Context): Nothing =
     throw CompilationUnit.SuspendException()
 
   override def assignmentSpans(using Context): Map[Int, List[Span]] = Map.empty

compiler/src/dotty/tools/dotc/Driver.scala

@@ -52,7 +52,10 @@ class Driver {
     if !ctx.reporter.errorsReported && run.suspendedUnits.nonEmpty then
       val suspendedUnits = run.suspendedUnits.toList
       if (ctx.settings.XprintSuspension.value)
+        val suspendedHints = run.suspendedHints.toList
         report.echo(i"compiling suspended $suspendedUnits%, %")
+        for (unit, hint) <- suspendedHints do
+          report.echo(s"  $unit: $hint")
       val run1 = compiler.newRun
       run1.compileSuspendedUnits(suspendedUnits)
       finish(compiler, run1)(using MacroClassLoader.init(ctx.fresh))

compiler/src/dotty/tools/dotc/Run.scala

@@ -130,6 +130,7 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
     myUnits = us
 
   var suspendedUnits: mutable.ListBuffer[CompilationUnit] = mutable.ListBuffer()
+  var suspendedHints: mutable.Map[CompilationUnit, String] = mutable.HashMap()
 
   def checkSuspendedUnits(newUnits: List[CompilationUnit])(using Context): Unit =
     if newUnits.isEmpty && suspendedUnits.nonEmpty && !ctx.reporter.errorsReported then

compiler/src/dotty/tools/dotc/core/Symbols.scala

@@ -165,6 +165,10 @@ object Symbols extends SymUtils {
     final def isDefinedInSource(using Context): Boolean =
       span.exists && isValidInCurrentRun && associatedFileMatches(!_.isScalaBinary)
 
+    /** Is this symbol valid in the current run, but comes from the classpath? */
+    final def isDefinedInBinary(using Context): Boolean =
+      isValidInCurrentRun && associatedFileMatches(_.isScalaBinary)
+
     /** Is symbol valid in current run? */
     final def isValidInCurrentRun(using Context): Boolean =
       (lastDenot.validFor.runId == ctx.runId || stillValid(lastDenot)) &&

compiler/src/dotty/tools/dotc/inlines/Inliner.scala

@@ -1058,22 +1058,30 @@ class Inliner(val call: tpd.Tree)(using Context):
     }
   }
 
-  private def expandMacro(body: Tree, splicePos: SrcPos)(using Context) = {
+  private def expandMacro(body: Tree, splicePos: SrcPos)(using Context): Tree = {
     assert(level == 0)
     val inlinedFrom = enclosingInlineds.last
     val dependencies = macroDependencies(body)(using spliceContext)
     val suspendable = ctx.compilationUnit.isSuspendable
+    val printSuspensions = ctx.settings.XprintSuspension.value
     if dependencies.nonEmpty && !ctx.reporter.errorsReported then
+      val hints: mutable.ListBuffer[String] | Null =
+        if printSuspensions then mutable.ListBuffer.empty[String] else null
       for sym <- dependencies do
         if ctx.compilationUnit.source.file == sym.associatedFile then
           report.error(em"Cannot call macro $sym defined in the same source file", call.srcPos)
         else if ctx.settings.YnoSuspendedUnits.value then
           val addendum = ", suspension prevented by -Yno-suspended-units"
           report.error(em"Cannot call macro $sym defined in the same compilation run$addendum", call.srcPos)
-        if (suspendable && ctx.settings.XprintSuspension.value)
-          report.echo(i"suspension triggered by macro call to ${sym.showLocated} in ${sym.associatedFile}", call.srcPos)
+        if suspendable && printSuspensions then
+          hints.nn += i"suspension triggered by macro call to ${sym.showLocated} in ${sym.associatedFile}"
       if suspendable then
-        ctx.compilationUnit.suspend() // this throws a SuspendException
+        if ctx.settings.YnoSuspendedUnits.value then
+          return ref(defn.Predef_undefined)
+            .withType(ErrorType(em"could not expand macro, suspended units are disabled by -Yno-suspended-units"))
+            .withSpan(splicePos.span)
+        else
+          ctx.compilationUnit.suspend(hints.nn.toList.mkString(", ")) // this throws a SuspendException
 
     val evaluatedSplice = inContext(quoted.MacroExpansion.context(inlinedFrom)) {
       Splicer.splice(body, splicePos, inlinedFrom.srcPos, MacroClassLoader.fromContext)

compiler/src/dotty/tools/dotc/quoted/Interpreter.scala

@@ -166,8 +166,8 @@ class Interpreter(pos: SrcPos, classLoader0: ClassLoader)(using Context):
     val inst =
       try loadModule(moduleClass)
       catch
-        case MissingClassDefinedInCurrentRun(sym) =>
-          suspendOnMissing(sym, pos)
+        case MissingClassValidInCurrentRun(sym, origin) =>
+          suspendOnMissing(sym, origin, pos)
     val clazz = inst.getClass
     val name = fn.name.asTermName
     val method = getMethod(clazz, name, paramsSig(fn))
@@ -213,8 +213,8 @@ class Interpreter(pos: SrcPos, classLoader0: ClassLoader)(using Context):
   private def loadClass(name: String): Class[?] =
     try classLoader.loadClass(name)
     catch
-      case MissingClassDefinedInCurrentRun(sym) =>
-        suspendOnMissing(sym, pos)
+      case MissingClassValidInCurrentRun(sym, origin) =>
+        suspendOnMissing(sym, origin, pos)
 
 
   private def getMethod(clazz: Class[?], name: Name, paramClasses: List[Class[?]]): JLRMethod =
@@ -223,8 +223,8 @@ class Interpreter(pos: SrcPos, classLoader0: ClassLoader)(using Context):
       case _: NoSuchMethodException =>
         val msg = em"Could not find method ${clazz.getCanonicalName}.$name with parameters ($paramClasses%, %)"
         throw new StopInterpretation(msg, pos)
-      case MissingClassDefinedInCurrentRun(sym) =>
-        suspendOnMissing(sym, pos)
+      case MissingClassValidInCurrentRun(sym, origin) =>
+        suspendOnMissing(sym, origin, pos)
     }
 
   private def stopIfRuntimeException[T](thunk: => T, method: JLRMethod): T =
@@ -242,8 +242,8 @@ class Interpreter(pos: SrcPos, classLoader0: ClassLoader)(using Context):
         ex.getTargetException match {
           case ex: scala.quoted.runtime.StopMacroExpansion =>
             throw ex
-          case MissingClassDefinedInCurrentRun(sym) =>
-            suspendOnMissing(sym, pos)
+          case MissingClassValidInCurrentRun(sym, origin) =>
+            suspendOnMissing(sym, origin, pos)
           case targetException =>
             val sw = new StringWriter()
             sw.write("Exception occurred while executing macro expansion.\n")
@@ -348,8 +348,11 @@ object Interpreter:
     }
   end Call
 
-  object MissingClassDefinedInCurrentRun {
-    def unapply(targetException: Throwable)(using Context): Option[Symbol] = {
+  enum ClassOrigin:
+    case Classpath, Source
+
+  object MissingClassValidInCurrentRun {
+    def unapply(targetException: Throwable)(using Context): Option[(Symbol, ClassOrigin)] = {
       if !ctx.compilationUnit.isSuspendable then None
       else targetException match
         case _: NoClassDefFoundError | _: ClassNotFoundException =>
@@ -358,16 +361,34 @@ object Interpreter:
           else
             val className = message.replace('/', '.')
             val sym =
-              if className.endsWith(str.MODULE_SUFFIX) then staticRef(className.toTermName).symbol.moduleClass
-              else staticRef(className.toTypeName).symbol
-            // If the symbol does not a a position we assume that it came from the current run and it has an error
-            if sym.isDefinedInCurrentRun || (sym.exists && !sym.srcPos.span.exists) then Some(sym)
-            else None
+              if className.endsWith(str.MODULE_SUFFIX) then
+                staticRef(className.stripSuffix(str.MODULE_SUFFIX).toTermName).symbol.moduleClass
+              else
+                staticRef(className.toTypeName).symbol
+            if sym.isDefinedInBinary then
+              // i.e. the associated file is `.tasty`, if the macro classloader is not able to find the class,
+              // possibly it indicates that it comes from a pipeline-compiled dependency.
+              Some((sym, ClassOrigin.Classpath))
+            else if sym.isDefinedInCurrentRun || (sym.exists && !sym.srcPos.span.exists) then
+              // If the symbol does not a a position we assume that it came from the current run and it has an error
+              Some((sym, ClassOrigin.Source))
+            else
+              None
         case _ => None
     }
   }
 
-  def suspendOnMissing(sym: Symbol, pos: SrcPos)(using Context): Nothing =
-    if ctx.settings.XprintSuspension.value then
-      report.echo(i"suspension triggered by a dependency on $sym", pos)
-    ctx.compilationUnit.suspend() // this throws a SuspendException
+  def suspendOnMissing(sym: Symbol, origin: ClassOrigin, pos: SrcPos)(using Context): Nothing =
+    if origin == ClassOrigin.Classpath then
+      throw StopInterpretation(
+          em"""Macro code depends on ${sym.showLocated} found on the classpath, but could not be loaded while evaluating the macro.
+              |  This is likely because class files could not be found in the classpath entry for the symbol.
+              |
+              |  A possible cause is if the origin of this symbol was built with pipelined compilation;
+              |  in which case, this problem may go away by disabling pipelining for that origin.
+              |
+              |  $sym is defined in file ${sym.associatedFile}""", pos)
+    else if ctx.settings.YnoSuspendedUnits.value then
+      throw StopInterpretation(em"suspension triggered by a dependency on missing ${sym.showLocated} not allowed with -Yno-suspended-units", pos)
+    else
+      ctx.compilationUnit.suspend(i"suspension triggered by a dependency on missing ${sym.showLocated}") // this throws a SuspendException

compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala

@@ -107,8 +107,8 @@ class MacroAnnotations(phase: IdentityDenotTransformer):
             if !ctx.reporter.hasErrors then
               report.error("Macro expansion was aborted by the macro without any errors reported. Macros should issue errors to end-users when aborting a macro expansion with StopMacroExpansion.", annot.tree)
             List(tree)
-          case Interpreter.MissingClassDefinedInCurrentRun(sym) =>
-            Interpreter.suspendOnMissing(sym, annot.tree)
+          case Interpreter.MissingClassValidInCurrentRun(sym, origin) =>
+            Interpreter.suspendOnMissing(sym, origin, annot.tree)
           case NonFatal(ex) =>
             val stack0 = ex.getStackTrace.takeWhile(_.getClassName != "dotty.tools.dotc.transform.MacroAnnotations")
             val stack = stack0.take(1 + stack0.lastIndexWhere(_.getMethodName == "transform"))

compiler/src/dotty/tools/dotc/typer/Namer.scala

@@ -1678,7 +1678,7 @@ class Namer { typer: Typer =>
 
     final override def complete(denot: SymDenotation)(using Context): Unit =
       denot.resetFlag(Touched) // allow one more completion
-      ctx.compilationUnit.suspend()
+      ctx.compilationUnit.suspend(i"reset $denot")
   }
 
   /** Typecheck `tree` during completion using `typed`, and remember result in TypedAhead map */

sbt-test/pipelining/pipelining-scala-macro-splice-ok/build.sbt

@@ -0,0 +1,9 @@
+ThisBuild / usePipelining := true
+
+// m defines a macro depending on b.B, it also tries to use the macro in the same project,
+// which will succeed even though B.class is not available when running the macro,
+// because compilation can suspend until B is available.
+lazy val m = project.in(file("m"))
+  .settings(
+    scalacOptions += "-Ycheck:all",
+  )

sbt-test/pipelining/pipelining-scala-macro-splice-ok/m/src/main/scala/a/A.scala

@@ -0,0 +1,3 @@
+package a
+
+class A(val i: Int)

sbt-test/pipelining/pipelining-scala-macro-splice-ok/m/src/main/scala/b/B.scala

@@ -0,0 +1,26 @@
+package b
+
+import a.A
+import scala.quoted.*
+
+object B {
+
+  transparent inline def transparentPower(x: Double, inline n: Int): Double =
+    ${ powerCode('x, 'n)  }
+
+  def powerCode(x: Expr[Double], n: Expr[Int])(using Quotes): Expr[Double] = {
+    // this macro will cause a suspension in compilation of C.scala, because it calls
+    // transparentPower. This will try to invoke the macro but fail because A.class
+    // is not yet available until the run for A.scala completes.
+
+    // see sbt-test/pipelining/pipelining-scala-macro-splice/m/src/main/scala/b/B.scala
+    // for a corresponding implementation that uses a class from an upstream project
+    // instead, and fails because pipelining is turned on for the upstream project.
+    def impl(x: Double, n: A): Double =
+      if (n.i == 0) 1.0
+      else if (n.i % 2 == 1) x * impl(x, A(n.i - 1))
+      else impl(x * x, A(n.i / 2))
+
+    Expr(impl(x.valueOrError, A(n.valueOrError)))
+  }
+}

sbt-test/pipelining/pipelining-scala-macro-splice-ok/m/src/main/scala/c/C.scala

@@ -0,0 +1,11 @@
+package c
+
+import b.B
+
+object C {
+  @main def run = {
+    assert(B.transparentPower(2.0, 2) == 4.0)
+    assert(B.transparentPower(2.0, 3) == 8.0)
+    assert(B.transparentPower(2.0, 4) == 16.0)
+  }
+}

sbt-test/pipelining/pipelining-scala-macro-splice-ok/project/DottyInjectedPlugin.scala

@@ -0,0 +1,11 @@
+import sbt._
+import Keys._
+
+object DottyInjectedPlugin extends AutoPlugin {
+  override def requires = plugins.JvmPlugin
+  override def trigger = allRequirements
+
+  override val projectSettings = Seq(
+    scalaVersion := sys.props("plugin.scalaVersion"),
+  )
+}

sbt-test/pipelining/pipelining-scala-macro-splice-ok/test

@@ -0,0 +1,3 @@
+# shows that it is ok to depend on a class, defined in the same project,
+# in a macro implementation. Compilation will suspend at typer.
+> m/run

sbt-test/pipelining/pipelining-scala-macro-splice/a/src/main/scala/a/A.scala

@@ -0,0 +1,3 @@
+package a
+
+class A(val i: Int)

sbt-test/pipelining/pipelining-scala-macro-splice/a_alt/.keep

@@ -0,0 +1,32 @@
+ThisBuild / usePipelining := true
+
+lazy val a = project.in(file("a"))
+  .settings(
+    scalacOptions += "-Ycheck:all",
+  )
+
+// same as a, but does not use pipelining
+lazy val a_alt = project.in(file("a_alt"))
+  .settings(
+    Compile / sources := (a / Compile / sources).value,
+    Compile / exportPipelining := false,
+  )
+
+
+// m defines a macro depending on a, it also tries to use the macro in the same project,
+// which will fail because A.class is not available when running the macro,
+// because the dependency on a is pipelined.
+lazy val m = project.in(file("m"))
+  .dependsOn(a)
+  .settings(
+    scalacOptions += "-Ycheck:all",
+  )
+
+// same as m, but depends on a_alt, so it will compile
+// because A.class will be available when running the macro.
+lazy val m_alt = project.in(file("m_alt"))
+  .dependsOn(a_alt)
+  .settings(
+    Compile / sources := (m / Compile / sources).value,
+    scalacOptions += "-Ycheck:all",
+  )

sbt-test/pipelining/pipelining-scala-macro-splice/build.sbt

@@ -0,0 +1,26 @@
+package b
+
+import a.A
+import scala.quoted.*
+
+object B {
+
+  transparent inline def transparentPower(x: Double, inline n: Int): Double =
+    ${ powerCode('x, 'n)  }
+
+  def powerCode(x: Expr[Double], n: Expr[Int])(using Quotes): Expr[Double] = {
+    // this macro is invoked during compilation of C.scala. When project a is pipelined
+    // This will fail because A.class will never be available, because the classpath entry
+    // is the early-output jar. The compiler detects this and aborts macro expansion with an error.
+
+    // see sbt-test/pipelining/pipelining-scala-macro-splice-ok/m/src/main/scala/b/B.scala
+    // for a corresponding implementation that uses a class from the same project
+    // instead, but succeeds because it can suspend compilation until classes become available.
+    def impl(x: Double, n: A): Double =
+      if (n.i == 0) 1.0
+      else if (n.i % 2 == 1) x * impl(x, A(n.i - 1))
+      else impl(x * x, A(n.i / 2))
+
+    Expr(impl(x.valueOrError, A(n.valueOrError)))
+  }
+}

sbt-test/pipelining/pipelining-scala-macro-splice/m/src/main/scala/b/B.scala

@@ -0,0 +1,11 @@
+package c
+
+import b.B
+
+object C {
+  @main def run = {
+    assert(B.transparentPower(2.0, 2) == 4.0)
+    assert(B.transparentPower(2.0, 3) == 8.0)
+    assert(B.transparentPower(2.0, 4) == 16.0)
+  }
+}

sbt-test/pipelining/pipelining-scala-macro-splice/m/src/main/scala/c/C.scala

@@ -0,0 +1,11 @@
+import sbt._
+import Keys._
+
+object DottyInjectedPlugin extends AutoPlugin {
+  override def requires = plugins.JvmPlugin
+  override def trigger = allRequirements
+
+  override val projectSettings = Seq(
+    scalaVersion := sys.props("plugin.scalaVersion"),
+  )
+}

sbt-test/pipelining/pipelining-scala-macro-splice/m_alt/.keep

@@ -0,0 +1,10 @@
+# as described in build.sbt, this will fail to compile.
+# m defines a macro, depending on a.A, defined in upstream project a
+# however because m also tries to run the macro in the same project,
+# a/A.class is not available yet, so a reflection error will occur.
+# This is caught by the compiler and presents a pretty diagnostic to the user,
+# suggesting to disable pipelining in the project defining A.
+-> m/compile
+# This will run, simulating a user following the suggestion to
+# disable pipelining in project a.
+> m_alt/run