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

How to use auto-derivation with contravariant TC #336

Open
ghostbuster91 opened this issue Sep 12, 2021 · 10 comments
Open

How to use auto-derivation with contravariant TC #336

ghostbuster91 opened this issue Sep 12, 2021 · 10 comments
Labels

Comments

@ghostbuster91
Copy link
Contributor

ghostbuster91 commented Sep 12, 2021

Hi,

I am struggling with the migration of diffx to scala3, especially with the derivation mechanism.
I did a quick check and it seems that changing the Eq tc from magnolia examples also doesn't work. It fails with the following error:

[error] -- Error: /home/kghost0/workspace/magnolia/src/test/tests.scala:295:34 ---------
[error] 295 |      val res = summon[Eq[Person]].equal(Person("John Smith", 34), Person("", 0))
[error]     |                                  ^
[error]     |no implicit argument of type magnolia1.examples.Eq[magnolia1.tests.Person] was found for parameter x of method summon in object Predef.
[error]     |I found:
[error]     |
[error]     |    magnolia1.examples.Eq.autoDerived[A](
[error]     |      /* missing */
[error]     |        summon[
[error]     |          deriving.Mirror{
[error]     |            MirroredType >: magnolia1.tests.Person; 
[error]     |              MirroredMonoType >: magnolia1.tests.Person
[error]     |            ; MirroredElemTypes <: Tuple
[error]     |          }
[error]     |        ]
[error]     |    )
[error]     |
[error]     |But no implicit values were found that match type deriving.Mirror{
[error]     |  MirroredType >: magnolia1.tests.Person; 
[error]     |    MirroredMonoType >: magnolia1.tests.Person
[error]     |  ; MirroredElemTypes <: Tuple
[error]     |}.

Am I missing something? Has somebody tried already something like that?

Full code to reproduce this error can be found here: https://github.com/ghostbuster91/magnolia/tree/contravariant-derivation-3

Notice that calling derivation explicitly Eq.derived[Person] works without any problems.

@KacperFKorban
Copy link
Collaborator

HI, this might be connected to scala/scala3#13146

@ghostbuster91
Copy link
Contributor Author

ghostbuster91 commented Sep 13, 2021

Well yes and no :) In the linked issue it is said that this is due to the limitation of the type inference. Providing more information to the compiler using alternative syntax solves this problem.

In my linked example I have already done it for the Tree ADT where new syntax for derivation

sealed trait Tree[+T] derives Eq
object Tree:
  given [T: [X] =>> Show[String, X]] : Show[String, Tree[T]] = Show.derived

was replaced with

sealed trait Tree[+T]
object Tree:
  given [T: [X] =>> Show[String, X]] : Show[String, Tree[T]] = Show.derived
  given lEq[T: Eq] : Eq[Leaf[T]] = Eq.derived[Leaf[T]]
  given bEq[T: Eq] : Eq[Branch[T]] = Eq.derived[Branch[T]]
  given tEq[T : Eq]: Eq[Tree[T]] = Eq.derived[Tree[T]]

But the Eq tc implements AutoDerivation from magnolia so it should be possible to use it without any additional definitions for the Entity ADT.

Last but not least it seems like a good idea to incorporate some tests against contravariant type-classes into magnolia's test suite.

@adamw
Copy link
Member

adamw commented Sep 13, 2021

@ghostbuster91 what if you add an explicit type parameter to derived? e.g. Show.derived[[X] =>> Show[String, X]]]?

@ghostbuster91
Copy link
Contributor Author

ghostbuster91 commented Sep 13, 2021

@adamw I mentioned the change in Eq definition for Tree ADT, because without that the project didn't want to compile after making Eq tc contravariant (and it is connected with the issue @KacperFKorban linked). As soon as I did it, it worked. The second change which I made was to replace single line in tests:
val res = Eq.derived[Entity].equal(Person("John Smith", 34), Person("", 0))

with summoning an implicit instant:
val res = summon[Eq[Person]].equal(Person("John Smith", 34), Person("", 0))

This change alone (without going contravariant) works, but combined with contravariant modification it fails to compile.
Sorry if that wasn't clear from my original issue.

All the changes can be seen in this single commit: ghostbuster91@3e5c919

@KacperFKorban
Copy link
Collaborator

KacperFKorban commented Sep 13, 2021

@ghostbuster91
Ok, so the problem actually involves a hierarchy that wasn't edited in the commit. I must have missed it before, sorry 😅
Adding explicit instances for Entity seems to solve the problem.

object Entity:
  given Eq[Entity] = Eq.derived[Entity]
  given Eq[Person] = Eq.derived[Person]
  given Eq[Company] = Eq.derived[Company]

@ghostbuster91
Copy link
Contributor Author

No problem :) But that seems like a semi-auto derivation, doesn't it?

@KacperFKorban
Copy link
Collaborator

Yes, it does. But that feels like a dotty bug since the instance is generated correctly. The only problem is with finding already existing instances.

@dantb
Copy link

dantb commented Oct 7, 2021

Came here from diffx. Thanks for looking at this @ghostbuster91 and @KacperFKorban. Do we have a way forward or are we still blocked by scala/scala3#13146? Happy to help out if needed

@ghostbuster91
Copy link
Contributor Author

ghostbuster91 commented Oct 9, 2021

Hi @dantb

I created a new scala-3 migration branch (softwaremill/diffx#311) as the previous one got a little bit stale and messy. However, the old one was very useful as it showed some problematic areas. With regards to what is there still needed to be done please refer to the TODO list in the PR. Any help would be greatly appreciated :)

When it comes to blockers, auto-derivation seems to be out of the game, but semi-auto generally works.

@KacperFKorban Do you think that scala/scala3#13146 covers that problem with auto-derivation or should we create a separate issue?

@KacperFKorban
Copy link
Collaborator

I think that this dotty issue is unlikely to be solved in the near future. It seams like a limitation and not a bug. Maybe we will have to hack something on magnolia side.

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

No branches or pull requests

4 participants