From c70f6e5019885a2dbb666e6cf3b78d5c9d1036b4 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 11 May 2019 01:09:54 +0200 Subject: [PATCH] Fix #6484: Properly unpickle some Scala 2 type lambdas Scala 2 pickles both type lambdas and polymorphic methods using the POLYtpe tag. When we unpickle them, we distinguish them based on whether the symbol for the owner of the type parameters is a term or a type, but it seems that some type parameter symbols for type lambdas are pickled with a term owner. In Eff for example, the pickled information for the `runReader` method in: https://github.com/atnos-org/eff/blob/85bd7b2dc1cd26c22e45d69910755f2a9ea4ece4/shared/src/main/scala/org/atnos/eff/syntax/reader.scala#L12 contains a POLYtpe `[A] => A` (because `Reader` is defined in cats as `type Reader[A, B] = Kleisli[Id, A, B]`, and `Id` is defined as `type Id[A] = A`), somehow the owner of the symbol for `A` is the object `reader` defined in the same file. That doesn't make any sense to me, but just checking if the owner is actually a method should work just as well and fixes the problem. Given that Scala 2 unpickling support is a temporary crutch I think that's good enough. --- .../dotc/core/unpickleScala2/Scala2Unpickler.scala | 6 +++++- sbt-dotty/sbt-test/scala2-compat/eff/build.sbt | 4 ++++ sbt-dotty/sbt-test/scala2-compat/eff/i6484.scala | 13 +++++++++++++ .../sbt-test/scala2-compat/eff/project/plugins.sbt | 1 + sbt-dotty/sbt-test/scala2-compat/eff/test | 1 + 5 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 sbt-dotty/sbt-test/scala2-compat/eff/build.sbt create mode 100644 sbt-dotty/sbt-test/scala2-compat/eff/i6484.scala create mode 100644 sbt-dotty/sbt-test/scala2-compat/eff/project/plugins.sbt create mode 100644 sbt-dotty/sbt-test/scala2-compat/eff/test diff --git a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala index 6cd15e737264..61683bd1c854 100644 --- a/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala @@ -46,7 +46,11 @@ object Scala2Unpickler { /** Convert temp poly type to poly type and leave other types alone. */ def translateTempPoly(tp: Type)(implicit ctx: Context): Type = tp match { case TempPolyType(tparams, restpe) => - (if (tparams.head.owner.isTerm) PolyType else HKTypeLambda) + // This check used to read `owner.isTerm` but that wasn't always correct, + // I'm not sure `owner.is(Method)` is 100% correct either but it seems to + // work better. See the commit message where this change was introduced + // for more information. + (if (tparams.head.owner.is(Method)) PolyType else HKTypeLambda) .fromParams(tparams, restpe) case tp => tp } diff --git a/sbt-dotty/sbt-test/scala2-compat/eff/build.sbt b/sbt-dotty/sbt-test/scala2-compat/eff/build.sbt new file mode 100644 index 000000000000..0229055e2bad --- /dev/null +++ b/sbt-dotty/sbt-test/scala2-compat/eff/build.sbt @@ -0,0 +1,4 @@ +scalaVersion := sys.props("plugin.scalaVersion") + +libraryDependencies += + ("org.atnos" %% "eff" % "5.4.1").withDottyCompat(scalaVersion.value) diff --git a/sbt-dotty/sbt-test/scala2-compat/eff/i6484.scala b/sbt-dotty/sbt-test/scala2-compat/eff/i6484.scala new file mode 100644 index 000000000000..652ce4efd776 --- /dev/null +++ b/sbt-dotty/sbt-test/scala2-compat/eff/i6484.scala @@ -0,0 +1,13 @@ +import cats._ +import cats.data._ +import cats.implicits._ +import org.atnos.eff._ +import org.atnos.eff.all._ +import org.atnos.eff.syntax.all._ + +import scala.language.implicitConversions + +object Test { + def resolve[T](e: Eff[Fx1[[X] => Kleisli[Id, String, X]], T], g: String): T = + e.runReader[String](g)(Member.Member1[[X] => Kleisli[Id, String, X]]).run +} diff --git a/sbt-dotty/sbt-test/scala2-compat/eff/project/plugins.sbt b/sbt-dotty/sbt-test/scala2-compat/eff/project/plugins.sbt new file mode 100644 index 000000000000..c17caab2d98c --- /dev/null +++ b/sbt-dotty/sbt-test/scala2-compat/eff/project/plugins.sbt @@ -0,0 +1 @@ +addSbtPlugin("ch.epfl.lamp" % "sbt-dotty" % sys.props("plugin.version")) diff --git a/sbt-dotty/sbt-test/scala2-compat/eff/test b/sbt-dotty/sbt-test/scala2-compat/eff/test new file mode 100644 index 000000000000..5df2af1f3956 --- /dev/null +++ b/sbt-dotty/sbt-test/scala2-compat/eff/test @@ -0,0 +1 @@ +> compile