puniverse / quasar

Fibers, Channels and Actors for the JVM
http://docs.paralleluniverse.co/quasar/
Other
4.56k stars 575 forks source link

Kotlin 1.1 support #281

Closed circlespainter closed 7 years ago

circlespainter commented 7 years ago

Some notes:

On Java 8, tests run fine with verification enabled in all of the following configs:

(Yes, Kotlin's compiler has now 2 targets: 1.6 and 1.8; the stdlib instead has now 3 versions: 1.6, 1.7 and 1.8).

pron commented 7 years ago

@circlespainter I don't understand the Java 8/7 issue. Does it work with quasar-core-jdk8 ?

circlespainter commented 7 years ago

@pron Yes it does.

The problem is that when using a JDK7 (and Java 7 deps), the Kotlin build prints:

w: Running the Kotlin compiler under Java 6 or 7 is unsupported and will no longer be possible in a future update.

And some test/lib code using Kotlin overloads breaks (I could actually see the wrong overloads called in bytecode), with the workaround simply being renaming the problematic overloaded methods (but of course this can happen in user code as well).

Instead with JDK8 no such warning is printed and everything works fine, no matter what the target source or bytecode (that is, both with target 7 and 8).

Actually, given such a warning, I think it would be sensible to avoid building the Kotlin module when using < JDK8.

circlespainter commented 7 years ago

I added conditional (on java8) inclusion for quasar-kotlin and I also added a quasar-kotlin-jdk8 project with the same inclusion condition and using the very same sources as quasar-kotlin, except that it depends on the jdk8 Quasar, on the jdk8 Kotlin stdlib and it compiles both Java and Kotlin sources with a 1.8 target.

Since the quasar-kotlin-jdk8's deps are different than quasar-kotlin I think the former can't be just an additional quasar-kotlin artifact and instead it needs to be a separate subproject.

jkbbwr commented 7 years ago

Do you have a timeline on the release cycle? I would love to be using this today but I can't seem to get anything working...

pron commented 7 years ago

@circlespainter Why do we need both quasar-kotlin and quasar-kotlin-jdk8 if Kotlin only works with JDK 8 anyway?

circlespainter commented 7 years ago

quasar-kotlin works with Java7 and the user Kotlin code can work with Java7 too (of course appropriate deps and target must also be used) but it has to be compiled with Kotlin running on Java 8 as the Kotlin compiler doesn't officially support anymore being run on Java7 and probably such support will be removed completely.

pron commented 7 years ago

Now I got it. Thanks. So why not make quasar-kotlin support JDK 8 only instead of adding yet another artifact?

circlespainter commented 7 years ago

Currently quasar-kotlin can only be used by people compiling with JDK 8 (because the Kotlin compiler only supports that) but it uses at most the Java 7 class format and APIs [1]; this means that people depending on it could still target the Java 7 class format and APIs and so support JRE 7 users.

quasar-kotlin-jdk8 instead uses the Java 8 APIs and class format [2] so there's no choice: all depending artifacts can only run on JRE 8.

As a simplifying choice we can surely drop the Java 7 quasar-kotlin and have only a Java 8 one, at the cost of losing JRE 7 users.

-- [1] More precisely the Kotlin jdk7 stdlib dependency uses the Java 7 class format and APIs but the Kotlin compiler targets the Java 6 class format (because it has no Java 7 target, only 6 and 8). [2] Because it uses the Kotlin compiler's Java 8 target, the jdk8 Quasar and the jdk8 Kotlin stdlib.

pron commented 7 years ago

I think we can drop JDK 7 support for Kotlin, seeing that the Kotlin compiler has also done that. There's no need for a new artifact name. We can make quasar-kotlin JDK 8 only.

circlespainter commented 7 years ago

Done, in addition I found a better workaround for the plugin issue (it's enough to disable timewarp for quasar-kotlin) and I upgraded to 1.1.3-2.

jkbbwr commented 7 years ago

Any idea how long til this goes to release?

pron commented 7 years ago

Released.

jkbbwr commented 7 years ago

@pron Cheers!