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

Preparation work for Scala 3 #252

Closed
wants to merge 5 commits into from
Closed

Preparation work for Scala 3 #252

wants to merge 5 commits into from

Conversation

lolgab
Copy link
Member

@lolgab lolgab commented May 19, 2021

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 absent macros.

@lolgab lolgab force-pushed the scala-3-preparation branch 2 times, most recently from 52abbcd to 877ddbc Compare May 19, 2021 17:48
@lolgab lolgab marked this pull request as ready for review May 19, 2021 18:32
@ngbinh
Copy link

ngbinh commented May 24, 2021

awesome!

@lolgab lolgab marked this pull request as draft May 24, 2021 10:17
@lolgab lolgab force-pushed the scala-3-preparation branch 2 times, most recently from 83ffa4b to 611bfa6 Compare November 13, 2021 15:53
@lolgab lolgab marked this pull request as ready for review November 14, 2021 10:26
lolgab added 4 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
@lolgab lolgab force-pushed the scala-3-preparation branch from 611bfa6 to 18b6971 Compare March 1, 2022 11:53
@lolgab lolgab force-pushed the scala-3-preparation branch from 8ad9635 to 7a1b4c3 Compare April 20, 2022 21:23
@SethTisue
Copy link
Contributor

is this superseded by #262?

@lolgab
Copy link
Member Author

lolgab commented Jul 8, 2022

@SethTisue I guess it is! Thank you for commenting.
Closing as superseded by #262

@lolgab lolgab closed this Jul 8, 2022
@lolgab lolgab deleted the scala-3-preparation branch July 8, 2022 22:45
@lolgab lolgab restored the scala-3-preparation branch July 8, 2022 23:03
@lolgab lolgab reopened this Jul 8, 2022
@lolgab
Copy link
Member Author

lolgab commented Jul 8, 2022

Reopening since the PR targets this branch, so we need to understand if we want to split this into two PRs or not.

@lolgab
Copy link
Member Author

lolgab commented Jul 11, 2022

Changed #262 to target master so I can close this safely as superseded

@lolgab lolgab closed this Jul 11, 2022
@lolgab lolgab deleted the scala-3-preparation branch July 11, 2022 17:12
@lihaoyi lihaoyi mentioned this pull request 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.

4 participants