Calls that rewritten with macros annotation are not applied in runtime

Question

Calls that rewritten with macros annotation are not applied in runtime

Opened this issue 8 months ago · 5 comments

Compiler version

3.3.5 (LTS)

Minimized code

I'm afraid there is no way to demonstrate the problem in one snippet or a file, so I created entire repo with reproducible code: https://github.com/NikKozh/error-example/tree/master

Tried to make it as small as possible.

I have described the short essence of the problem in repo's Readme, but also will duplicate it here:

I needed a macros that can rewrite some calls in every method of the annotated class. I made macro annotation for that in Scala 2 and it worked perfectly, but after migrating to Scala 3 I discovered that same macro algorithm doesn't work as expected.

Annotation rewrites methods correctly, I see that in build output. But in runtime app still using old methods, as if macros was never applied. However, I also added new def in macros, and override toString, just to check if macros even worked. And new toString totally was applied in runtime.

So I'm very confused - macros was partly applied? Am I doing something wrong, there is some macros limits in Scala 3 or it's actually a bug?

Build output

New body classDef:

List(@SourceFile("app/src/main/scala/org/error/app/Main.scala") @rewriteDefaultTransactorCalls @experimental class DbService[F[_$9]](database: Database[F], transactor: Transactor[F])(using evidence$1: Sync[F]) extends TransactorSyntax {
  def string$macro$1: String = "macros string"
  override def toString(): String = DbService.this.string$macro$1
  def doAction: F[Unit] = DbService.this.toDefaultTransactionOps[F, Unit](database.run[Unit](database.insert("ValueToInsert"))).transactWithMaster(transactor)
  def doRead: F[String] = DbService.this.toDefaultTransactionOps[F, String](database.run[String](database.read("SomeKey"))).transactWithReplicas(transactor)
})

Everything is ok, old methods was just .transact, now new transact methods are here: .transactWithMaster and .transactWithReplicas, and should be called accordingly in runtime.

Runtime output

ERROR! THERE IS SHOULDN'T BE DEFAULT TRANSACTION CALLS
ERROR! THERE IS SHOULDN'T BE DEFAULT TRANSACTION CALLS
service.toString: macros string
running transaction inside DB
inserting value ValueToInsert in DB
Transacted with master
running transaction inside DB
reading key SomeKey from DB
Transacted with master

But alas, there is output that shouldn't be here if methods would actually rewritten - two errors in the beginning and last "transacted with master" (should be replica). Nevertheless, .toString macro part is here, so macros changes was applied, but only partially. It seems like transact calls wasn't rewritten, despite build output.

Expectation

Runtime output:

service.toString: macros string
running transaction inside DB
inserting value ValueToInsert in DB
Transacted with master
running transaction inside DB
reading key SomeKey from DB
Transacted with replica

So the last "transaction" should be "in replica", according to rewrited transactWithReplicas call.

Answer 1 · 2025-03-06T12:28:55.000Z

@jchyb can you help to validate/diagnose this?

Answer 2 · 2025-03-06T13:52:27.000Z

I would like to add another example, which I believe relates to the same problem.

If we have a macro that add implicit val, which should affect existing code, it also doesn't do that. For example, part of the macro annotation, that rewrites every def in class this way:

given Quotes = methodTree.symbol.asQuotes
methodBody.asExpr match {
  case '{ $rhsExpr: t } =>
    val newMethodBody = '{
      @unused implicit val transactionModeImplicit: TransactionMode = TransactionMode.Replica;
      println("macros working");
      $rhsExpr
    }.asTerm
    DefDef.copy(methodTree)(methodName, params, returningType, Some(newMethodBody))
}

So if we have a def which contains a method call with implicit parameter TransactionMode, and somewhere in the scope we have fallback implicit with TransactionMode.Master, with this macros annotation we should have Replica mode, but implicit won't affect anything. But macros surely was applied, since I see text "macros working" in runtime.

Taking into account my first example, it seems like annotation macros just can't modify existing code in any way, only add something on top, or I just missing something.

(I can make another example repo with this particular problem as well, if needed)

Answer 3 · 2025-03-11T17:39:40.000Z

Hi @NikKozh,
When developing/designing macros, you always want to have -Xcheck-macro option added to the compiler options. This turns on the correctness checking for code generated by the macros (this is turned off by default as it causes a performance hit for checks which macro library users generally do not need).
WIth that said, turning -Xcheck-macros reveals an error which, to be honest, does not make much sense:

[error] -- Error: /Users/jchyb/workspace/error-example/app/src/main/scala/org/error/app/Main.scala:68:6 
[error] 67 |@experimental @rewriteDefaultTransactorCalls
[error] 68 |class DbService[F[_]: Sync](database: Database[F], transactor: Transactor[F]) extends TransactorSyntax {
[error]    |^
[error]    |Malformed tree was found while expanding macro with -Xcheck-macros.
[error]    |               |The tree does not conform to the compiler's tree invariants.
[error]    |               |
[error]    |               |Macro was:
[error]    |               |@scala.annotation.internal.SourceFile("app/src/main/scala/org/error/app/Main.scala") @org.error.macros.TransactorMacros.rewriteDefaultTransactorCalls @scala.annotation.experimental class DbService[F[_$9]](database: org.error.app.Database[F], transactor: org.error.app.Transactor[F])(using evidence$1: cats.effect.Sync[F]) extends org.error.app.TransactorSyntax {
[error]    |  def doAction: DbService.this.F[scala.Unit] = DbService.this.toDefaultTransactionOps[DbService.this.F, scala.Unit](DbService.this.database.run[scala.Unit](DbService.this.database.insert("ValueToInsert"))).transact(DbService.this.transactor)
[error]    |  def doRead: DbService.this.F[scala.Predef.String] = DbService.this.toDefaultTransactionOps[DbService.this.F, java.lang.String](DbService.this.database.run[java.lang.String](DbService.this.database.read("SomeKey"))).transact(DbService.this.transactor)
[error]    |}
[error]    |               |
[error]    |               |The macro returned:
[error]    |               |@scala.annotation.internal.SourceFile("app/src/main/scala/org/error/app/Main.scala") @org.error.macros.TransactorMacros.rewriteDefaultTransactorCalls @scala.annotation.experimental class DbService[F[_$9]](database: org.error.app.Database[F], transactor: org.error.app.Transactor[F])(using evidence$1: cats.effect.Sync[F]) extends org.error.app.TransactorSyntax {
[error]    |  def string$macro$1: java.lang.String = "macros string"
[error]    |  override def toString(): java.lang.String = DbService.this.string$macro$1
[error]    |  def doAction: DbService.this.F[scala.Unit] = DbService.this.toDefaultTransactionOps[DbService.this.F, scala.Unit](DbService.this.database.run[scala.Unit](DbService.this.database.insert("ValueToInsert"))).transactWithMaster(DbService.this.transactor)
[error]    |  def doRead: DbService.this.F[scala.Predef.String] = DbService.this.toDefaultTransactionOps[DbService.this.F, java.lang.String](DbService.this.database.run[java.lang.String](DbService.this.database.read("SomeKey"))).transactWithReplicas(DbService.this.transactor)
[error]    |}
[error]    |               |
[error]    |               |Error:
[error]    |               |assertion failed: symbols differ for this.toDefaultTransactionOps[F, Unit](
[error]    |  DbService.this.database.run[Unit](
[error]    |    DbService.this.database.insert("ValueToInsert"))
[error]    |).transact
[error]    |was                 : method transact
[error]    |alternatives by type: method transactWithMaster of types (xa: org.error.app.Transactor[F]): F[A]
[error]    |qualifier type      : DbService.this.DefaultTransactionOps[F, Unit]
[error]    |tree type           : (DbService.this.DefaultTransactionOps[F, Unit]#transact :
[error]    |  (xa: org.error.app.Transactor[F]): F[Unit]) of class class dotty.tools.dotc.core.Types$CachedTermRef
[error]    |               |
[error]    |stacktrace available when compiling with `-Ydebug`
[error]    |               |
[error] 69 |  def doAction: F[Unit] =
[error] 70 |    database.run(database.insert("ValueToInsert")).transact(transactor)
[error] 71 |  def doRead: F[String] =
[error] 72 |    database.run(database.read("SomeKey")).transact(transactor)
[error] 73 |}

Again, even as a compiler developer this does not make much sense (and should be improved), but it did cause me to dig a little deeper and I eventually found out that the Select.copy(tree)(transformTerm(qualifier)(owner), resultTransactMethod) could create a Select pointing to .transactWithReplicas which does not exist. Removing the s fixes the compilation error and outputs the expected code (again, the above error does not makes sense, especially as it does not even mention "transactWithReplica"). We should improve the error reporting here (at the very least Select.copy should have a similar check about the existence of selected symbol as Symbol.unique, Symbol.overloaded, etc. have). As for why it silently switches to the old implementation in case of errorneus tree - I don't know yet, still investigating.

Also, the is probably a leftover from the scala-2 implementation, but when using scala-3s macros we do not need that scala-reflect library dependency

Answer 4 · 2025-03-11T17:45:23.000Z

