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

Inconsistent export of case classes' synthetic methods #13228

Closed
tabdulradi opened this issue Aug 1, 2021 · 5 comments · Fixed by #13234
Closed

Inconsistent export of case classes' synthetic methods #13228

tabdulradi opened this issue Aug 1, 2021 · 5 comments · Fixed by #13234
Assignees
Milestone

Comments

@tabdulradi
Copy link

Compiler version

3.0.1

Minimized code

case class User(name: String)

case class RegisteredUser(id: String, data: User) {
  export data.*
}

@main def app() = 
  println(RegisteredUser("Id", User("Name"))) // RegisteredUser(Name)
  println(RegisteredUser("Id", User("Name")).canEqual(User("Name"))) // True
  // The rest works as expected
  println(RegisteredUser("Id", User("Name")) == User("Name")) // False
  println(RegisteredUser("Id", User("Name")).hashCode == User("Name").hashCode) // False

Output

RegisteredUser(Name)
true
false
false

Expectation

RegisteredUser(Id, User(Name))
false
false
false

According to @bishabosha

So what's happening here is that productArity and productElement are delegated to data field, (those methods are called from toString) but equals, hashCode, toString are not exported. (problematic that canEqual is also exported)

and
@kubukoz

Yeah, strange. toString defers to productPrefix I think, that should probably have been exported as well...

[1] https://twitter.com/bishabosha/status/1421803014722052098
[2] https://twitter.com/kubukoz/status/1421810894967906304

@odersky
Copy link
Contributor

odersky commented Aug 1, 2021

This looks according to do spec to me. canEqual gets a forwarder since it is defined in User. Since it has a forwarder it won't be defined again in RegisteredUser. toString and hashCode and equals are different since they already exist as members of RegisteredUser.

Summary: When dealing with case classes, stay away from wildcard exports. The results might be unintuitive. As usual, explicit is better.

@odersky odersky closed this as completed Aug 1, 2021
@tabdulradi
Copy link
Author

Can we at least issue a compiler warning for wildcard exports of case classes? Seems it might be a source of many confusions.

@odersky
Copy link
Contributor

odersky commented Aug 1, 2021

I think that would be a job for a linter. Compiler warnings have diminishing and at some point negative returns.

@smarter
Copy link
Member

smarter commented Aug 1, 2021

I think we do have a problem here because the result is not consistent under separate compilation, if I split the example into a file containing just:

case class User(name: String)

which I then compile, followed by compiling the rest of the example I get:

RegisteredUser(Id,User(Name))
false
false
false

because the PostTyper phase added synthetic definitions for canEqual/productXXX and wildcard exports filters synthetic definitions out: https://github.com/lampepfl/dotty/blob/9e8d5c55153fdbe0d3f78d661cb3b51113403128/compiler/src/dotty/tools/dotc/typer/Namer.scala#L1159

@smarter smarter reopened this Aug 1, 2021
@odersky
Copy link
Contributor

odersky commented Aug 1, 2021

@smarter Fair point :-)

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