From 7535dd94c04c722b1d813231919f402c639d94fe Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Mon, 31 May 2021 17:14:47 +0200 Subject: [PATCH] Scala2Unpickler: Don't trust the owner field of tparams When a type parameter of a class external to the current pickle is referenced, Scala 2 will sometimes pickle it as if it was a type parameter of the current class. When we unpickle such a type parameter on our side, we enter it in the class, this leads the compiler to believe the class has more type parameters than it actually has. This doesn't happen in Scala 2 itself because it never enters unpickled type parameters in the class that owns them (it must be recreating them in some way, but I don't know how). It would be interesting to dig deeper to understand exactly how Scala 2 handles type parameter unpickling so we could emulate it given that this is not the first time we run into trouble here (cf #12120), but for now we fix the problem with the following heuristic: once we've loaded all the type parameters of a class (which we do eagerly via `ClassUnpickler#init()`), we simply stop from being entered any subsequent type parameter we see for this class. Time will tell if this is good enough. While I was touching that code, I also made ClassCompleter#completerTypeParams idempotent as one would expect it to be. Fixes #12641. --- .../core/unpickleScala2/Scala2Unpickler.scala | 47 +++++++++++++++---- sbt-test/scala2-compat/i12641/app/App.scala | 9 ++++ sbt-test/scala2-compat/i12641/build.sbt | 13 +++++ sbt-test/scala2-compat/i12641/lib/Lib.scala | 29 ++++++++++++ sbt-test/scala2-compat/i12641/test | 1 + 5 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 sbt-test/scala2-compat/i12641/app/App.scala create mode 100644 sbt-test/scala2-compat/i12641/build.sbt create mode 100644 sbt-test/scala2-compat/i12641/lib/Lib.scala create mode 100644 sbt-test/scala2-compat/i12641/test diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index d90e7b03e3f9..8684d7316963 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -495,7 +495,27 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas sym.setFlag(Scala2x) if (!(isRefinementClass(sym) || isUnpickleRoot(sym) || sym.is(Scala2Existential))) { val owner = sym.owner - if (owner.isClass) + val canEnter = + owner.isClass && + (!sym.is(TypeParam) || + owner.infoOrCompleter.match + case completer: ClassUnpickler => + // Type parameters seen after class initialization are not + // actually type parameters of the current class but of some + // external class because of the bizarre way in which Scala 2 + // pickles them (see + // https://github.com/scala/scala/blob/aa31e3e6bb945f5d69740d379ede1cd514904109/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala#L181-L197). + // Make sure we don't enter them in the class otherwise the + // compiler will get very confused (testcase in sbt-test/scala2-compat/i12641). + // Note: I don't actually know if these stray type parameters + // can also show up before initialization, if that's the case + // we'll need to study more closely how Scala 2 handles type + // parameter unpickling and try to emulate it. + !completer.isInitialized + case _ => + true) + + if (canEnter) owner.asClass.enter(sym, symScope(owner)) } sym @@ -625,23 +645,30 @@ class Scala2Unpickler(bytes: Array[Byte], classRoot: ClassDenotation, moduleClas object localMemberUnpickler extends LocalUnpickler class ClassUnpickler(infoRef: Int) extends LocalUnpickler with TypeParamsCompleter { - private def readTypeParams()(using Context): List[TypeSymbol] = { + private var myTypeParams: List[TypeSymbol] = null + + private def readTypeParams()(using Context): Unit = { val tag = readByte() val end = readNat() + readIndex - if (tag == POLYtpe) { - val unusedRestpeRef = readNat() - until(end, () => readSymbolRef()(using ctx)).asInstanceOf[List[TypeSymbol]] - } - else Nil + myTypeParams = + if (tag == POLYtpe) { + val unusedRestpeRef = readNat() + until(end, () => readSymbolRef()(using ctx)).asInstanceOf[List[TypeSymbol]] + } else Nil } - private def loadTypeParams(using Context) = + private def loadTypeParams()(using Context) = atReadPos(index(infoRef), () => readTypeParams()(using ctx)) + /** Have the type params of this class already been unpickled? */ + def isInitialized: Boolean = myTypeParams ne null + /** Force reading type params early, we need them in setClassInfo of subclasses. */ - def init()(using Context): List[TypeSymbol] = loadTypeParams + def init()(using Context): List[TypeSymbol] = + if !isInitialized then loadTypeParams() + myTypeParams override def completerTypeParams(sym: Symbol)(using Context): List[TypeSymbol] = - loadTypeParams + init() } def rootClassUnpickler(start: Coord, cls: Symbol, module: Symbol, infoRef: Int): ClassUnpickler = diff --git a/sbt-test/scala2-compat/i12641/app/App.scala b/sbt-test/scala2-compat/i12641/app/App.scala new file mode 100644 index 000000000000..ad94f2d8ac83 --- /dev/null +++ b/sbt-test/scala2-compat/i12641/app/App.scala @@ -0,0 +1,9 @@ +import cek.Async + + +object demo { + + def test1[F[_]](ev: Async[F]): Unit = ??? + def test2[F[_]](ev: Async[F]): Unit = ??? + +} diff --git a/sbt-test/scala2-compat/i12641/build.sbt b/sbt-test/scala2-compat/i12641/build.sbt new file mode 100644 index 000000000000..433a5e8baddf --- /dev/null +++ b/sbt-test/scala2-compat/i12641/build.sbt @@ -0,0 +1,13 @@ +val scala3Version = sys.props("plugin.scalaVersion") +val scala2Version = sys.props("plugin.scala2Version") + +lazy val lib = project.in(file("lib")) + .settings( + scalaVersion := scala2Version + ) + +lazy val app = project.in(file("app")) + .dependsOn(lib) + .settings( + scalaVersion := scala3Version + ) diff --git a/sbt-test/scala2-compat/i12641/lib/Lib.scala b/sbt-test/scala2-compat/i12641/lib/Lib.scala new file mode 100644 index 000000000000..f58697edf4f8 --- /dev/null +++ b/sbt-test/scala2-compat/i12641/lib/Lib.scala @@ -0,0 +1,29 @@ +package cek + +trait Async[F[_]] + +object Async { + trait WriterTAsync[F[_], L1] + extends Async[({ type LL[A] = WriterT[F, L1, A] })#LL] + with MonadCancel.WriterTMonadCancel[F, L1] { + + override def delegate = super.delegate + } + +} + +case class WriterT[F[_], L0, V]() + +trait MonadError[F[_]] +trait MonadCancel[F[_]] + +object MonadCancel { + + trait WriterTMonadCancel[F[_], L2] + extends MonadCancel[({ type LL[A] = WriterT[F, L2, A] })#LL] { + + def delegate: MonadError[({ type LL[A] = WriterT[F, L2, A] })#LL] = + ??? + + } +} diff --git a/sbt-test/scala2-compat/i12641/test b/sbt-test/scala2-compat/i12641/test new file mode 100644 index 000000000000..19aca297fdcf --- /dev/null +++ b/sbt-test/scala2-compat/i12641/test @@ -0,0 +1 @@ +> app/compile