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 preparation #266

Closed
wants to merge 47 commits into from
Closed

Conversation

ajrnz
Copy link

@ajrnz ajrnz commented Dec 27, 2022

This PR build on 2 previous PRs for scala 3 support

cssparse and pythonparse now compile and pass tests

fastparse still has some test failures

scalaparse has an issue with a macro

lolgab and others added 30 commits March 1, 2022 12:52
This commit does some preparation work to allow the port to Scala 3
The idea is to move all the code using macros to the `src-2` directory,
update the dependencies to the ones supporting Scala 3 ( mill, geny,
sourcecode, utest ), avoid depending on `scala-reflect` if it's Scala 3
and fixing the compile errors the Scala 3 compiler gives before reaching
the missing macros.
Skip scala-compiler dependency for Scala 3
using instead of implicit seems to work out
@davidrwood
Copy link

Is this essentially competing with #267 ?

@kyri-petrou
Copy link

Hi there 👋 I'm one of the contributors of Caliban, and I've been occasionally checking the status of the Scala 3 release of fastparse. Caliban offers supports Scala 2.12, 2.13 and 3. The Scala 2.x versions use fastparse to parse the GraphQL queries and cats-parse for Scala 3 until fastparse becomes available for Scala 3 as well.

I wanted to mention that once you're at a stage with this PR that you're confident in (or have a snapshot release), I'd be happy to test the Scala 3 version of fastparse in Caliban. Since we already have the Scala 2 code, unit tests and benchmarks (for the graphql query parsing), it should be relatively low effort to implement and test

@davidrwood
Copy link

@ajrnz Just wondering if you've got a chance to get this wrapped up? It seems like it's very close...

lihaoyi added a commit that referenced this pull request Feb 28, 2023
#266 uses `$`, and I think `$` is probably the better option for now (since it's shorter), so let's go with that for now
@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
@ajrnz ajrnz deleted the scala-3-preparation branch March 1, 2023 08:15
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.

7 participants