twitter / rsc

Experimental Scala compiler focused on compilation speed
Apache License 2.0
1.24k stars 54 forks source link

Classpath loading misreads ExtModClassRefs to classes that end with $ as d.Term #379

Open xeno-by opened 5 years ago

xeno-by commented 5 years ago

It is a known problem that ScalaSignatures cannot distinguish between references to Scala objects and static parts of Java classes (https://github.com/scalameta/scalameta/issues/1491).

As a result, in order to convert from ScalaSignatures to SemanticDB, we need some additional information beyond just ScalaSignatures (because in SemanticDB, Scala objects and Java classes are modelled differently). Both Metacp and Rsc use classpath lookups to disambiguate.

Metacp does the right thing - to disambiguate an ExtModClassRef, it looks up a classfile that would host a ScalaSignature of a Scala object and then tries to load that classfile and check for the presense of ScalaSignature metadata. This, however, does a lot of work (reading from disk, parsing classfiles), so in Rsc I tried to cut corners and only look at filenames:

case sym: ExternalSymbol =>
  if (sym.isToplevelPackage) {
    d.Package(svalue)
  } else {
    val spackageSym = Symbols.Global(sowner, d.Package(sym.name.value))
    if (index.contains(spackageSym.bytecodeLoc)) {
      d.Package(svalue)
    } else {
      sym match {
        case _: ExtRef =>
          sym.name match {
            case _: TermName => d.Term(svalue)
            case _: TypeName => d.Type(svalue)
          }
        case _: ExtModClassRef =>
          val smoduleSym = Symbols.Global(sowner, d.Term(sym.name.value))
          if (index.contains(smoduleSym.bytecodeLoc)) d.Term(svalue)
          else d.Type(svalue)
      }
    }
  }

This approximation works well for a pretty big case study project, but one situation where it'll produce incorrect results is when there are classes that end with a $. Their corresponding classfiles will look like Scala object $.class files, which will confuse the classpath loader.

At the moment, this is not a problem at all, but I figured it would be a good idea to open this ticket for future reference if this becomes a problem down the line.

xeno-by commented 5 years ago

Reproduction of this ticket seems to be currently blocked by https://github.com/twitter/rsc/issues/380.