About the implicit val, the code you get inside of the macro in the form of a reflect.Tree is actually already typechecked and has all of the needed implicits applied, so you have to rewrite those yourself

Answer 5 · 2025-03-13T05:47:09.000Z

Hello @jchyb,
thank you very much for the response, really appreciate your help!
I am quite new to the macros and didn't know about -Xcheck-macro option. Following your instructions, I have added it, fixed the "s" typo in method call rewriting, fully cleaned project and tried to compile, but unfortunately got same error:

[error] -- Error: /home/nkozhevnikov/macros-error-example/error-example/app/src/main/scala/org/error/app/Main.scala:68:6 
[error] 67 |@experimental @rewriteDefaultTransactorCalls
[error] 68 |class DbService[F[_]: Sync](database: Database[F], transactor: Transactor[F]) extends TransactorSyntax {
[error]    |^
[error]    |Malformed tree was found while expanding macro with -Xcheck-macros.
[error]    |               |The tree does not conform to the compiler's tree invariants.
[error]    |               |
[error]    |               |Macro was:
[error]    |               |@scala.annotation.internal.SourceFile("app/src/main/scala/org/error/app/Main.scala") @org.error.macros.TransactorMacros.rewriteDefaultTransactorCalls @scala.annotation.experimental class DbService[F[_$9]](database: org.error.app.Database[F], transactor: org.error.app.Transactor[F])(using evidence$1: cats.effect.Sync[F]) extends org.error.app.TransactorSyntax {
[error]    |  def doAction: DbService.this.F[scala.Unit] = DbService.this.toDefaultTransactionOps[DbService.this.F, scala.Unit](DbService.this.database.run[scala.Unit](DbService.this.database.insert("ValueToInsert"))).transact(DbService.this.transactor)
[error]    |  def doRead: DbService.this.F[scala.Predef.String] = DbService.this.toDefaultTransactionOps[DbService.this.F, java.lang.String](DbService.this.database.run[java.lang.String](DbService.this.database.read("SomeKey"))).transact(DbService.this.transactor)
[error]    |}
[error]    |               |
[error]    |               |The macro returned:
[error]    |               |@scala.annotation.internal.SourceFile("app/src/main/scala/org/error/app/Main.scala") @org.error.macros.TransactorMacros.rewriteDefaultTransactorCalls @scala.annotation.experimental class DbService[F[_$9]](database: org.error.app.Database[F], transactor: org.error.app.Transactor[F])(using evidence$1: cats.effect.Sync[F]) extends org.error.app.TransactorSyntax {
[error]    |  def string$macro$1: java.lang.String = "macros string"
[error]    |  override def toString(): java.lang.String = DbService.this.string$macro$1
[error]    |  def doAction: DbService.this.F[scala.Unit] = DbService.this.toDefaultTransactionOps[DbService.this.F, scala.Unit](DbService.this.database.run[scala.Unit](DbService.this.database.insert("ValueToInsert"))).transactWithMaster(DbService.this.transactor)
[error]    |  def doRead: DbService.this.F[scala.Predef.String] = DbService.this.toDefaultTransactionOps[DbService.this.F, java.lang.String](DbService.this.database.run[java.lang.String](DbService.this.database.read("SomeKey"))).transactWithReplica(DbService.this.transactor)
[error]    |}
[error]    |               |
[error]    |               |Error:
[error]    |               |assertion failed: symbols differ for this.toDefaultTransactionOps[F, Unit](
[error]    |  DbService.this.database.run[Unit](
[error]    |    DbService.this.database.insert("ValueToInsert"))
[error]    |).transact
[error]    |was                 : method transact
[error]    |alternatives by type: method transactWithMaster of types (xa: org.error.app.Transactor[F]): F[A]
[error]    |qualifier type      : DbService.this.DefaultTransactionOps[F, Unit]
[error]    |tree type           : (DbService.this.DefaultTransactionOps[F, Unit]#transact :
[error]    |  (xa: org.error.app.Transactor[F]): F[Unit]) of class class dotty.tools.dotc.core.Types$CachedTermRef
[error]    |               |
[error]    |stacktrace available when compiling with `-Ydebug`
[error]    |               |
[error] 69 |  def doAction: F[Unit] =
[error] 70 |    database.run(database.insert("ValueToInsert")).transact(transactor)
[error] 71 |  def doRead: F[String] =
[error] 72 |    database.run(database.read("SomeKey")).transact(transactor)
[error] 73 |}

As you can see, there is now correct call transactWithReplica, but something else still wrong. Is fixing method name the only thing I needed to do for solving this problem? Not sure how to dig forward with this error.