Imported From: https://issues.scala-lang.org/browse/SI-4695?orig=1
Reporter: @paulp

@adriaanm said:
could you have a look and bounce back if needed?

@som-snytt said:
This crash was more interesting before it was minimized; it's the same example, with the twist that if you compile the package object first, then compile the class with the same name, you get:

how can getCommonSuperclass() do its job if different class symbols get the same bytecode-level internal name: top/baz/package$

...
at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaName(GenASM.scala:611)
at scala.tools.nsc.backend.jvm.GenASM$JBuilder$$anonfun$addInnerClasses$4.apply(GenASM.scala:704)
at scala.tools.nsc.backend.jvm.GenASM$JBuilder$$anonfun$addInnerClasses$4.apply(GenASM.scala:698)
at scala.collection.immutable.List.foreach(List.scala:309)
at scala.tools.nsc.backend.jvm.GenASM$JBuilder.addInnerClasses(GenASM.scala:698)
at scala.tools.nsc.backend.jvm.GenASM$JPlainBuilder.genClass(GenASM.scala:1476)
at scala.tools.nsc.backend.jvm.GenASM$AsmPhase.run(GenASM.scala:180)

It must be serious when the message is a rhetorical question.

@retronym said:
Here's a means to partest this sort of problem:

retronym/scala@2.10.0-wip...retronym:topic/package-object-stub-flakiness

I'm going to take a look at this one.

@retronym said:
Regarding the case that I've seen a lot in practice:

package object demo extends demo.A

Martin suggests that if we took a strict approach we would outlaw this with a cyclic reference error, as it is conceptually the same manner as:

scala> object demo extends demo.A {
     |   trait A
     | }
<console>:7: error: illegal cyclic reference involving object demo
       object demo extends demo.A {
                           ^

This would break quite a lot of code, as nested types in the package are the most appealing for the package object to inherit. So we would have to deprecate it, or just leave it in a "use-at-your-own-risk" zone. We can tailor the error message for package objects to suggest a clean build.

However, if there is enough will to make this work, there might be a way. In this patch, I detect if any parents of a package module loaded from a classfile is missing (ie, is a stub symbol). If so, we pretend that we didn't see the package.class at all. At the conclusion of the run, we can check that the a package module we discarded was indeed reconstructed from sources.

retronym/scala@scala:2.10.0-wip...retronym:ticket/4695

I'm going to sit on this one for a while; comments are very welcome.

@harrah said:
If it helps add context, I put my sbt package object in the final subproject that aggregates everything. So, I'm personally ok with a restriction that the contents of package object x are not visible within x. I realize that this is probably a common use case in practice (if I remember correctly, my example above was a minimization of a user's code), but I figured I'd mention it as how I deal with package objects. It has the drawback that I haven't figured out how to build scaladoc that includes the package object (introduces cycles).

@odersky said:
I think it's best to be consistent with normal objects. My recommendation is that we should

1. Deprecate use of classes inside package p as base classes of package object p. The deprecation message should contain a note that says that such a pattern will lead to failures in incremental builds.

2. Instead of crashing, issue an error message that indicates that there's a cyclic reference, that this pattern has been deprecated, and that you should change your code or else do a clean build.

@adriaanm said:
upped to critical because we should add deprecation warnings as soon as we can in the release cycle -- if we decide to deprecate, that is

@paulp said:
Is there really any particular urgency? We can't meaningfully deprecate it before 2.11.0. Giving people warning in M3 or whatever would be a lovely gesture but hardly seems critical. I've spent a lot of time on this now and I don't think we need to deprecate, but it's not in pull-request shape yet.

@adriaanm said:
let's consider again whether to emit a warning now that M3 has suddenly turned into RC1

@retronym said:
My vote is to leave this one as is. SBT works around it by deleting package.class when recompiling the package object. One of my roadmap items for 2.12 was to spend a bit of time in package objects (detecting package objects from source before loading the one from the classpath, as Paul has prototyped)

@dwijnand said:
Semi-related, handing package objects in the incremental compiler was improved in sbt/sbt#2432.

s/wont-fix/have-fixed/

scala/scala#9661