Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Mirrors are not generated for Case Classes with multiple parameter lists #17777

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
Lasering opened this issue Feb 2, 2022 · 9 comments
Closed

Comments

@Lasering
Copy link

Lasering commented Feb 2, 2022

Compiler version

3.1.1

Minimized code

import scala.deriving.Mirror

case class Foo(a: Int, b: Int)
case class Bar1(a: Int)(b: Int)
case class Bar2(a: Int)(val b: Int)

summon[Mirror.Of[Foo]]
summon[Mirror.Of[Bar1]]
summon[Mirror.Of[Bar2]]

Scastie Link

Output

From scastie

no implicit argument of type deriving.Mirror.Of[Playground.Bar1] was found for parameter x of method summon in object Predef
no implicit argument of type deriving.Mirror.Of[Playground.Bar2] was found for parameter x of method summon in object Predef

Expectation

A Mirror should be generated for Bar1 and Bar2

@Lasering Lasering added the stat:needs triage Every issue needs to have an "area" and "itype" label label Feb 2, 2022
@Lasering Lasering changed the title Mirrors are not generated for Case Classes with multiple parameters Mirrors are not generated for Case Classes with multiple parameter lists Feb 2, 2022
@bishabosha
Copy link
Member

this is as expected the compiler. There is an explicit check for secondary argument lists in the constructor. The main problem is how do we implement a correct fromProduct method in the mirror for these cases. Also what would you expect for the MirroredElemTypes to look?

@bishabosha bishabosha added itype:enhancement area:typeclass-derivation and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 2, 2022
@Lasering
Copy link
Author

Lasering commented Feb 2, 2022

Ideally a tuple of tuples mapping to the parameter lists.

@joroKr21
Copy link
Member

But secondary parameter lists don't participate in hashCode, equals, unapply - I don't think it's a good idea in general

@joroKr21
Copy link
Member

Shapeless also has issues with this. A related problem is what happens with implicits like case class Foo[A: Order](value: A, label: String) - Shapeless 2 would capture the implicit Order[x] at the place where Generic[Foo[x]] is summoned. Which works but can be a gotcha.

@Lasering
Copy link
Author

But secondary parameter lists don't participate in hashCode, equals, unapply - I don't think it's a good idea in general

For example when deriving a Decoder for a class with multiple parameter lists I want to be able to create an instance of that class. Without knowing the types of the remaining parameter lists there is no way to do that.

It is known that parameters (vals or not) of secondary parameter lists are not used for the automatically generated hashCode, equals, unapply. Can you explain how that is affected/impacted by the inclusion of the secondary parameter lists on the Mirror?

@joroKr21
Copy link
Member

Consider that Mirror.Product is based on two functions - fromProduct and toProduct. Well toProduct for a case class is just identity because case classes are already products. However the fields that are considered part of this product are only the primary fields. Now if you include the secondary fields to fromProduct you get an inconsistency - it would not work correctly (i.e. fromProduct after toProduct no longer gives the original case class back.

And then there is the interaction with implicits that I already mentioned.

@Lasering
Copy link
Author

To make things consistent toProduct would have to also produce a tuple of tuples. But I'm still missing the relation to hashCode, equal, unapply are those implemented using the result of toProduct?

Is there other avenues that could be pursued? Maybe a specialized Mirror or toProductAll/fromProductAll

@joroKr21
Copy link
Member

All this stuff should be consistent (also with Mirror):

scala> case class Foo(x: Int)(val y: String)
// defined case class Foo

scala> val foo = Foo(42)("bar")
val foo: Foo = Foo(42)

scala> foo.hashCode
val res1: Int = 759270620

scala> import scala.util.hashing.MurmurHash3

scala> MurmurHash3.productHash(foo)
val res2: Int = 759270620

scala> foo.productArity
val res3: Int = 1

scala> foo.productIterator.toList
val res4: List[Any] = List(42)

@odersky
Copy link
Contributor

odersky commented Mar 20, 2022

I think all this goes too far. We have already exhausted the complexity and code size budget for mirrors and related stuff. I'll move to feature request, but don't think this will be adopted in the end.

@odersky odersky transferred this issue from scala/scala3 Mar 20, 2022
@ckipp01 ckipp01 transferred this issue from lampepfl/dotty-feature-requests Jun 5, 2023
@scala scala locked and limited conversation to collaborators Jun 5, 2023
@ckipp01 ckipp01 converted this issue into discussion #17778 Jun 5, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

4 participants