2.12 regression, backend crash: Cannot create ClassBType from non-class symbol

[lrytz]

➜  scala git:(2.12.0) ✗ cat p/package.scala
package object p {
  type Prop = String
}
➜  scala git:(2.12.0) ✗ cat p/Prop.scala
package p

object Prop {
  class Whitelist
}
➜  scala git:(2.12.0) ✗ cat p/PropTest.scala
package p
object PropTest {
  def t = new Prop.Whitelist
}
➜  scala git:(2.12.0) ✗ qsc p/package.scala p/Prop.scala && qsc p/PropTest.scala
java.lang.AssertionError: assertion failed:
  Cannot create ClassBType from non-class symbol type Prop

[adriaanm]

Here's a weird inconsistency that may explain this bug:

class Symbol ... { ...
    /** For a module: the class with the same name in the same package.
     *  For all others: NoSymbol
     *  Note: does not work for classes owned by methods, see Namers.companionClassOf
     *
     *  object Foo  .  companionClass -->  class Foo
     *
     *  !!! linkedClassOfClass depends on companionClass on the module class getting
     *  to the class.  As presently implemented this potentially returns class for
     *  any symbol except NoSymbol.
     */
    def companionClass: Symbol = flatOwnerInfo.decl(name.toTypeName).suchThat(_ isCoDefinedWith this)

class ModuleSymbol ... extends Symbol ... {
..
  override def companionClass =
      flatOwnerInfo.decl(name.toTypeName).suchThat(sym => sym.isClass && (sym isCoDefinedWith this))

[adriaanm]

Why only check sym.isClass in ModuleSymbol? Seems to me that condition should be included in class Symbol

[lrytz]

i have a patch ready soon, this is one of the issues.

[adriaanm]

Ok, cool!

[lrytz]

Another weird thing is that two ModuleClassSymbols for Prop are created: one because there's a classfile Prop.class, another when unpickling the ModuleSymbol for Prop.

Later the associatedFile is defined on the first of those two (to Prop.class), but never on the second.

Any intuition if creating the two symbols is fishy or not?

[adriaanm]

Hmm, that does sound suboptimal, but I never dug deep into module symbol creation.

[lrytz]

In sequence

• The ModuleSymbol is created because there's a classfile Prop.class
• While doing so, a corresponding ModuleClassSymbol (MCS) is created and linked (module.referenced == moduleClass)
• When completing the type of the ModuleSymbol, Prop.class is passed to the classfile parser
  • The file has a ScalaSignature, so it goes to the unpickler
  • The signature contains a full pickle of the MCS (it's not a reference to a symbol), so a second MCS is created
  • The referenced field in the module is not updated, so it still points to the first MCS
• The completer of the module then sets the module.associatedFile, which delegates to moduleClass.associatedFile, so it goes to the first of the two MCS.
• Later in the compilation, the second of the two MCS shows up in the owner chain of a nested class
• The lack of an associatedFile causes isCodefinedWith to be true

Your patch fixes it, and that test should be there, but there's also something fishy in the above.

[lrytz]

will cont. tomorrow, need to pay some overdue bills tonight 💵

[lrytz]

Unfortunately I don't see an easy workaround for people affected by this (japgolly/nyaya#22 (comment)). If an already compiled module (or a dependency) contains code like this:

package object p {
  type Prop = String
}
package p {
  object Prop {
    class Whitelist
  }
}

any seperately-compiled code that accesses Whitelist will crash the compiler. Ideas?

[adriaanm]

Use a value class instead of a type alias? Dare I ask what weird interactions that would yield? :-)

[adriaanm]

Lift Whitelist out of the object?

[sjrd]

Is that supposed to be valid code to begin with? type Prop and object Prop are basically companions, so the object should be put within the package object, next to the type alias.

[lrytz]

Lift Whitelist out of the object?

Sure, but that's a change to the project's API.. :(

Use a value class instead of a type alias?

?

Is that supposed to be valid code to begin with? type Prop and object Prop are basically companions

I think by the spec, a companion class needs to be a class, not a type def. So they are not companions.

the object should be put within the package object, next to the type alias.

That actually helps as a workaround. By writing

package object p {
  type Prop = String
  object Prop { class Whitelist }
}

the classfile name is package$Prop$.class, there's no more Prop.class, so the bug goes away (there's only a single Prop ModuleClassSymbol now).

But it's not binary compatible, so users could not easily switch back to the other version. Also it's not recommended to put object / class definitions within a package object.

[japgolly]

Firstly, thanks @lrytz for narrowing that down and determining the cause so quickly.

Secondly, allow me to provide some more info as to Why I Do These Things. Nyaya isn't the best example as there was a bunch of unplanned, organic growth that I've yet to clean up in Nyaya. A better example is Callback in scalajs-react which I think highlights this as quite a reasonable practice. To explain:

1. There exists a class called CallbackTo[A]. It has a companion object for constructing CallbackTo[A] instances and lives in Callback.scala.
2. The most common usage of CallbackTo[A] is CallbackTo[Unit]. Therefore I've created a type alias type Callback = CallbackTo[Unit]. Unfortunately, because package-level type aliases must be in package.scala, it exists there.
3. There are a bunch of helper methods related to Callback specifically (i.e. that specialise on the carrier being Unit) and so object Callback exists. It's around 105 LoC and makes sense organisationally to be in Callback.scala and not package.scala.
4. Thus, the object is in a named file for manageability and the alias is in the package file for necessity.

A possible workaround would be to move the Callback object to another package or give it a silly name, then drop val Callback = CallbackObject or similar into the package object but that's messy, artificial and a bit confusing for users. I'd really like to avoid that.

Ideally we'd be able to define package-level type aliases anywhere in the package, just like classes and objects but I doubt that change would make it into 2.12.

Anyhow, nothing useful in terms of technically resolving this, but hopefully some use in providing insight into cases like this.

[lrytz]

The bug is almost the same as https://issues.scala-lang.org/browse/SI-5031, with fixes in scala/scala#1016 and scala/scala#1532

I'm almost at the bottom of the issue in the classfile parser / unpickler, but don't see a way out yet..

[lrytz]

Minimal "fix" (work around the underlying issue) in the backend: scala/scala#5479

[lrytz]

WIP for frontend fixes / cleanups: scala/scala#5482 - see individual commit messages.