-
Notifications
You must be signed in to change notification settings - Fork 346
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 #1105
base: develop
Are you sure you want to change the base?
Scala 3 #1105
Conversation
address compiler warnings/errors and setup kind-projector migration
Thanks for doing this! I'll look at it in the next days. It's quite large obviously so it will take some time. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #1105 +/- ##
===========================================
- Coverage 89.60% 89.58% -0.03%
===========================================
Files 119 124 +5
Lines 10016 10050 +34
Branches 695 572 -123
===========================================
+ Hits 8975 9003 +28
- Misses 1041 1047 +6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This is great! I can help out with the review. |
Some source files are moved or copy-pasted to reduce compile errors in Scala 3. They will be removed after tweaking build in future commits.
private[algebird] trait CompatDecayedVector { | ||
// This is the default monoid that never thresholds. | ||
// If you want to set a specific accuracy you need to implicitly override this | ||
implicit def monoid[F, C[_]](implicit vs: VectorSpace[F, C], metric: Metric[C[F]]):Monoid[DecayedVector[C]] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't compile in Scala 3 as Scala 3 requires explicit type annotation for implicit def.
Error says
Cannot find VectorSpace type class for Container: C
where: C is a type in method monoid with bounds <: [_] =>> Any
and Ring: Double
Now, algebirdCore 3.2.2 compiles in d123047 |
copy changes from twitter#1108 to make it easier to resolve conflict
use Math.random() to avoid auto application of `()` while keeping compatibility between 2.11 and 2.13
Now, algebird-core and algebird-test compile in Scala 2.11, 2.12, 2.13 and 3 at 7042270 and all tests but CollectionSpecification pass. CollectionSpecification gets StackOverflow probably because of difference of implicit resolution between Scala 2.x and Scala 3. I'm going to reduce copy-paste files in algebird-core using traits as in https://github.com/twitter/algebird/pull/1105/files#diff-1ae6af35211d6d6fea648cfe842339cd2d4d08933aed665a3ec78e5aee3c12f1. |
@@ -85,22 +85,23 @@ class CollectionSpecification extends CheckProperties { | |||
monoidLaws[Set[Int]] | |||
} | |||
|
|||
implicit def mapArb[K: Arbitrary, V: Arbitrary: Monoid] = Arbitrary { // scalafix:ok | |||
implicit def mapArb[K: Arbitrary, V: Arbitrary: Monoid]:Arbitrary[Map[K, V]] = Arbitrary { // scalafix:ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strangely, explicit type annotation Arbitrary[Map[K, V]]
here makes tests stack overflow.
@regadas Hi, could you run CI and look through changes? For now, algebird-core and algebird-test compile in Scala 3 andall tests in algebird-core and all tests but CollectionSpecification in algebird-test pass on my local machine. |
Sorry for forgetting fmt and check before asking for the review. |
scalameta says "pattern must be a value" for macro entrypoint, but it compiles and is proper format.
I found the same issue in scalameta. scalameta/scalameta#3063 |
Workaround: skip compiling java api for Scala 3 because 1. it seems Scala 3 generates Monoid$ in later stage than Scala 2.x, which prevents javaapi from compiling due to missing symbol. 2. we cannot use `compileOrder := CompileOrder.ScalaThenJava` as algebird-core must compile CassandraMurmurHash.java BEFORE Scala, but at the same time algebird-core must compile javaapi AFTER Scala.
@regadas Sorry for the previous request, it is now ready for review (this time, for sure) |
minimized reproduction of d51aa1b https://github.com/i10416/reproduce-missing-symbol-error-in-scala-3 |
@regadas Could you review changes when you have a time? |
#1099
NOTE:
changes for compatibility between 2.11, 2.12, 2.13 and 3.x
Source files compatible across all Scala version above are located in algebird-core/src/main/scala directory.
The most of changes in scala dir are to add explicit apply (
()
) and to resolve ambiguous values.Some files in scala-2.11 and scala-2.12+ are almost the same but there are a few differences.
*
s from kind-projector are replaced by_
in 2.12+._
are replaced by?
in 2.12+.asInstanceOf[T]
)(1) and (2) are necessary mainly because Scala 2.11 does not support "-Xsource:3" and "-P:kind-projector:underscore-placeholders" which are necessary for smooth kind-projector migration.
These duplicated files can be deleted after dropping Scala 2.11. For more details, see https://docs.scala-lang.org/scala3/guides/migration/plugin-kind-projector.html
Scala 3 macros
Macros in algebird-core are ported to Scala 3, but there are still room for peformance and type safety improvement.
Scala 3 macros in algebird-core are extracted to https://github.com/i10416/algebird-macros for debug and test purpose.