Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #20471: owners of top-level symbols in cached quoted code being incorrect #21945

Merged
merged 1 commit into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,9 @@ object PickledQuotes {
treeOwner(tree) match
case Some(owner) =>
// Copy the cached tree to make sure the all definitions are unique.
TreeTypeMap(oldOwners = List(owner), newOwners = List(owner)).apply(tree)
val treeCpy = TreeTypeMap(oldOwners = List(owner), newOwners = List(owner)).apply(tree)
// Then replace the symbol owner with the one pointed by the quote context.
treeCpy.changeNonLocalOwners(ctx.owner)
Comment on lines +244 to +246
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious why not propose this change, I've tried it locally with the test for #20471 but the following test (and only one) tests/run-staging/quote-ackermann-1.scala fails.

Suggested change
val treeCpy = TreeTypeMap(oldOwners = List(owner), newOwners = List(owner)).apply(tree)
// Then replace the symbol owner with the one pointed by the quote context.
treeCpy.changeNonLocalOwners(ctx.owner)
TreeTypeMap(oldOwners = List(owner), newOwners = List(ctx.owner)).apply(tree)
// Then replace the symbol owner with the one pointed by the quote context.
//treeCpy.changeNonLocalOwners(ctx.owner)

case _ =>
tree

Expand Down
63 changes: 63 additions & 0 deletions tests/pos-macros/i20471/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import scala.annotation.experimental
import scala.quoted.*
import scala.annotation.tailrec

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]
}

@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
}
}

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]]
7 changes: 7 additions & 0 deletions tests/pos-macros/i20471/Main_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
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")
Loading