Scala 3 support - Githubissues

rmgk commented 2 years ago

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 with using instead of implicit.
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 for rep(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.

SethTisue commented 2 years ago

who might be able to review and/or merge this...?

lolgab commented 2 years ago

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!

lolgab commented 2 years ago

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.

davidrwood commented 1 year ago

Just wondering what the state of this is please @lihaoyi ?

reid-spencer commented 1 year ago

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

lihaoyi commented 1 year ago

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

davidrwood commented 1 year ago

@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.🙂

reid-spencer commented 1 year ago

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

rmgk commented 1 year ago

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 :-)

lihaoyi commented 1 year ago

@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 commented 1 year ago

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

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.

lihaoyi commented 1 year ago

@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

davidrwood commented 1 year ago

@reid-spencer Just wondering how it's going with the Scala 3 update?

reid-spencer commented 1 year ago

@reid-spencer Just wondering how it's going with the Scala 3 update?

Unfortunately, I haven't started yet. Higher priority work is taking precedence.

xaperret commented 1 year ago

Hi, I'm trying to use fastparse for a project at university.

In the mean time is there any way to use this library in Scala 3 ? like in the build.sbt

libraryDependencies += ("com.lihaoyi" %% "fastparse" % "2.3.3").cross(CrossVersion.for3Use2_13)

Shouldn't this work ? I'm new to Scala so there is probably something I'm misunderstanding, but reading https://www.scala-lang.org/blog/2021/04/08/scala-3-in-sbt.html#using-scala-213-libraries-in-scala-3 is confusing me quite a bit.

reid-spencer commented 1 year ago

Hi, I'm trying to use fastparse for a project at university.

In the mean time is there any way to use this library in Scala 3 ? like in the build.sbt

This PR is about preparing fastparse to work with Scala 3. This PR has done much of the work but it is not complete and not verified.

reid-spencer commented 1 year ago

@lihaoyi - I'm going to back off my work converting fastparse to Scala 3. After 2 days of trying, I can't get grok mill and its many confounding error messages. I've fixed many of them in this PR but won't commit because I don't know where the end of that trail will lead. I am a total mill neophyte and unwilling to learn it since I will never use it again.

Sorry to disappoint everyone, but I'll just stick with 2.13.10 for now or switch to a LALR parser generator. Seems easier.

SethTisue commented 1 year ago

In the mean time is there any way to use this library in Scala 3 ?

No. Only macro-free Scala 2 libraries work with Scala 3. Fastparse is macro-based.

ajrnz commented 1 year ago

I've just create #266 which builds upon this and the previous PR

I've updated the mill project and library version and made fixes to cssparse and pythonparse. Now

mill cssparse.__.test
mill pythonparse.__.test

appear to work (jvm, js native).

scalaparse needs some macro work and a couple of tests are still failing.

I'm not sure how much time I'll have to continue, still we are almost at the point of having something working,

lihaoyi commented 1 year ago

superseded by https://github.com/com-lihaoyi/fastparse/pull/266

lihaoyi commented 1 year ago

superseded by https://github.com/com-lihaoyi/fastparse/pull/271

com-lihaoyi / fastparse

Scala 3 support #262