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

Scala 3 support #262

Closed
wants to merge 42 commits into from
Closed

Scala 3 support #262

wants to merge 42 commits into from

Conversation

rmgk
Copy link

@rmgk rmgk commented May 19, 2022

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 Overload resolution for extension methods fails in some cases where named parameters are involved. scala/scala3#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.

@lihaoyi
Copy link
Member

lihaoyi commented Sep 28, 2022

@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

pythonparse/test/src/pythonparse/UnitTests.scala Outdated Show resolved Hide resolved

import language.experimental.macros

package object fastparse {
Copy link

Choose a reason for hiding this comment

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

package object are deprecated in scala 3. It seems thier support will be dropped in 3.2 . I think it would be nice to not have to have another migration to be done for a futur scala 3 version.

Copy link
Member

Choose a reason for hiding this comment

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

Migrated to top-level functions ✅

@davidrwood
Copy link

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

@reid-spencer
Copy link
Collaborator

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

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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Contributor

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
Copy link

ajrnz commented Dec 27, 2022

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,

lolgab added a commit to lolgab/fastparse that referenced this pull request Dec 27, 2022
These examples compiled in Scala 2.13.4 (the Scala version used in
com-lihaoyi#262) but don't compile in
Scala 2.13.6
I didn't understand the real reason, but helped the compiler by adding
some explicit type annotations
lolgab added a commit that referenced this pull request Dec 27, 2022
* Use `[_p: P]` instead of `[_: P]` for Scala 3 compatibility

* Fix compilation errors from Scala 2.13.6

These examples compiled in Scala 2.13.4 (the Scala version used in
#262) but don't compile in
Scala 2.13.6
I didn't understand the real reason, but helped the compiler by adding
some explicit type annotations
@lihaoyi
Copy link
Member

lihaoyi commented Feb 28, 2023

superseded by #266

@lihaoyi lihaoyi mentioned this pull request Feb 28, 2023
@lihaoyi
Copy link
Member

lihaoyi commented Feb 28, 2023

superseded by #271

@lihaoyi lihaoyi closed this Feb 28, 2023
lihaoyi added a commit that referenced this pull request Mar 2, 2023
All tests pass on Scala 3, on all platforms (JVM/JS/Native), and on all old versions of Scala.

Pulls in a bunch of work by @lolgab (#252) @rmgk (#262) and @ajrnz (#266)


# Notable Changes:

1. Moved MIMA checking only to the core `fastparse` modules; I don't think we can promise the same stability guarantees for the example parsers 
2. Tweaked the source file config in each module to allow per-platform-per-version sources, and extended that ability to test sources.
3. All the `.map(Foo)` calls have to become `.map(Foo.apply)`, `.map(Foo.tupled)` calls have to become `(Foo.apply _).tupled`
4. Duplicated `package.scala` for Scala 2 and 3, but splitting out the shared non-macro logic into `SharedPackageDefs.scala`
5. All macros had to be re-implemented in `fastparse/src-3/`
6. All implicits had to be given explicit return types
7. Fixed a bug in `aggregateMsgInRep` where we were not properly propagating failure strings
8. `then` has to be back-ticked
9. Replaced all `scala.Symbol`s in PythonParse with `String`s
10. Replaced the `_` method in ScalaParse with `Underscore`
11. Replaced some residual usage of uTests' `'foo -` syntax with `test("foo") -`


# Performance 
Performance seems to have taken about a 10% hit in Scala 3. Probably missing out on some optimizations that we do in Scala 2. I'm not super familiar with how scala 3 macros work, clawing back the 10% can come in a follow up PR

Scala 2 Bench
```
------------------ Running Tests perftests.string.ScalaParse ------------------
ScalaParse Benchmark
Max time - 10000 ms. Iterations - 5.
Iteration 1
Benchmark 0. Result: 984
Benchmark 1. Result: 82
Iteration 2
Benchmark 0. Result: 632
Benchmark 1. Result: 88
Iteration 3
Benchmark 0. Result: 618
Benchmark 1. Result: 96
Iteration 4
Benchmark 0. Result: 625
Benchmark 1. Result: 99
Iteration 5
Benchmark 0. Result: 596
Benchmark 1. Result: 99
984 82
632 88
618 96
625 99
596 99
```

Scala 3 Bench
```
------------------ Running Tests perftests.string.ScalaParse ------------------
ScalaParse Benchmark
Max time - 10000 ms. Iterations - 5.
Iteration 1
Benchmark 0. Result: 899
Benchmark 1. Result: 62
Iteration 2
Benchmark 0. Result: 535
Benchmark 1. Result: 86
Iteration 3
Benchmark 0. Result: 545
Benchmark 1. Result: 85
Iteration 4
Benchmark 0. Result: 547
Benchmark 1. Result: 86
Iteration 5
Benchmark 0. Result: 538
Benchmark 1. Result: 85
899 62
535 86
545 85
547 86
538 85
```
@lefou lefou added this to the 3.0.0 milestone Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants