Skip to content

ClassCastException when summoning Mirror for hierarchical sum compiled by 3.0.x #13777

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
bishabosha opened this issue Oct 20, 2021 · 10 comments · Fixed by #14035
Closed

ClassCastException when summoning Mirror for hierarchical sum compiled by 3.0.x #13777

bishabosha opened this issue Oct 20, 2021 · 10 comments · Fixed by #14035
Assignees
Labels
area:implicits related to implicits itype:bug
Milestone

Comments

@bishabosha
Copy link
Member

bishabosha commented Oct 20, 2021

Compiler version

3.1.0

Minimized code

compile with Scala 3.0.2 to out dir

sealed trait Top
object Top // companion is necessary

case class Middle() extends Top with Bottom
sealed trait Bottom extends Top

compile with Scala 3.1.0 and with out on classpath

val m = summon[deriving.Mirror.SumOf[Top]]

Output

runtime error:

java.lang.ClassCastException: Top$ cannot be cast to scala.deriving.Mirror$Sum
  ... 28 elided

This happens because Top is an eligible sum, and it has a companion object (which if compiled by Scala 3.1.0 would extend deriving.Mirror.Sum), so the companion object is used as the mirror and is cast to Sum.

Expectation

prevent summoning mirrors for hierarchical sums with companion objects compiled before 3.1.0

@dwijnand
Copy link
Member

Expectation

prevent summoning mirrors for hierarchical sums with companion objects compiled before 3.1.0

I'd say don't use the companion object as the mirror if it doesn't extend Mirror.Sum, but rather summon a local mirror. Any reason that can't be made to work?

@bishabosha
Copy link
Member Author

bishabosha commented Oct 20, 2021

I'd say don't use the companion object as the mirror if it doesn't extend Mirror.Sum, but rather summon a local mirror. Any reason that can't be made to work?

This sounds very reasonable to me.

Summarising a Slack discussion:
@dwijnand and @julienrf also asked if we can check if the companion is a subtype of Mirror.Sum,
I said this is not possible with code compiled from source, as mirror synthesis happens too early in the compiler currently,
so @dwijnand suggested that if we can check if a type is a compiled dependency, then check that its companion extends Mirror.Sum, else keep the current behavior.

this change can be implemented in SymUtils.useCompanionAsMirror

@bishabosha bishabosha changed the title ClassCastException when summoning Mirror for hierarchical sum compiled by 3.0.2 ClassCastException when summoning Mirror for hierarchical sum compiled by 3.0.x Oct 20, 2021
@bishabosha bishabosha added the area:implicits related to implicits label Oct 20, 2021
@smarter smarter added this to the 3.1.1 milestone Oct 20, 2021
@dwijnand
Copy link
Member

Btw, someone would've hit this during the RC cycle if they had a CI setup to test RCs.

@dwijnand
Copy link
Member

@bishabosha you fixing this? Otherwise I'll take it.

@SethTisue
Copy link
Member

someone would've hit this during the RC cycle if they had a CI setup to test RCs

should the community build have caught it?

@smarter
Copy link
Member

smarter commented Oct 20, 2021

Currently, the community build rebuilds all transitive dependencies from sources, this is a great showcase of why we may want to also have a version of the community build which keeps the binary dependencies.

@bishabosha
Copy link
Member Author

@dwijnand you are very welcome to take this on

@smarter
Copy link
Member

smarter commented Dec 2, 2021

@dwijnand are you still planning to work on this? It'd be great to get this in 3.1.1 which should be out soon as far as I can tell.

@dwijnand
Copy link
Member

dwijnand commented Dec 2, 2021

I haven't forgotten it but I'm happy to hand it off if someone wants it.

bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 3, 2021
@bishabosha
Copy link
Member Author

@dwijnand I have added a test case on the branch https://github.com/dotty-staging/dotty/tree/fix-13777

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:implicits related to implicits itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants