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
WIP: Migrate quasiquote macros to Scala 3 #3347
base: main
Are you sure you want to change the base?
Conversation
I tried to come up with a way to publish just the parsers module, but that still has a bunch of errors when using fastparse and also there is a small macro that we would need to rewrite 🤔 https://github.com/scalameta/scalameta/compare/main...tgodzik:scalameta:cross-publish?expand=1 If successful it would be possible to just depend on |
Ok it seems I managed to progress further with the above branch. I also merged two PRs the reduce the number of errors when cross compiling. I am a bit stuck at adding the AstInfo macro, if you could help, we could merge that first and then quasiquotes. I am currently stuck at:
If anyone knows why this would pop up I would be super grateful. Otherwise, we might need to keep the macro in trees and only add the Scala 3 macro to the new macro module (though not sure if that would work) Let's talk after you are back from holidays |
I decided to instead remove the problematic AstInfo Macros and we should be able to cross compile part of Scalameta soon https://github.com/scalameta/scalameta/compare/main...tgodzik:scalameta:cross-publish2?expand=1 <- works already with parsers module |
b065913
to
44d1d72
Compare
Progress update:
|
Thanks for the great work! I will have to go over it and ask some questions since it's something I haven't dealt with yet. I almost finished with making scalameta release without quasiquotes for Scala 3, I need to tidy it up and figure out some smaller sbt issues.
So only Boolean ones were working previously? There were no tests for that?
Looks like we should wait for 3.3.2 for that? It would be super useful to have the correct types here without a need for casts. I think we can wait for the next release since it should be soon.
Which feature do you mean? Do we have it in a test suite somewhere? |
} | ||
|
||
// TODO should be generated by a custom codegen script | ||
trait ExprLifts(using override val internalQuotes: Quotes)(isPatternMode: Boolean) extends ReificationMacros with InternalTrait { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this generated previously? Why do we need so much more in Scala 3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was all generated by a macro triggered inside ReificationMacros.scala referencing scalameta/common/shared/src/main/scala/scala/meta/internal/trees/Liftables.scala (`materializeAst[]')
} | ||
|
||
// unapply methods | ||
extension (stringContext: scala.StringContext) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to repeat the methods here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they have different names, and those names point to different parsers in ReificationMacros
} | ||
object XTensionQuasiquoteSource { | ||
private[meta] def parse(input: scala.meta.inputs.Input, dialect: scala.meta.Dialect) = { | ||
val parse = implicitly[Parse[Source]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is all repeated could we not have a method:
def parseImpl[T](using parse: Parse[T] )(input: scala.meta.inputs.Input, dialect: scala.meta.Dialect) =
{
parse(input, dialect)
} match {
case x: scala.meta.parsers.Parsed.Success[_] => x.tree
case x: scala.meta.parsers.Parsed.Error => throw x.details
}
and use it everwhere?
|
||
|
||
// parsers | ||
object XTensionQuasiquoteTerm { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are those parsers used? I don't see any explicit usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In ReificationMacros
in instantiateParser()
, they are classloaded and invoked (just like in scala 2). In scala 2 this boilerplate was hidden inside classes with a macro annotation
} | ||
|
||
// test("deconstruction ascriptions") { | ||
// val q"foo(..${xs: List[Term]})" = q"foo(x, y)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ach, I see where we have the type ascriptions, I wasn't even sure it was possible. Would be good to support but if it's too hard we can leave it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without a new SIP it is impossible unfortunately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine to drop that functionality for Scala 3 then
import meta.prettyprinters.XtensionSyntax | ||
|
||
class Success extends TreeSuiteBase { | ||
test("rank-0 liftables") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this the same test suite like for Scala 2? Could we reuse it? Or would it need my PR first I guess?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now it has tests slightly changed to accommodate the Any
types returned from the unapply. I hope to use the old suite once we can get transparent inline unapply
working, outside of those 2 tests with non-standard type ascriptions of course
|
@tgodzik is this logic close enough, to be included in 4.9.0? |
I think it needs more work still, especially on the cross publishing branch |
This draft PR is meant to serve as a signal of progress made in migrating scalameta quasiquote macros to Scala3. Apologies for taking up the PR tracker with another draft PR. I will be unavailable for a few weeks, but I intend to come back to this PR after that time and finish it. It is very much not ready for review.
The aim here was to migrate only the macro methods used for defining quasiquotes, leaving all of the codegen-by-macro-annotations stuff in scala 2 for now. So far, I managed to get some quasiquote apply methods working, although sometimes the resolved type is incorrect (requiring asInstanceOf, as seen in the tests). The unapply methods do need some more work. Of course, things like the build structure etc. are not final.