fastparse
fastparse copied to clipboard
Scala 3 support
Hi,
so there was this Scala 3 branch that seemed like it would work when just substituting some macro implementations. This is my experiment what that would entail. And maybe to start a discussion if its desirable to address the remaining issues, and if so, how.
In short, what works
- All functionality & macros ported to scala 3.
- all fastparse.jvm[3.1.2].test pass, except for one error message that has a slightly changed parser name, and I am not sure why.
What does not work / is problematic:
- ~~
.rep
has a lot of weird overloads that did not directly work on Scala 3 (and everything became ambiguous), so they are removed for now. This is source incompatible in the sense that one has to write.rep()
instead of.rep
.~~ This works withusing
instead ofimplicit
. - ~~This does not use macros yet. Lots of methods can probably be justed marked
inline
to get the old runtime behavior, however, for methods that build some matching logic at compile time like charsWhile in and StringsIn, this PR currently does a super literal translation of the prior logic, which is assumed to be incredibly inefficient.~~ All macros are ported, there are some annoyances with the way Scala 3 changed overload resolution, how fastparse uses overloads, and also https://github.com/lampepfl/dotty/issues/15287, which make it seemingly impossible to use an overload forrep(min)
while still remaining API compatibility for the rest. But even the non overloaded version should be fast enough for now. - I do know basically nothing about
mill
, and I did not spend time figuring out how to update dependencies to make building for Scala Native with Scala 3 happen.
who might be able to review and/or merge this...?
This PR passed under my radar. Thank you for commenting on my old PR @SethTisue. I'm not familiar with the FastParse Scala 2 macros but I can do a first round of review before someone more experienced with them and with Scala 3 macros can help :) This is great!! @rmgk Thank you!
I think I messed up 🙈 I deleted my PR with its branch not understanding that your PR was targeting my branch, not the master branch.
Just wondering what the state of this is please @lihaoyi ?
@rmgk @SethTisue @davidrwood @lolgab - Maybe it is time to consider a fork of this repository? It seems abandoned, yet it is critical to a project I'm working on and the lagging Scala 3 support is holding up our Scala 3 migration. I'm sure that is true for many others who use fastparse.
@lihaoyi - would you like to unburden yourself of the maintenance chore of this library?
Anyone who wants commit access can have it. I already have given commit access to several others. No need to fork, you can do it right in this repo if you would like. The only question is who wants to do it. There's about a half dozen individuals with commit access already
If someone wants to push Scala 3 support for Fastparse over the finish line, they are welcome to do so. I personally am stuck on Scala 2.12/2.13, so that person is unlikely to be me for the foreseeable future. The question is not whether I want to unburden myself, but if anyone else would like to pick up the burden
@lihaoyi Thanks for replying! Not something I can do myself but from what I can tell this exact PR should do the trick and just needs to be reviewed by someone qualified to do so.
Do you have time check it over perhaps? If so that might be all that's needed.🙂
@lihaoyi - Please add me as a committer. I'm highly invested in Fastparse and want to move the ball forward to Scala 3, so I may just take this on. Thanks.
Maybe as a meta comment; the largest additions in this PR are just translations of the Scala 2 macros. For the most part, the Scala 2 macros are used to inline code and the only difference between Scala 2 and 3 is the syntax for inlining – some do some code generation (support for literals and some kinda loop unrolling), these also require mostly syntactic translation. The only likely bugs in this part are because I did some copy & paste errors :-)
The API has very minor differences, specifically, it may be necessary to sometimes add/remove empty parameters lists (rant: Scala support for multiple argument lists, default arguments, and overloading are giant hacks which get exposed when using macros/inline). My gut feeling is, that it is not worth it to add too many workarounds to be fully backwards compatible with the current API (as a comparison, most changes in the test suite are because Scala 3 syntax changes independent of fastparse). But this feels like a “project maintainer” decision :-)
I never managed to run the larger test examples as I have no experience with mill and my updates to Scala 3 did break the full project (only the core library worked in my branch). The branch worked for my medium-sized parser, which is the point where I consider it good enough. Thus, I think getting the tests to run and testing on more projects is what I think the PR needs most currently.
Feel free to ask questions if you need help getting this over the finishing line – though no promises that I will find the time to answer :-)
@reid-spencer I have sent you an invite for write access to this repository
The goal for Scala 3 support should be to get all test suites in this repo passing, including the "integration tests" exercising ScalaParse/PythonParse/CssParse/etc. That would ensure you get all edge cases nailed down.
We should maintain Scala 2 support, so all those existing tests should continue to pass, to avoid regressions.
Ideally we should preserve backwards compatibility as much as possible, but I think it's OK if you find you need to break some stuff in the interest of Scala 3 support. If that becomes necessary, we can always just label the new version FastParse 3.0.0.
Some APIs would look different in Scala 3. IMO the common def parse[_: P] = P(...)
can probably be improved to def parse = P(...)
by making P(...)
a context function. That would be a nice simplification in the FastParse developer experience, and maybe even help encourage people to migrate
@reid-spencer I have sent you an invite for write access to this repository
Invitation accepted. Thank you.
The goal for Scala 3 support should be to get all test suites in this repo passing, including the "integration tests" exercising ScalaParse/PythonParse/CssParse/etc. That would ensure you get all edge cases nailed down.
Thanks for making it easy to achieve the goal
We should maintain Scala 2 support, so all those existing tests should continue to pass, to avoid regressions.
Agreed.
Ideally we should preserve backwards compatibility as much as possible, but I think it's OK if you find you need to break some stuff in the interest of Scala 3 support. If that becomes necessary, we can always just label the new version FastParse 3.0.0.
Hopefully that won't be needed, but thanks for your approval in advance.
Some APIs would look different in Scala 3. IMO the common
def parse[_: P] = P(...)
can probably be improved todef parse = P(...)
by makingP(...)
a context function. That would be a nice simplification in the FastParse developer experience, and maybe even help encourage people to migrate
It would indeed. The simpler we can make it the better. I expect that would require a 3.0.0 version number unless the older syntax can be retained at the same time.
@reid-spencer One more point of note: we do not need to upgrade Scalaparse's business logic to parse Scala 3 syntax as part of this effort, we only need to update its source code to be Scala 3 compatible. The existing Scala 2 test input files should be enough to validate Fastparse itself, for people using Fastparse elsewhere. Making Scalaparse able to parse Scala 3 would be a nice-to-have, but is not strictly necessary for the vast majority of use cases