Skip to content

Commit

Permalink
Fix #397: Use a fake owner for orphan symbols in Scala 2 pickles.
Browse files Browse the repository at this point in the history
We had a long-standing issue with "orphan" `TYPEsym`s and `VALsym`s
in Scala 2 pickles. We were replacing them by "no symbol"
themselves, because we did not know what to do with them.

Issue #397 finally provided a good setup to diagnose it. It turns
out that those orphan symbols can be found as term and type params
of method types and poly types, in particular, Scala 2's encoding
of type lambdas, which can be produced by kind-projector.

Since we now have a good reason to have those orphan symbols, we
bite the bullet and allocate one global symbol to "host" all those
symbols coming from Scala 2.
  • Loading branch information
sjrd committed Nov 27, 2023
1 parent 9f94cad commit 3df9336
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 13 deletions.
10 changes: 10 additions & 0 deletions tasty-query/shared/src/main/scala/tastyquery/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ final class Definitions private[tastyquery] (ctx: Context, rootPackage: PackageS

val NothingAnyBounds = AbstractTypeBounds(SyntacticNothingType, AnyClass.topLevelRef)

// See `case noRef: NoExternalSymbolRef` in PickleReader
private[tastyquery] val scala2FakeOwner: TermSymbol =
TermSymbol
.createNotDeclaration(termName("<nosymbol>"), scalaPackage)
.withFlags(Synthetic, None)
.withDeclaredType(AnyType)
.setAnnotations(Nil)
.checkCompleted()
end scala2FakeOwner

private def createSpecialTypeAlias(
name: TypeName,
owner: DeclaringSymbol,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ private[reader] final class ReaderContext(underlying: Context):

def uninitializedMethodTermRef: TermRef = underlying.defn.uninitializedMethodTermRef

def scala2FakeOwner: TermSymbol = underlying.defn.scala2FakeOwner

def findPackageFromRootOrCreate(fullyQualifiedName: PackageFullName): PackageSymbol =
underlying.findPackageFromRootOrCreate(fullyQualifiedName)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,17 +203,13 @@ private[pickles] class PickleReader {
case sym: Symbol =>
sym
case noRef: NoExternalSymbolRef if tag == TYPEsym || tag == VALsym =>
/* For some reason, Scala 2 pickles `TYPEsym` and `VALsym` symbols whose owner references `NONEsym`.
* However, they do not appear to be referenced *themselves* from anywhere. They
* only appear in the table, and hence get read from the loop in `Unpickler.run`.
* We ignore these entries here by replacing them with `NONEsym`. If they end up
* being actually referenced somewhere, then that somewhere will then crash with
* an unexpected reference to `NONEsym`, which would provide us with more context
* to perhaps solve this at a deeper level.
* If someone wants to investigate, there is a case of `TYPEsym` in `scala.collection.Iterator`.
* There is a case of `VALsym` somewhere in sbt, triggered when tasty-mima-checking sbt-tasty-mima itself.
/* Sometimes, Scala 2 pickles `TYPEsym` and `VALsym` symbols whose owner references `NONEsym`.
* This seems to happen in METHODtpe and POLYtpe that are not for declared methods
* (but for type lambdas, or in annotations), for their parameter symbols.
* Our system does not have a notion of "no symbol". Instead, we use one fake
* owner to host these "orphan" symbols.
*/
return NoExternalSymbolRef.instance
rctx.scala2FakeOwner
case external =>
errorBadSignature(
s"expected local symbol reference but found $external for owner of ${name1.toDebugString} with tag $tag"
Expand Down Expand Up @@ -727,9 +723,7 @@ private[pickles] class PickleReader {
// create PolyType
// - PT => register at index
val restpe = readTypeMappableRef()
val typeParams = pkl.until(end, () => readMaybeExternalSymbolRef()).collect { case typeParam: TypeParamSymbol =>
typeParam
}
val typeParams = pkl.until(end, () => readLocalSymbolRef().asInstanceOf[TypeParamSymbol])
if typeParams.nonEmpty then TempPolyType(typeParams, restpe)
else restpe
case EXISTENTIALtpe =>
Expand Down

0 comments on commit 3df9336

Please sign in to comment.