WIP: Add cross-build for Scala 3.0

[felixbr]

IMPORTANT: This is WIP, I just want to put it up for discussion and to avoid duplication of efforts. There are lots of things that still need to be fixed. I've prefixed them with // TODO Scala3 so it's easy to find.

I took the initial work by @martijnhoekstra and tried to push it as far as possible. Many things work with little or no modification but anything that touches reflection is likely a blocker at this point. There are also one or two things that might be compiler-bugs and we should probably open tickets for the Scala 3 team.

Blockers so far:

1. Usage of Manifest. In some cases replacing it with ClassTag might be fine but it can be unsafe when used with the wrong types. The alternative would be source-incompatible changes. Both options aren't great.
2. Dependency on scala-reflect. I haven't looked into it yet but util-reflect needs scala-reflect and is used by other modules (util-benchmark, util-validator)
3. Usage of Jackson. Jackson has a scala-module for Scala3 but it will probably require Jackson 2.13.x. Twitter would probably have to upgrade Jackson first or we use different versions for Scala2 and Scala3.
4. The Java-API for Duration and Time is not compiling with Scala3. This could be a compiler-bug. In any case we have to solve it or provide the Java-API separately (which would be a breaking change).
5. util-mock depends on mockito-scala which uses macros and doesn't have a Scala3 version. I haven't checked yet if we can port it without mockito-scala macros, but I doubt it will work without breaking changes.

A lot of things are already working for both 2.13.6 and 3.0.0. Most importantly util-core. I had to replace sun.misc.Unsafe usage because in Scala3 it caused weird crashes. But with AtomicInteger and AtomicReference it works fine and if there are no performance issues, it also makes the code a bit cleaner.

There's probably more to be said but I'm hungry and the most important part is discussing the blockers. If anyone wants to collaborate directly on this PR, I'll gladly give you commit-access to the fork.

Cheers
~ Felix

[felixbr]

I've thought a bit more about what we can do with all the code using Manifest:

Manifest has been deprecated (and later "temporarily" undeprecated) a while ago and split into ClassTag and TypeTag. ClassTag is also available in Scala3 but it only gives you information that isn't erased at runtime. TypeTag can give you full type information (e.g. also type-parameters) but it is not available in Scala3.

For cases where Manifest was only used to get access to the runtime class (via classTag[T].runtimeClass) we can probably just switch to ClassTag and don't lose any safety or features.

For cases where "deeper" type information is extracted, we have to find a different solution because TypeTag is not available. First we will have to collect all of those cases and then we have to discuss what kind of breaking changes are tolerable.

My main goal is to unblock twitter/twitter-server and twitter/finagle for Scala3 migrations, so technically I only need the modules migrated which are used there. I haven't checked yet but I suspect the answer is "almost everything and especially the reflection stuff" 😅

[mosesn]

Thanks, this is great! Just as a heads up, we will probably want to merge this stuff in incrementally, rather than as a big-bang patch, so that we can review it carefully (especially where we have to break APIs w/ Manifest / TypeTag). However, I think keeping a bunch of ideas on a single big branch is also good. We'll just need to break it up later.

One other thing is that we're considering having a couple of interns take a stab at some of this work too, so we may be able to put together a crew to work on this.

Re: Manifest / ClassTag, I guess we have to switch to ClassTag? What's the alternative, use Java Class types?

[felixbr]

We have something similar to twitter/util internally and for the 2.12->2.13 migration we set up the sbt build with root and root2_13 as aggregations, while adding migrated modules incrementally to root2_13.

I could imagine a similar strategy here as it allows us to migrate modules one-by-one, split up work between people, and maybe even cross-publish the already migrated subset (not all modules might be needed for downstream projects).

Once #288 is merged, I can try and open a PR for just the cross-build setup and we go from there.

I'll leave this PR up for now if you don't mind, as the diff might be useful to others working on it just like I used the previous work by @martijnhoekstra .

Re: Manifest / ClassTag, I guess we have to switch to ClassTag? What's the alternative, use Java Class types?

Scala 3 will only have ClassTag for now (no plans for TypeTag yet). Alternatives for Manifest depend on what it's currently used for. Passing the Class explicitly might work but I'm not sure about erasure of type-parameters. The safest route is not using reflection at all and using typeclasses (or explicit parser-combinators) but that's probably also the biggest API change.

We will have to test how libraries like Jackson work with type-parameters. For example if something like decodeJson(classOf[Map[String, Foo]]) could work despite erasure.

[dotordogh]

Thank you so much for looking into this! I was imagining that if there wasn't a way to create binary compatible I did some work to tease apart what could be done in parallel, I'll add the table below. I suspected we'd have to have some 2.13 and 3 specific code for the cross-build until we can figure out blockers.

The information you provided about ClassTags is really useful, thank you!

Layer		Stream 1	Stream 2
Layer 1	util-core is a dependency for everything else	util-core done together (paired work)
Layer 2	Things that only depend on util-core / layer 1	util-lint, util-registry, util-app-lifecycle, util-hashing	util-mock, util-reflect, util-cache, util-codec
Layer 3	Things that depend on layer 2	util-app, util-stats, util-zk-test	util-thrift, util-cache-guava, util-slf4j-api
Layer 4	Things that depend on layer 3 and above	util-jvm, util-logging, util-tunable,	util-slf4j-jul-bridge, util-validator, util-routing
Layer 5	Things that depend on layer 4 and above	util-security, util-zk, util-test	util-jackson
Layer 6	Things that depend on layer 5 and above	util-benchmark

