Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler error on Eff code compiled with 2.12 #6484

Closed
benhutchison opened this issue May 9, 2019 · 8 comments
Closed

Compiler error on Eff code compiled with 2.12 #6484

benhutchison opened this issue May 9, 2019 · 8 comments

Comments

@benhutchison
Copy link
Contributor

Scastie 1: Uses the Scala 2.12 version of Eff library under Scala 2.12. Compiles OK
https://scastie.scala-lang.org/aINnRnZOTBas6SrzWRBmfQ

Scastie 2: Uses the Scala 2.12 version of Eff library under Dotty. Doesn't compile
https://scastie.scala-lang.org/vTzj2G33SEqO1WtRBUo8oA

The sample code runReader makes use of implicits to compute the output type, and it seems that the semantics of implicit resolution varies in some way between Dotty and Scala 2.12, resulting in different behavior.

@OlivierBlanvillain
Copy link
Contributor

Might be working as intended, see changed-features/implicit-resolution.md

I think there is higher chance of having someone looking into this if the issue comes with a minimized example.

@benhutchison
Copy link
Contributor Author

benhutchison commented May 10, 2019

On further investigation and some advice from Eric Torreborre, I don't think the problem is implicit resolution. It has actually resolved the correct implicit.

Rather, I wonder if it may be related to how Dotty resolves the equality of two types...

In the Scastie below, Ive manually invoked the correct implicit with the correct type params. It still fails, but the error message is revealing. There's a lot going on in the types, type-lambdas, aliases and the Aux pattern, but to my reading the two types do look equal (Reader is Kleisli). But the compiler doesnt agree:

Found:    org.atnos.eff.Member.Aux[[X] => cats.data.Kleisli[cats.Id, String, X], 
  org.atnos.eff.Fx1[[X] => cats.data.Kleisli[cats.Id, String, X]]
, org.atnos.eff.NoFx]
Required: org.atnos.eff.Member'[[β$0$] => cats.data.Kleisli[[A] => A, String, β$0$], 
  org.atnos.eff.Fx1[[X] => cats.data.Kleisli[cats.Id, String, X]]
]

where:    Member  is a object in package eff
          Member' is a trait in package eff

https://scastie.scala-lang.org/m05H3wCqTzqkW4wdgTB5wA

@OlivierBlanvillain
Copy link
Contributor

String(g) is g.type, we need to fix that in the pretty printer...

@benhutchison
Copy link
Contributor Author

Another experiment: Here's a related example, but with no compiled Eff binaries, and the stripped down, relevant parts of Eff moved into the Scastie example.

https://scastie.scala-lang.org/osccD4TmQEGfCnvMR63r9g

This compiles OK! Whereas when the same code is loaded from an Eff binary compiled with Scala 2.12, it gets the type error in the previous comment above.

So it seems the Dotty compiler can resolve the code when it compiles it directly with Dotty. But when it reconstructs the types from the 2.12 binary file using withDottyCompat, something is a little different and the compiler can't see the Found type == the Expected type.

@smarter
Copy link
Member

smarter commented May 10, 2019

Running the code from #6484 (comment) with -explain-types to get the compiler to tell me why it thinks these types are not equal, I see:

   |                  ==> cats.Id <:< [A] => A
   |                    ==> cats.Id <:< [A] => A recur
   |                      ==> [A] => A <:< [A] => A recur
   |                      <== [A] => A <:< [A] => A recur  = false
   |                    <== cats.Id <:< [A] => A recur  = false
   |                  <== cats.Id <:< [A] => A   = false

So something weird is definitely going on. Further debugging reveals that one of the [A] => A is an HKTypeLambda (used to represent type lambdas) and the other one is a PolyType (used to represent polymorphic methods). So we're confusing the two somehow when unpickling symbols from Scala 2, and this piece of code looks suspiciously related: https://github.com/lampepfl/dotty/blob/cdd844f3794910410482ffb2da449b86623e5248/compiler/src/dotty/tools/dotc/core/unpickleScala2/Scala2Unpickler.scala#L49

smarter added a commit to dotty-staging/dotty that referenced this issue May 10, 2019
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.
@benhutchison
Copy link
Contributor Author

Thanks for the rapid diagnosis and fix @smarter, much appreciated!

It has certainly helped build my trust in the Dotty migration to see these sort of early stage compatibility bugs vigorously investigated & resolved.

@benhutchison
Copy link
Contributor Author

I notice these commits went to dotty-staging, but they didnt seem to publish a binary that I could retest with... is there a way I can test the fix in my own project?
http://dotty-ci.epfl.ch/lampepfl/dotty/12728

smarter added a commit that referenced this issue May 11, 2019
Fix #6484: Properly unpickle some Scala 2 type lambdas
@smarter
Copy link
Member

smarter commented May 11, 2019

dotty-staging is just the remote we use to push branches for each PR to facilitate collaboration (only one remote that everyone can push to). In this case the PR is at #6494.

is there a way I can test the fix in my own project?

We don't (yet) publish binaries for every open PR. But since #6494 has been merged it will be part of the next nightly build. So tomorrow you'll be able to test it by setting scalaVersion to the latest nightly version from https://repo1.maven.org/maven2/ch/epfl/lamp/dotty_0.15/ (or alternatively by setting scalaVersion := dottyLatestNightlyBuild.get to automatically fetch the latest one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants