From 371ffef1a12b91710181e3f0be73e8f61310a7d7 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 13 Dec 2022 09:08:26 +0100 Subject: [PATCH] Handle macro annotation suspends and crashes --- .../dotty/tools/dotc/quoted/Interpreter.scala | 60 +++++++++---------- .../dotc/transform/MacroAnnotations.scala | 30 +++++++++- .../dotty/tools/dotc/transform/Splicer.scala | 2 +- tests/neg-macros/annot-crash.check | 8 +++ tests/neg-macros/annot-crash/Macro_1.scala | 8 +++ tests/neg-macros/annot-crash/Test_2.scala | 2 + tests/neg-macros/annot-ill-abort.check | 5 ++ .../neg-macros/annot-ill-abort/Macro_1.scala | 8 +++ tests/neg-macros/annot-ill-abort/Test_2.scala | 2 + tests/neg-macros/annot-suspend-cycle.check | 12 ++++ .../annot-suspend-cycle/Macro.scala | 9 +++ .../neg-macros/annot-suspend-cycle/Test.scala | 5 ++ tests/neg-macros/ill-abort.check | 2 +- 13 files changed, 119 insertions(+), 34 deletions(-) create mode 100644 tests/neg-macros/annot-crash.check create mode 100644 tests/neg-macros/annot-crash/Macro_1.scala create mode 100644 tests/neg-macros/annot-crash/Test_2.scala create mode 100644 tests/neg-macros/annot-ill-abort.check create mode 100644 tests/neg-macros/annot-ill-abort/Macro_1.scala create mode 100644 tests/neg-macros/annot-ill-abort/Test_2.scala create mode 100644 tests/neg-macros/annot-suspend-cycle.check create mode 100644 tests/neg-macros/annot-suspend-cycle/Macro.scala create mode 100644 tests/neg-macros/annot-suspend-cycle/Test.scala diff --git a/compiler/src/dotty/tools/dotc/quoted/Interpreter.scala b/compiler/src/dotty/tools/dotc/quoted/Interpreter.scala index 5833e3b8dc9f..38cecb7953b8 100644 --- a/compiler/src/dotty/tools/dotc/quoted/Interpreter.scala +++ b/compiler/src/dotty/tools/dotc/quoted/Interpreter.scala @@ -166,10 +166,8 @@ class Interpreter(pos: SrcPos, classLoader: ClassLoader)(using Context): (inst, inst.getClass) } catch - case MissingClassDefinedInCurrentRun(sym) if ctx.compilationUnit.isSuspendable => - if (ctx.settings.XprintSuspension.value) - report.echo(i"suspension triggered by a dependency on $sym", pos) - ctx.compilationUnit.suspend() // this throws a SuspendException + case MissingClassDefinedInCurrentRun(sym) => + suspendOnMissing(sym, pos) val name = fn.name.asTermName val method = getMethod(clazz, name, paramsSig(fn)) @@ -214,12 +212,10 @@ class Interpreter(pos: SrcPos, classLoader: ClassLoader)(using Context): private def loadClass(name: String): Class[?] = try classLoader.loadClass(name) - catch { - case MissingClassDefinedInCurrentRun(sym) if ctx.compilationUnit.isSuspendable => - if (ctx.settings.XprintSuspension.value) - report.echo(i"suspension triggered by a dependency on $sym", pos) - ctx.compilationUnit.suspend() // this throws a SuspendException - } + catch + case MissingClassDefinedInCurrentRun(sym) => + suspendOnMissing(sym, pos) + private def getMethod(clazz: Class[?], name: Name, paramClasses: List[Class[?]]): JLRMethod = try clazz.getMethod(name.toString, paramClasses: _*) @@ -227,10 +223,8 @@ class Interpreter(pos: SrcPos, classLoader: 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) if ctx.compilationUnit.isSuspendable => - if (ctx.settings.XprintSuspension.value) - report.echo(i"suspension triggered by a dependency on $sym", pos) - ctx.compilationUnit.suspend() // this throws a SuspendException + case MissingClassDefinedInCurrentRun(sym) => + suspendOnMissing(sym, pos) } private def stopIfRuntimeException[T](thunk: => T, method: JLRMethod): T = @@ -248,10 +242,8 @@ class Interpreter(pos: SrcPos, classLoader: ClassLoader)(using Context): ex.getTargetException match { case ex: scala.quoted.runtime.StopMacroExpansion => throw ex - case MissingClassDefinedInCurrentRun(sym) if ctx.compilationUnit.isSuspendable => - if (ctx.settings.XprintSuspension.value) - report.echo(i"suspension triggered by a dependency on $sym", pos) - ctx.compilationUnit.suspend() // this throws a SuspendException + case MissingClassDefinedInCurrentRun(sym) => + suspendOnMissing(sym, pos) case targetException => val sw = new StringWriter() sw.write("Exception occurred while executing macro expansion.\n") @@ -268,19 +260,6 @@ class Interpreter(pos: SrcPos, classLoader: ClassLoader)(using Context): } } - private object MissingClassDefinedInCurrentRun { - def unapply(targetException: Throwable)(using Context): Option[Symbol] = { - targetException match - case _: NoClassDefFoundError | _: ClassNotFoundException => - val className = targetException.getMessage - if className eq null then None - else - val sym = staticRef(className.toTypeName).symbol - if (sym.isDefinedInCurrentRun) Some(sym) else None - case _ => None - } - } - /** List of classes of the parameters of the signature of `sym` */ private def paramsSig(sym: Symbol): List[Class[?]] = { def paramClass(param: Type): Class[?] = { @@ -364,3 +343,22 @@ object Interpreter: } } end Call + + object MissingClassDefinedInCurrentRun { + def unapply(targetException: Throwable)(using Context): Option[Symbol] = { + if !ctx.compilationUnit.isSuspendable then None + else targetException match + case _: NoClassDefFoundError | _: ClassNotFoundException => + val className = targetException.getMessage + if className eq null then None + else + val sym = staticRef(className.toTypeName).symbol + if (sym.isDefinedInCurrentRun) Some(sym) 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 diff --git a/compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala b/compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala index 44bc11e90602..076b9df79917 100644 --- a/compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala +++ b/compiler/src/dotty/tools/dotc/transform/MacroAnnotations.scala @@ -13,11 +13,15 @@ import dotty.tools.dotc.core.DenotTransformers.DenotTransformer import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.MacroClassLoader import dotty.tools.dotc.core.Symbols.* +import dotty.tools.dotc.core.Types._ import dotty.tools.dotc.quoted.* import dotty.tools.dotc.util.SrcPos import scala.quoted.runtime.impl.{QuotesImpl, SpliceScope} import scala.quoted.Quotes +import scala.util.control.NonFatal + +import java.lang.reflect.InvocationTargetException class MacroAnnotations(thisPhase: DenotTransformer): import tpd.* @@ -53,7 +57,31 @@ class MacroAnnotations(thisPhase: DenotTransformer): debug.println(i"Expanding macro annotation: ${annot}") // Interpret call to `new myAnnot(..).transform(using )()` - val transformedTrees = callMacro(macroInterpreter, tree, annot) + val transformedTrees = + try callMacro(macroInterpreter, tree, annot) + catch + // TODO: Replace this case when scala.annaotaion.MacroAnnotation is no longer experimental and reflectiveSelectable is not used + // Replace this case with the nested cases. + case ex0: InvocationTargetException => + ex0.getCause match + case ex: scala.quoted.runtime.StopMacroExpansion => + 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 NonFatal(ex) => + val stack0 = ex.getStackTrace.takeWhile(_.getClassName != "dotty.tools.dotc.transform.MacroAnnotations") + val stack = stack0.take(1 + stack0.lastIndexWhere(_.getMethodName == "transform")) + val msg = + em"""Failed to evaluate macro. + | Caused by ${ex.getClass}: ${if (ex.getMessage == null) "" else ex.getMessage} + | ${stack.mkString("\n ")} + |""" + report.error(msg, annot.tree) + List(tree) + case _ => + throw ex0 transformedTrees.span(_.symbol != tree.symbol) match case (prefixed, newTree :: suffixed) => allTrees ++= prefixed diff --git a/compiler/src/dotty/tools/dotc/transform/Splicer.scala b/compiler/src/dotty/tools/dotc/transform/Splicer.scala index 761a19d122a3..b936afb73dc8 100644 --- a/compiler/src/dotty/tools/dotc/transform/Splicer.scala +++ b/compiler/src/dotty/tools/dotc/transform/Splicer.scala @@ -69,7 +69,7 @@ object Splicer { throw ex case ex: scala.quoted.runtime.StopMacroExpansion => 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 to facilitate debugging when aborting a macro expansion.", splicePos) + 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.", splicePos) // errors have been emitted EmptyTree case ex: StopInterpretation => diff --git a/tests/neg-macros/annot-crash.check b/tests/neg-macros/annot-crash.check new file mode 100644 index 000000000000..16eb0f68bc44 --- /dev/null +++ b/tests/neg-macros/annot-crash.check @@ -0,0 +1,8 @@ + +-- Error: tests/neg-macros/annot-crash/Test_2.scala:1:0 ---------------------------------------------------------------- +1 |@crash // error + |^^^^^^ + |Failed to evaluate macro. + | Caused by class scala.NotImplementedError: an implementation is missing + | scala.Predef$.$qmark$qmark$qmark(Predef.scala:344) + | crash.transform(Macro_1.scala:7) diff --git a/tests/neg-macros/annot-crash/Macro_1.scala b/tests/neg-macros/annot-crash/Macro_1.scala new file mode 100644 index 000000000000..f3d5b3f602f8 --- /dev/null +++ b/tests/neg-macros/annot-crash/Macro_1.scala @@ -0,0 +1,8 @@ +import scala.annotation.{experimental, MacroAnnotation} +import scala.quoted._ + +@experimental +class crash extends MacroAnnotation { + def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] = + ??? +} diff --git a/tests/neg-macros/annot-crash/Test_2.scala b/tests/neg-macros/annot-crash/Test_2.scala new file mode 100644 index 000000000000..3e8fd3cf785f --- /dev/null +++ b/tests/neg-macros/annot-crash/Test_2.scala @@ -0,0 +1,2 @@ +@crash // error +def test = () diff --git a/tests/neg-macros/annot-ill-abort.check b/tests/neg-macros/annot-ill-abort.check new file mode 100644 index 000000000000..b969b3ad4313 --- /dev/null +++ b/tests/neg-macros/annot-ill-abort.check @@ -0,0 +1,5 @@ + +-- Error: tests/neg-macros/annot-ill-abort/Test_2.scala:1:0 ------------------------------------------------------------ +1 |@crash // 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. diff --git a/tests/neg-macros/annot-ill-abort/Macro_1.scala b/tests/neg-macros/annot-ill-abort/Macro_1.scala new file mode 100644 index 000000000000..446ce0a5331b --- /dev/null +++ b/tests/neg-macros/annot-ill-abort/Macro_1.scala @@ -0,0 +1,8 @@ +import scala.annotation.{experimental, MacroAnnotation} +import scala.quoted._ + +@experimental +class crash extends MacroAnnotation { + def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] = + throw new scala.quoted.runtime.StopMacroExpansion +} diff --git a/tests/neg-macros/annot-ill-abort/Test_2.scala b/tests/neg-macros/annot-ill-abort/Test_2.scala new file mode 100644 index 000000000000..3e8fd3cf785f --- /dev/null +++ b/tests/neg-macros/annot-ill-abort/Test_2.scala @@ -0,0 +1,2 @@ +@crash // error +def test = () diff --git a/tests/neg-macros/annot-suspend-cycle.check b/tests/neg-macros/annot-suspend-cycle.check new file mode 100644 index 000000000000..237cbe4188b2 --- /dev/null +++ b/tests/neg-macros/annot-suspend-cycle.check @@ -0,0 +1,12 @@ +-- [E129] Potential Issue Warning: tests/neg-macros/annot-suspend-cycle/Macro.scala:7:4 -------------------------------- +7 | new Foo + | ^^^^^^^ + | A pure expression does nothing in statement position; you may be omitting necessary parentheses + | + | longer explanation available when compiling with `-explain` +Cyclic macro dependencies in tests/neg-macros/annot-suspend-cycle/Test.scala. +Compilation stopped since no further progress can be made. + +To fix this, place macros in one set of files and their callers in another. + +Compiling with -Xprint-suspension gives more information. diff --git a/tests/neg-macros/annot-suspend-cycle/Macro.scala b/tests/neg-macros/annot-suspend-cycle/Macro.scala new file mode 100644 index 000000000000..4143e2c32062 --- /dev/null +++ b/tests/neg-macros/annot-suspend-cycle/Macro.scala @@ -0,0 +1,9 @@ +import scala.annotation.{experimental, MacroAnnotation} +import scala.quoted._ + +@experimental +class cycle extends MacroAnnotation { + def transform(using Quotes)(tree: quotes.reflect.Definition): List[quotes.reflect.Definition] = + new Foo + List(tree) +} diff --git a/tests/neg-macros/annot-suspend-cycle/Test.scala b/tests/neg-macros/annot-suspend-cycle/Test.scala new file mode 100644 index 000000000000..c1e1289742c1 --- /dev/null +++ b/tests/neg-macros/annot-suspend-cycle/Test.scala @@ -0,0 +1,5 @@ +// nopos-error +class Foo + +@cycle +def test = () diff --git a/tests/neg-macros/ill-abort.check b/tests/neg-macros/ill-abort.check index 2f76c89d88dd..c267c2e79ecf 100644 --- a/tests/neg-macros/ill-abort.check +++ b/tests/neg-macros/ill-abort.check @@ -2,7 +2,7 @@ -- Error: tests/neg-macros/ill-abort/quoted_2.scala:1:15 --------------------------------------------------------------- 1 |def test = fail() // error | ^^^^^^ - |Macro expansion was aborted by the macro without any errors reported. Macros should issue errors to end-users to facilitate debugging when aborting a macro expansion. + |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. |--------------------------------------------------------------------------------------------------------------------- |Inline stack trace |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -