Symbol.asQuotes doesn't work as advertised
Opened this issue · 4 comments
Compiler version
3.4.2
Minimized code
CI: https://github.com/typelevel/cats-tagless/actions/runs/9236004309/job/25411405213?pr=547
Here is my change:
body = {
case (method, tpe, body) if tpe =:= B =>
+ given Quotes = method.asQuotes
'{
@tailrec def step(x: A): B =
${ body.replace(a, '{ x }).asExprOf[Either[A, B]] } match
case Left(a) => step(a)
case Right(b) => b
step($a)
- }.asTerm.changeOwner(method)
+ }.asTerm
The test which fails to compile:
trait TestAlgebra[T] derives FlatMap:
def abstractEffect(a: String): T
def concreteEffect(a: String): T = abstractEffect(a + " concreteEffect")
def abstractOther(a: String): String
def concreteOther(a: String): String = a + " concreteOther"
def withoutParams: T
Output
[error] -- Error: /home/runner/work/cats-tagless/cats-tagless/tests/src/test/scala-3/cats/tagless/tests/autoFlatMapTests.scala:39:31
[error] 39 | trait TestAlgebra[T] derives FlatMap:
[error] | ^
[error] |Exception occurred while executing macro expansion.
[error] |java.lang.AssertionError: assertion failed: Tree had an unexpected owner for method step
[error] |Expected: method withoutParams (cats.tagless.tests.autoFlatMapTests$.TestAlgebra$._$_$$anon._$$anon$macro$3.withoutParams)
[error] |But was: method concreteEffect (cats.tagless.tests.autoFlatMapTests$.TestAlgebra$._$_$$anon._$$anon$macro$3.concreteEffect)
[error] |
[error] |The code of the definition of method step is
[error] |@scala.annotation.tailrec def step(x: A): B = f.apply(x).withoutParams match {
[error] | case scala.Left(a) =>
[error] | step(a)
[error] | case scala.Right(b) =>
[error] | (b: B)
[error] |}
[error] |
[error] |which was found in the code
[error] |{
[error] | @scala.annotation.tailrec def step(x: A): B = f.apply(x).withoutParams match {
[error] | case scala.Left(a) =>
[error] | step(a)
[error] | case scala.Right(b) =>
[error] | (b: B)
[error] | }
[error] | step(`a₂`)
[error] |}
[error] |
[error] |which has the AST representation
[error] |Inlined(Some(TypeIdent("MacroFlatMap$")), Nil, Block(List(DefDef("step", List(TermParamClause(List(ValDef("x", Inferred(), None)))), Inferred(), Some(Match(Inlined(None, Nil, Select(Inlined(Some(TypeIdent("MacroFlatMap$")), Nil, Apply(Select(Inlined(None, Nil, Ident("f")), "apply"), List(Inlined(None, Nil, Ident("x"))))), "withoutParams")), List(CaseDef(TypedOrTest(Unapply(TypeApply(Select(Ident("Left"), "unapply"), List(Inferred(), Inferred())), Nil, List(Bind("a", Wildcard()))), Inferred()), None, Block(Nil, Apply(Ident("step"), List(Ident("a"))))), CaseDef(TypedOrTest(Unapply(TypeApply(Select(Ident("Right"), "unapply"), List(Inferred(), Inferred())), Nil, List(Bind("b", Wildcard()))), Inferred()), None, Block(Nil, Typed(Ident("b"), Inferred())))))))), Apply(Ident("step"), List(Inlined(None, Nil, Ident("a"))))))
[error] |
[error] |
[error] |
[error] |Tip: The owner of a tree can be changed using method `Tree.changeOwner`.
[error] |Tip: The default owner of definitions created in quotes can be changed using method `Symbol.asQuotes`.
Expectation
I expect it to compile the same way it does if I use changeOwner
. It's even one of the tips:
Tip: The default owner of definitions created in quotes can be changed using method
Symbol.asQuotes
Hi @joroKr21! After printing the trees in cats-tagless I see everything behaving correctly:
(...)
[info] final lazy given val q2: scala.quoted.Quotes =
[info] given_DeriveMacros_q_type.q.reflect.SymbolMethods.
[info] asQuotes(method)
[info] q.reflect.asTerm(
[info] '{
[info] {
[info] @tailrec def step(x: A): B =
[info] ${
[info] {
[info] def $anonfun(using
[info] evidence$8: scala.quoted.Quotes):
[info] scala.quoted.Expr[Either[A, B]] =
[info] given_DeriveMacros_q_type.q.reflect.
[info] TreeMethods.asExprOf(
[info] given_DeriveMacros_q_type.replace(body)(
[info] a, '{x}.apply(evidence$8))
[info] )[Either[A, B]](
[info] scala.quoted.Type.of[Either[A, B]](
[info] evidence$8)
[info] )
[info] closure($anonfun)
[info] }
[info] } match
[info] {
[info] case Left.unapply[A, B](a @ _):Left[A, B] =>
[info] step(a)
[info] case Right.unapply[A, B](b @ _):Right[A, B]
[info] =>
[info] b:B
[info] }
[info] step(
[info] ${
[info] {
[info] def $anonfun(using
[info] evidence$9: scala.quoted.Quotes):
[info] scala.quoted.Expr[A] = a
[info] closure($anonfun)
[info] }
[info] }
[info] )
[info] }
[info] }.apply(q2)
(...)
(q2 is the Quotes object applied to the splice, which is what we want)
The issue you are having stems from changeOwner
and given Quotes
behaving slightly differently by design.
Supplying correct given Quotes
guarantees that every symbol defined as part of the quoted code block has its owner set to a pointed symbol.
Meanwhile changeOwner(symbol)
goes through the whole tree and replaces every nonlocal owner with symbol
.
With the first method we still have to set the correct owners inside splices, with the second those will likely be corrected.
So in our case, while the def step
does have a correct owner set, the stuff returned by DeriveMacros.replace does not. The (correct) Quotes object supplied by the splice is ignored, and instead the previous quotes object passed to DeriveMacros is used:
def replace(from: Expr[?], to: Expr[?]): Term =
ReplaceTerm(from.asTerm, to.asTerm).transformTerm(term)(Symbol.spliceOwner)
You could rewrite the replace method to something like
def replace(from: Expr[?], to: Expr[?])(using quotes: Quotes): quotes.reflect.Term =
import quotes.reflect._
ReplaceTerm(from.asTerm, to.asTerm).transformTerm(term)(Symbol.spliceOwner)
to use the correct quotes object supplied by the splice (it will not compile yet, but the point is to use a correct symbol).
Hmm I will try it but that sounds strange, because the error is explicitly about the owner of step
:
[error] |java.lang.AssertionError: assertion failed: Tree had an unexpected owner for method step
[error] |Expected: method withoutParams (cats.tagless.tests.autoFlatMapTests$.TestAlgebra$._$_$$anon._$$anon$macro$3.withoutParams)
[error] |But was: method concreteEffect (cats.tagless.tests.autoFlatMapTests$.TestAlgebra$._$_$$anon._$$anon$macro$3.concreteEffect)
For context, body.replace(a, '{ x })
doesn't introduce any definitions.
Ah, you are right, I now replaced step
implementation with ???
and those errors are still thrown. I will have to investigate this further. Thank you for reporting!
Self-contained minimisation:
Macro_1.scala
import scala.annotation.experimental
import scala.quoted.*
import scala.annotation.tailrec
@experimental
object MacroFlatMap:
inline def derive[F[_]]: FlatMap[F] = ${ flatMap }
def flatMap[F[_]: Type](using Quotes): Expr[FlatMap[F]] = '{
new FlatMap[F]:
def tailRecM[A, B](a: A)(f: A => F[Either[A, B]]): F[B] =
${ deriveTailRecM('{ a }, '{ f }) }
}
def deriveTailRecM[F[_]: Type, A: Type, B: Type](
a: Expr[A],
f: Expr[A => F[Either[A, B]]]
)(using q: Quotes): Expr[F[B]] =
import quotes.reflect.*
val body: PartialFunction[(Symbol, TypeRepr), Term] = {
case (method, tpe) => {
given q2: Quotes = method.asQuotes
'{
def step(x: A): B = ???
???
}.asTerm//.changeOwner(method)
}
}
val term = '{ $f($a) }.asTerm
val name = Symbol.freshName("$anon")
val parents = List(TypeTree.of[Object], TypeTree.of[F[B]])
extension (sym: Symbol) def overridableMembers: List[Symbol] =
val member1 = sym.methodMember("abstractEffect")(0)
val member2 = sym.methodMember("concreteEffect")(0)
def meth(member: Symbol) = Symbol.newMethod(sym, member.name, This(sym).tpe.memberType(member), Flags.Override, Symbol.noSymbol)
List(meth(member1), meth(member2))
val cls = Symbol.newClass(Symbol.spliceOwner, name, parents.map(_.tpe), _.overridableMembers, None)
def transformDef(method: DefDef)(argss: List[List[Tree]]): Option[Term] =
val sym = method.symbol
Some(body.apply((sym, method.returnTpt.tpe)))
val members = cls.declarations
.filterNot(_.isClassConstructor)
.map: sym =>
sym.tree match
case method: DefDef => DefDef(sym, transformDef(method))
case _ => report.errorAndAbort(s"Not supported: $sym in ${sym.owner}")
val newCls = New(TypeIdent(cls)).select(cls.primaryConstructor).appliedToNone
Block(ClassDef(cls, parents, members) :: Nil, newCls).asExprOf[F[B]]
Test_2.scala
import scala.annotation.experimental
@experimental
object autoFlatMapTests:
trait TestAlgebra[T] derives FlatMap:
def abstractEffect(a: String): T
def concreteEffect(a: String): T = abstractEffect(a + " concreteEffect")
object FlatMap {
@experimental inline def derived[F[_]]: FlatMap[F] = MacroFlatMap.derive
}
trait FlatMap[F[_]]{
def tailRecM[A, B](a: A)(f: A => F[Either[A, B]]): F[B]
}