lightbend / genjavadoc

A compiler plugin for generating doc’able Java source from Scala source
Other
58 stars 32 forks source link

Set compiler phase `runsBefore` to `tailcalls` #191

Closed lrytz closed 5 years ago

lrytz commented 5 years ago

runsAfter is set to fields. However, in reality the phase assembly algorithm used in 2.12.9 and before, also 2.13.0, places the GenJavadoc phase right after specialize, which is valid, but probably not intended.

With some changes in the phase assembly algorithm in 2.13.1, the GenJavadoc phase was now placed right after fields, which caused tests to fail.

lrytz commented 5 years ago

Interesting... I can take a look. Which branch of akka did you use?

raboof commented 5 years ago

I checked with Akka master (bc590af56f39cff358d7a67d28f01e98f05741a8). I did a sbt ++2.12.9 publishLocalSigned of this branch with #197 cherry-picked on top on the genjavadoc side, and updated unidocGenjavadocVersion in project/Doc.scala on the akka side.

lrytz commented 5 years ago

The problem is that adding the javadoc plugin shuffles around the compiler's own phases...

Without the plugin:

$> scalac Test.scala -Vphases
    phase name  id  description
    ----------  --  -----------
        fields  11  synthesize accessors and fields, add bitmaps for lazy vals
     tailcalls  12  replace tail calls by jumps
    specialize  13  @specialized-driven class and method specialization
 explicitouter  14  this refs to outer pointers
       erasure  15  erase types, add interfaces for traits
   posterasure  16  clean up erased inline classes
    lambdalift  17  move nested functions to top level

With the plugin, tailcalls and specialize move to the back and are now after lambdalift, they used to be after fields:

$> scalac -Xplugin:/Users/luc/.ivy2/local/com.typesafe.genjavadoc/genjavadoc-plugin_2.13.0/0.11_2.13.0-M4+58-bb141048+20190917-1239/jars/genjavadoc-plugin_2.13.0.jar Test.scala -Vphases
    phase name  id  description
    ----------  --  -----------
        fields  11  synthesize accessors and fields, add bitmaps for lazy vals
    GenJavadoc  12
 explicitouter  13  this refs to outer pointers
       erasure  14  erase types, add interfaces for traits
   posterasure  15  clean up erased inline classes
    lambdalift  16  move nested functions to top level
     tailcalls  17  replace tail calls by jumps
    specialize  18  @specialized-driven class and method specialization

We really have to clean up the phases list in the compiler, the ordering should not be left to the algorithm. Currently we have

  object tailCalls extends {
    val runsAfter = List("uncurry")

  object fields extends {
    val runsAfter = List("uncurry")

  object explicitOuter extends {
    val runsAfter = List("fields")

that's why tailcalls can move around...

So this PR works for 2.12.10 (tested locally) and 2.13.1 (tested in community build). These two versions include the changes to the phase assembly algorithm that causes breakages without this PR.

So it seems we really need to have two versions here, one for 2.10, 2.11, 2.12.0-2.12.9, 2.13.0, and the other one for 2.12.10+, 2.13.1+.

lrytz commented 5 years ago

@raboof please take another look? And if you have time, it would be great to try if it works with akka, with both 2.12.9 and 2.12.10, maybe also 2.13.0 and 2.13.1-bin-3987e83 from https://scala-ci.typesafe.com/artifactory/scala-integration, this is our 2.13.1 RC.

lrytz commented 5 years ago

The community build for 2.13.1-bin-3987e83 is also running (https://scala-ci.typesafe.com/job/scala-2.13.x-integrate-community-build/2610) and testing the latest revision of this PR (a7821432bf1b8f57d663f394e9feacc1d7c8464e).

raboof commented 5 years ago

Seemed to work for Akka with 2.12.9 and 2.13.0, so let's merge

lrytz commented 5 years ago

Thank you!