"illegal cyclic reference involving class" generated by protobuf-java 3.8.0

[thesamet]

Protobuf-java 3.8.0 was released recently and the code that it generates is not accessible from Scala when nested messages are involved.

The bug that causes this was introduced on Scala 2.12.3 and it exists all the way up to 2.12.8. The bug is not reproducible in Scala 2.13.0.

I created a repo with minimal code that can reproduce this (does not depend on protobuf-java): https://github.com/thesamet/proto380_scalabug

Inside the same repo, under the directory full contains an actual real-world example (that depends on protobuf-java) that demonstrates this problem and can be used to verify that it is fully fixed.

For completeness, here is the code that reproduces this. It requires JavaThenScala compilation order.

NestedBuilder.java:

package jpkg;

interface NestedBuilder {
  Nested.Nested2.Nested3.Nested4 getNested4();
}

Nested.java:

package jpkg;

public class Nested implements NestedBuilder {
  public Nested.Nested2.Nested3.Nested4 getNested4() { return null; }

  static public class Nested2 {
    private Nested2() { }

    public static final class Builder2 {
      public Nested2 buildPartial() {
        return new Nested2();
      }
    }

    static public class Nested3 {
      private Nested3() { }

      public static final class Builder3 {
        public Nested3 buildPartial() {
          return new Nested3();
        }
      }

      static public class Nested4 { }
    }
  }
}

Use.scala:

package spkg;

object Use {
  val a: jpkg.Nested = null
}

Compilation error:

[error] /tmp/proto380_scalabug/src/main/scala/Use.scala:4:15: illegal cyclic reference involving class Nested
[error]   val a: jpkg.Nested = null

Workarounds would be appreciated as well as this is preventing us from moving forward with ScalaPB to 3.8.0.

[SethTisue]

@adriaanm @retronym any idea which of the 2.13 changes might be responsible here, and whether a 2.12.x backport might be feasible?

I've tentatively added it to the 2.12.9 milestone to ensure that it is at least considered.

[retronym]

Bisecting with

#! /bin/bash -e

. /code/binfu/scafu.sh
CANNOT_BE_TESTED=125

BISECT_SHA=${BISECT_SHA:-$(cat "$SCALA_SRC_HOME/.git/BISECT_HEAD")}
V=$(scala-ref-version "$BISECT_SHA" || exit $CANNOT_BE_TESTED)

cd $(dirname $0)
set -x
sbt ++$V! update || exit $CANNOT_BE_TESTED
sbt ++$V! "$@"

$ cd $SCALA_SRC_HOME
$ git bisect start --no-checkout v2.13.0 v2.13.0-M1 start && git bisect run /Users/jz/code/proto380_scalabug/bisect.sh compile

Pinpoints the prior regression to: scala/scala@04d4f4c

Repeating the exercise to find the recent progression:

#! /bin/bash -e

. /code/binfu/scafu.sh
CANNOT_BE_TESTED=125

BISECT_SHA=${BISECT_SHA:-$(cat "$SCALA_SRC_HOME/.git/BISECT_HEAD")}
V=$(scala-ref-version "$BISECT_SHA" || exit $CANNOT_BE_TESTED)

cd $(dirname $0)
set -x
sbt ++$V! update || exit $CANNOT_BE_TESTED
! sbt ++$V! "$@"

$ git bisect start --no-checkout --term-old=broken --term-new=fixed v2.13.0-RC1 v2.13.0-M4 start && git bisect run /Users/jz/code/proto380_scalabug/bisect.sh compile

Leads me where I suspected: scala/scala#6653

I already have a WIP backport that includes the fix. This use case adds some weight to the argument I should push that through for 2.12.9.

[dwijnand]

Fixed in scala/scala#8161.