Skip to content

Strict equality CanEqual derivation fails for classes in which type argument is not used for equality #14544

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

Closed
SimY4 opened this issue Feb 23, 2022 · 9 comments · Fixed by #14553

Comments

@SimY4
Copy link

SimY4 commented Feb 23, 2022

Compiler version

Scala 3.1.1

Minimized code

import scala.language.strictEquality

case class MyClass[A](value: String)(val a: A) derives CanEqual

class Something {}

val a = MyClass[Something]("some")(new Something())
val b = MyClass[Something]("some")(new Something())

println(a == b)

Output

Values of types Playground.MyClass[Playground.Something] and Playground.MyClass[Playground.Something] cannot be compared with == or !=.
I found:

    Playground.MyClass.derived$CanEqual[Playground.Something, Playground.Something](
      /* missing */summon[CanEqual[Playground.Something, Playground.Something]]
    )

But no implicit values were found that match type CanEqual[Playground.Something, Playground.Something].

Expectation

Something is not part of case class universal equality check so it shouldn't be part of multiversal equality constraint either

@SimY4 SimY4 added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 23, 2022
@SimY4 SimY4 changed the title string equality CanEqual derivation fails for classes in which type argument is not used for equality Strict equality CanEqual derivation fails for classes in which type argument is not used for equality Feb 23, 2022
@spamegg1
Copy link

I cannot reproduce it:

  scala
Welcome to Scala 3.1.1 (17.0.1, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                                       
scala> case class MyClass[A](value: String)(val a: A) derives CanEqual
// defined case class MyClass
                                                                                                                                       
scala> class Something {}
// defined class Something
                                                                                                                                       
scala> val a = MyClass[Something]("some")(new Something())
val a: MyClass[Something] = MyClass(some)
                                                                                                                                       
scala> val b = MyClass[Something]("some")(new Something())
val b: MyClass[Something] = MyClass(some)
                                                                                                                                       
scala> println(a == b)
true

@SimY4
Copy link
Author

SimY4 commented Feb 23, 2022

@spameggo you need to enable strict quality via:

import scala.language.strictEquality

@nicolasstucki
Copy link
Contributor

It seems that the code above is correctly complaining as there is no instance of CanEqual[Something, Something] in scope. This implicit is required because derives CanEqual generates the following given instance

given derived$CanEqual[L, R](implicit x$0: CanEqual[L, R]): CanEqual[MyClass[L], MyClass[R]] = CanEqual.derived

Note that under strict equality the comparison new Something == new Something will also not work without the CanEqual[Something, Something]. To make this code compile you can add

given CanEqual[Something, Something] = CanEqual.derived

or derive it on Somthing

class Something derives CanEqual {}

@nicolasstucki nicolasstucki added itype:invalid and removed stat:needs triage Every issue needs to have an "area" and "itype" label itype:bug labels Feb 24, 2022
@nicolasstucki
Copy link
Contributor

@odersky should derive generate givens with implicit parameters or using parameters?

- given derived$CanEqual[L, R](implicit x$0: CanEqual[L, R]): CanEqual[MyClass[L], MyClass[R]] = CanEqual.derived
+ given derived$CanEqual[L, R](using x$0: CanEqual[L, R]): CanEqual[MyClass[L], MyClass[R]] = CanEqual.derived

I would have expected a using.

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Feb 24, 2022
@SimY4
Copy link
Author

SimY4 commented Feb 24, 2022

@nicolasstucki @odersky I would argue that my understanding of strict equality feature was merely about making equality checks safer at compiler time, not to actually change the semantics of case class equality.

@SimY4
Copy link
Author

SimY4 commented Feb 24, 2022

To be fair, I was hoping that CanEqual for case classes would simply delegate to case class generated equality.

@bishabosha
Copy link
Member

bishabosha commented Feb 24, 2022

To be fair, I was hoping that CanEqual for case classes would simply delegate to case class generated equality.

It does, CanEqual has no methods to call, the == operation still delegates to equals, it just checks at compiletime if there exists the correct CanEqual in implicit scope

@SimY4
Copy link
Author

SimY4 commented Feb 27, 2022

Sorry for the perseverance, but what's the outcome here? I disagree that the feature works as expected for this case and CanEqual derivation can be improved. I'm happy to be proven wrong if you point me to the documentation explaining why

@bishabosha
Copy link
Member

Something is not part of case class universal equality check so it shouldn't be part of multiversal equality constraint either

ok so in this case you would want a feature change so that CanEqual derivation can analyse the equals method - this is hard in general, but making it specialised for this one use case of "if user does not define custom equals, and generic type parameter does not appear in primary constructor of case class" could be possible

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

Successfully merging a pull request may close this issue.

4 participants