From 5648f12fb136886da4e1f00695627cbfde90ee9e Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 5 Jan 2024 11:07:25 +0100 Subject: [PATCH 1/2] Only evaluate transparent inline unapply once Fixes #19369 --- .../dotty/tools/dotc/inlines/Inlines.scala | 12 +++--- .../dotty/tools/dotc/typer/Applications.scala | 39 ++++++++++++++++++- .../src/dotty/tools/dotc/typer/Typer.scala | 4 +- tests/pos-macros/i19369/Macro_1.scala | 11 ++++++ tests/pos-macros/i19369/Test_2.scala | 2 + tests/run/i8530.check | 2 + tests/run/i8530.scala | 12 ++++++ 7 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 tests/pos-macros/i19369/Macro_1.scala create mode 100644 tests/pos-macros/i19369/Test_2.scala diff --git a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala b/compiler/src/dotty/tools/dotc/inlines/Inlines.scala index 4860913bdc63..8757f6029189 100644 --- a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala +++ b/compiler/src/dotty/tools/dotc/inlines/Inlines.scala @@ -172,10 +172,10 @@ object Inlines: /** Try to inline a pattern with an inline unapply method. Fail with error if the maximal * inline depth is exceeded. * - * @param unapp The tree of the pattern to inline + * @param fun The function of an Unapply node * @return An `Unapply` with a `fun` containing the inlined call to the unapply */ - def inlinedUnapply(unapp: tpd.UnApply)(using Context): Tree = + def inlinedUnapplyFun(fun: tpd.Tree)(using Context): Tree = // We cannot inline the unapply directly, since the pattern matcher relies on unapply applications // as signposts what to do. On the other hand, we can do the inlining only in typer, not afterwards. // So the trick is to create a "wrapper" unapply in an anonymous class that has the inlined unapply @@ -189,7 +189,6 @@ object Inlines: // transforms the patterns into terms, the `inlinePatterns` phase removes this anonymous class by β-reducing // the call to the `unapply`. - val fun = unapp.fun val sym = fun.symbol val newUnapply = AnonClass(ctx.owner, List(defn.ObjectType), sym.coord) { cls => @@ -199,7 +198,7 @@ object Inlines: val unapplyInfo = fun.tpe.widen val unapplySym = newSymbol(cls, sym.name.toTermName, Synthetic | Method, unapplyInfo, coord = sym.coord).entered - val unapply = DefDef(unapplySym.asTerm, argss => fun.appliedToArgss(argss).withSpan(unapp.span)) + val unapply = DefDef(unapplySym.asTerm, argss => fun.appliedToArgss(argss).withSpan(fun.span)) if sym.is(Transparent) then // Inline the body and refine the type of the unapply method @@ -214,9 +213,8 @@ object Inlines: List(unapply) } - val newFun = newUnapply.select(sym.name).withSpan(unapp.span) - cpy.UnApply(unapp)(fun = newFun) - end inlinedUnapply + newUnapply.select(sym.name).withSpan(fun.span) + end inlinedUnapplyFun /** For a retained inline method, another method that keeps track of * the body that is kept at runtime. For instance, an inline method diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 004b21ce4fb5..b48222d48dc5 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -34,6 +34,7 @@ import Denotations.SingleDenotation import annotation.threadUnsafe import scala.util.control.NonFatal +import dotty.tools.dotc.inlines.Inlines object Applications { import tpd.* @@ -1408,6 +1409,36 @@ trait Applications extends Compatibility { } } + /** Inlines the unapply function before the dummy argument + * + * A call `P.unapply[...](using l1, ..)(`dummy`)(using t1, ..)` becomes + * ``` + * { + * class $anon { + * def unapply(s: S)(using t1: T1, ..): R = + * ... // inlined code for: P.unapply[...](using l1, ..)(s)(using t1, ..) + * } + * new $anon + * }.unapply(`dummy`)(using t1, ..) + * ``` + */ + def inlinedUnapplyFnAndApp(dummyArg: Tree, unapplyAppCall: Tree): (Tree, Tree) = + def rec(unapp: Tree): (Tree, Tree) = + unapp match + case DynamicUnapply(_) => + report.error(em"Structural unapply is not supported", unapplyFn.srcPos) + (unapplyFn, unapplyAppCall) + case Apply(fn, `dummyArg` :: Nil) => + val inlinedUnapplyFn = Inlines.inlinedUnapplyFun(fn) + (inlinedUnapplyFn, inlinedUnapplyFn.appliedToArgs(`dummyArg` :: Nil)) + case Apply(fn, args) => + val (fn1, app) = rec(fn) + (fn1, tpd.cpy.Apply(unapp)(app, args)) + + if unapplyAppCall.symbol.isAllOf(Transparent | Inline) then rec(unapplyAppCall) + else (unapplyFn, unapplyAppCall) + end inlinedUnapplyFnAndApp + /** Add a `Bind` node for each `bound` symbol in a type application `unapp` */ def addBinders(unapp: Tree, bound: List[Symbol]) = unapp match { case TypeApply(fn, args) => @@ -1446,7 +1477,11 @@ trait Applications extends Compatibility { unapplyArgType val dummyArg = dummyTreeOfType(ownType) - val unapplyApp = typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil))) + val (newUnapplyFn, unapplyApp) = + val unapplyAppCall = withMode(Mode.NoInline): + typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil))) + inlinedUnapplyFnAndApp(dummyArg, unapplyAppCall) + def unapplyImplicits(unapp: Tree): List[Tree] = { val res = List.newBuilder[Tree] def loop(unapp: Tree): Unit = unapp match { @@ -1475,7 +1510,7 @@ trait Applications extends Compatibility { List.fill(argTypes.length - args.length)(WildcardType) } val unapplyPatterns = bunchedArgs.lazyZip(argTypes) map (typed(_, _)) - val result = assignType(cpy.UnApply(tree)(unapplyFn, unapplyImplicits(unapplyApp), unapplyPatterns), ownType) + val result = assignType(cpy.UnApply(tree)(newUnapplyFn, unapplyImplicits(unapplyApp), unapplyPatterns), ownType) unapp.println(s"unapply patterns = $unapplyPatterns") if (ownType.stripped eq selType.stripped) || ownType.isError then result else tryWithTypeTest(Typed(result, TypeTree(ownType)), selType) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 1303b64cbd12..bdaca93045e3 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1938,7 +1938,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer if (bounds != null) sym.info = bounds } b - case t: UnApply if t.symbol.is(Inline) => Inlines.inlinedUnapply(t) + case t: UnApply if t.symbol.is(Inline) => + assert(!t.symbol.is(Transparent)) + cpy.UnApply(t)(fun = Inlines.inlinedUnapplyFun(t.fun)) // TODO inline these in the inlining phase (see #19382) case t => t } } diff --git a/tests/pos-macros/i19369/Macro_1.scala b/tests/pos-macros/i19369/Macro_1.scala new file mode 100644 index 000000000000..151859d6e583 --- /dev/null +++ b/tests/pos-macros/i19369/Macro_1.scala @@ -0,0 +1,11 @@ +import scala.quoted._ + +object Unapplier: + transparent inline def unapply(arg: Any): Option[Int] = ${unapplyImpl('arg)} + + def unapplyImpl(using Quotes)(argExpr: Expr[Any]): Expr[Option[Int]] = + executionCount += 1 + assert(executionCount == 1, "macro should only expand once") + '{Some(1)} + + private var executionCount = 0 diff --git a/tests/pos-macros/i19369/Test_2.scala b/tests/pos-macros/i19369/Test_2.scala new file mode 100644 index 000000000000..8d697ff8f09e --- /dev/null +++ b/tests/pos-macros/i19369/Test_2.scala @@ -0,0 +1,2 @@ +@main def main() = + val Unapplier(result) = Some(5) diff --git a/tests/run/i8530.check b/tests/run/i8530.check index c8f75a704ff7..34749b91c683 100644 --- a/tests/run/i8530.check +++ b/tests/run/i8530.check @@ -3,3 +3,5 @@ MyBoooleanUnapply 3 (4,5) 5 +6 +7 diff --git a/tests/run/i8530.scala b/tests/run/i8530.scala index bbbc52587ee0..2ce24dc8e7ce 100644 --- a/tests/run/i8530.scala +++ b/tests/run/i8530.scala @@ -13,6 +13,12 @@ object MySeqUnapply: object MyWhiteboxUnapply: transparent inline def unapply(x: Int): Option[Any] = Some(x) +object MyWhiteboxUnapply1: + transparent inline def unapply(using DummyImplicit)(x: Int)(using DummyImplicit): Option[Any] = Some(x) + +object MyWhiteboxUnapply2: + transparent inline def unapply(using DummyImplicit)(using DummyImplicit)(x: Int)(using DummyImplicit)(using DummyImplicit): Option[Any] = Some(x) + @main def Test = 1 match @@ -30,4 +36,10 @@ object MyWhiteboxUnapply: 5 match case MyWhiteboxUnapply(x) => println(x: Int) + 6 match + case MyWhiteboxUnapply1(x) => println(x: Int) + + 7 match + case MyWhiteboxUnapply2(x) => println(x: Int) + end Test From 535f2306d6848ccd374755467230d557a09f4f36 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 18 Jan 2024 10:22:46 +0100 Subject: [PATCH 2/2] Move `unapplyImplicits` to the same level as other helpers in `typedUnapply` --- .../dotty/tools/dotc/typer/Applications.scala | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index b48222d48dc5..452c6d197310 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1439,6 +1439,20 @@ trait Applications extends Compatibility { else (unapplyFn, unapplyAppCall) end inlinedUnapplyFnAndApp + def unapplyImplicits(dummyArg: Tree, unapp: Tree): List[Tree] = + val res = List.newBuilder[Tree] + def loop(unapp: Tree): Unit = unapp match + case Apply(Apply(unapply, `dummyArg` :: Nil), args2) => assert(args2.nonEmpty); res ++= args2 + case Apply(unapply, `dummyArg` :: Nil) => + case Inlined(u, _, _) => loop(u) + case DynamicUnapply(_) => report.error(em"Structural unapply is not supported", unapplyFn.srcPos) + case Apply(fn, args) => assert(args.nonEmpty); loop(fn); res ++= args + case _ => ().assertingErrorsReported + + loop(unapp) + res.result() + end unapplyImplicits + /** Add a `Bind` node for each `bound` symbol in a type application `unapp` */ def addBinders(unapp: Tree, bound: List[Symbol]) = unapp match { case TypeApply(fn, args) => @@ -1482,20 +1496,6 @@ trait Applications extends Compatibility { typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil))) inlinedUnapplyFnAndApp(dummyArg, unapplyAppCall) - def unapplyImplicits(unapp: Tree): List[Tree] = { - val res = List.newBuilder[Tree] - def loop(unapp: Tree): Unit = unapp match { - case Apply(Apply(unapply, `dummyArg` :: Nil), args2) => assert(args2.nonEmpty); res ++= args2 - case Apply(unapply, `dummyArg` :: Nil) => - case Inlined(u, _, _) => loop(u) - case DynamicUnapply(_) => report.error(em"Structural unapply is not supported", unapplyFn.srcPos) - case Apply(fn, args) => assert(args.nonEmpty); loop(fn); res ++= args - case _ => ().assertingErrorsReported - } - loop(unapp) - res.result() - } - var argTypes = unapplyArgs(unapplyApp.tpe, unapplyFn, args, tree.srcPos) for (argType <- argTypes) assert(!isBounds(argType), unapplyApp.tpe.show) val bunchedArgs = argTypes match { @@ -1510,7 +1510,7 @@ trait Applications extends Compatibility { List.fill(argTypes.length - args.length)(WildcardType) } val unapplyPatterns = bunchedArgs.lazyZip(argTypes) map (typed(_, _)) - val result = assignType(cpy.UnApply(tree)(newUnapplyFn, unapplyImplicits(unapplyApp), unapplyPatterns), ownType) + val result = assignType(cpy.UnApply(tree)(newUnapplyFn, unapplyImplicits(dummyArg, unapplyApp), unapplyPatterns), ownType) unapp.println(s"unapply patterns = $unapplyPatterns") if (ownType.stripped eq selType.stripped) || ownType.isError then result else tryWithTypeTest(Typed(result, TypeTree(ownType)), selType)