[felixbr]

I've opened a Scala3 bug regarding Java-API not being able to call overridden trait-methods (e.g. Duration.fromSeconds(1);): scala/scala3#12753

[smarter]

Scala 3 will only have ClassTag for now (no plans for TypeTag yet).

To be clear: TypeTag and everything else from scala-reflect will never come back in Scala 3, Scala 3 has its own meta-programming mechanisms detailed in https://dotty.epfl.ch/docs/reference/metaprogramming/toc.html. But depending on what you're using TypeTag for you might not need any of that and could either use ClassTag (if all you need is a convenient way to get the erased class of a type) or the new TypeTest mechanism: http://dotty.epfl.ch/docs/reference/other-new-features/type-test.html

[felixbr]

@smarter Thanks for the clarification. It wasn't my intention to imply TypeTag could potentially come back, I worded that poorly.

I didn't know about TypeTest, though (there is so much to read up on for Scala 3) 👍

[felixbr]

I just confirmed that in Scala 3.0.2 the compile errors for the Java-API will be fixed. 🙂

(I tried 3.0.2-RC1-bin-20210621-4aa7f90-NIGHTLY and it works)

[dotordogh]

Hey @felixbr, was this an automated rewrite or did you change this yourself? Where you addressing a warning that looked like the type test for <blah> cannot be checked at runtime?

[felixbr]

I changed usage of com.sun.misc.Unsafe and @volatile to AtomicReference and other Atomic* types because the former no longer worked with Scala3 for me. I didn't try to fix or improve anything else because this code is very low-level and "dangerous". The tests all worked fine with Atomic* but I didn't do any performance checks.

There were some other changes in the file, simplifying the logic, which I've copied from earlier work by @martijnhoekstra. So maybe he was addressing that warning.

edit: After thinking some more about it, maybe it wasn't Scala3 why com.sun.misc.Unsafe was no longer working, could also be a JDK11+ thing. In any case, if performance is the same, I'd recommend using Atomic* types, as they are much simpler and safer to use.

[dotordogh]

Noted! thank you so much for expanding!

[felixbr]

I rebased the changes onto the last commit before the (twitter-internal?) Scala3 migration by @hamdiallam as there's a bunch of conflicts now.

The Scala 3.0.2-RC1 version works fine, so that's good.

If I can help in any way by splitting things from this PR into a separate PR which can actually be merged into develop, please let me know 🙂

[hamdiallam]

Do you know what the Scala 3 compiler doesn't like the implicit conversion from int to long? Perhaps a bug?

[felixbr]

I don't see it as a bug. In Scala 2 you can already force a warning/error with -Wnumeric-widen in most cases where Int would implicitly be treated as Long. I'm not sure if that includes extension methods but if not that's an oversight imo.

I think not doing these conversions implicitly by default is a good thing.

[felixbr]

edit: I mixed up the types before.

Here you can see that with the compiler-flags turned on, it also produces a warning/error: https://scastie.scala-lang.org/vfZWBXEBQ3i0lXIlj7Q8cA

If you switch to Scala 3 it no longer works at all.

[felixbr]

Another option would be to write the extension methods in terms of Numeric but that has other problems like how to handle floating-point numbers correctly and losing the value class:

https://scastie.scala-lang.org/S9UEDrf4R02nklksX6Qfaw

I'd not do this for the Scala 3 migration at least, as the implicit conversion to Long is much simpler.

[mosesn]

@felixbr Hey, I realized I dropped the ball on this. We decided to have our interns work on Scala 3.x directly to reduce their overhead so they could make as much progress during their internship as possible. I’m really sorry for not keeping you in the loop earlier! Your work greatly inspired their intern project, so I feel especially bad that I didn’t do a good job of communicating it with you. We’ll work with you to split out the rest of the patches you’ve made so you can finish the job after their internship ends next week. I owe you a beer (or the beverage of your choice), please look me up if you’re ever in NYC!

[dotordogh]

Hi @felixbr,so sorry we dropped the ball on this! Let's talk about breaking up some of this work so we can start merging it! We like to merge subproject by subproject, so if you take a look at the build.sbt file, you will see that we have two settings: sharedSettings and sharedScala3EnabledSettings. sharedSettings doesn't include the cross-build with scala 3. Could you please start with breaking out the changes in util-logging and enable cross-building with scala 3 in just that subproject?

[felixbr]

Sorry for not answering sooner, I was on vacation.

@mosesn No need to apologize. I honestly don't care too much about attribution if omitting it speeds up anything. I just want to use Scala 3 with Finagle, any progress on that is good news to me.
I currently don't have much capacity to work on this migration and since the PR code here now has greatly diverged from develop, I'd first have to solve that before making meaningful progress. In other words: If you're waiting for me to do the rest, you're probably waiting for quite a while.

@dotordogh The two settings are great, that'll make things easier for sure. 🙂
As mentioned before I don't have much capacity but I will try to take a look at util-logging tomorrow.