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

Derivation documentation: use inline def derived so that derives is employed #16919

Closed
wants to merge 2 commits into from

Conversation

sideeffffect
Copy link
Contributor

@sideeffffect sideeffffect commented Feb 14, 2023

This PR complements the the issue

@@ -419,7 +419,7 @@ object Eq:
case ((x, y), elem) => check(elem)(x, y)
}

inline given derived[T](using m: Mirror.Of[T]): Eq[T] =
inline def derived[T](using m: Mirror.Of[T]): Eq[T] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there is a test case with this example that also needs to be updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a cursory search of "given derived" reveals quite a lot of instances, including in the docs/_docs/reference/contextual/derivation-macro.md document, which is derivation using macro quotes and splices instead of inline match

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sideeffffect
Copy link
Contributor Author

I have some sad news to share. The version with inline def fails to compile with

No given instance of type Playground.Eq[Playground.Opt.Sm[T]] was found

https://scastie.scala-lang.org/aT00re3sQ3WArpD8OCQtqw

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many more examples of given derived on both of these documentation pages, with these changes now the content is inconsistent

@bishabosha
Copy link
Member

I have some sad news to share. The version with inline def fails to compile with

No given instance of type Playground.Eq[Playground.Opt.Sm[T]] was found

https://scastie.scala-lang.org/aT00re3sQ3WArpD8OCQtqw

oops :O, would you like to try and fix the example?

@sideeffffect
Copy link
Contributor Author

sideeffffect commented Feb 15, 2023

I tried but failed :(
I first suspected that the derivation only works on regular types, but break on Opt because it has a type parameter and the derivation mechanism can't deal with that.
But it turns out that the derivation can't deal with plain enums either... maybe I'm doing something totally wrong?

enum Color derives Eq:
  case Black
  case White
No given instance of type Playground.Eq[(Playground.Color.Black : Playground.Color)] was found

https://scastie.scala-lang.org/DfysP6t9QHOs4EZV4YepwA

@bishabosha
Copy link
Member

bishabosha commented Feb 15, 2023

I tried and fixed the example:
https://scastie.scala-lang.org/M0XuMdp9TAaxKFADtcHHxA

the issue is that because Eq[T] is invariant, when it comes to deriving elemInstances, there will not exist given Eq[Color.Black.type] so in that case you have to explicitly invoke the derived method again. So you need to do another type test on the element to see if it is the recursive case.

the example also protects against infinite types such as this:

scala> case class Stream[+T](t: T, ts: Stream[T]) derives Eq
-- Error: ----------------------------------------------------------------------
1 |case class Stream[+T](t: T, ts: Stream[T]) derives Eq
  |                                                   ^
  |                                             infinite recursive derivation

I guess if someone wants to support this case they could modify it to pass down the root instance for reuse

@sideeffffect
Copy link
Contributor Author

sideeffffect commented Feb 15, 2023

@bishabosha you seem to know what you're talking about. You're probably much more qualified than me. Feel free to take over this Pull Request.

About the infinitely recursive type, it could still be useful, Eq can return false upon first different elements. So there could be utility in that and it would be great if the documentation explained how to implement derivation for types like this.

@bishabosha
Copy link
Member

I can take over

@bishabosha bishabosha self-assigned this Feb 15, 2023
@sideeffffect
Copy link
Contributor Author

Hello @nicolasstucki @bishabosha , are there any news regarding this issue?

@bishabosha
Copy link
Member

Hey yeah I'll work on this today

@bishabosha
Copy link
Member

closed in favour of #17414

@bishabosha bishabosha closed this May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants