Skip to content

Improvements to Typeclass Derivation #5839

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

Merged
merged 3 commits into from
Feb 8, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 4, 2019

  • Add a Generic instance only if the implementation of a derived
    typeclass mentions the type.

  • Allow derivations for typeclasses with more or less type parameters than 1.

@odersky
Copy link
Contributor Author

odersky commented Feb 4, 2019

Based on a bunch of other in flight PRs in order to avoid rebases once these are in.

@odersky
Copy link
Contributor Author

odersky commented Feb 4, 2019

Only last two commits are new.

 1. add a Generic instance only if the implementation of a derived
    typeclass mentions the type.
@@ -210,14 +212,33 @@ trait Deriving { this: Typer =>
addDerivedInstance(defn.GenericType.name, genericCompleter, codePos, reportErrors = false)
}

/** If any of the instances has a companion with a `derived` member
Copy link
Contributor

Choose a reason for hiding this comment

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

docs/reference/derivation.md should be update to reflect that change. I think it would make sense to keep this under specified and say that Generic instance are only guaranteed to be cached when writing derives Generic.

@@ -161,33 +161,67 @@ trait Deriving { this: Typer =>
*
* implicit def derived$D(implicit ev_1: D[T_1], ..., ev_n: D[T_n]): D[C[Ts]] = D.derived
*
* See test run/typeclass-derivation2 for examples that spell out what would be generated.
* Note that the name of the derived method containd the name in the derives clause, not
* See the body of this method for who to generalize this to typeclasses with more
Copy link
Contributor

Choose a reason for hiding this comment

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

s/who/how/

if (nparams == 0) Nil
else if (nparams == 1) tparam :: Nil
else typeClass.typeParams.map(tcparam =>
tparam.copy(name = s"${tparam.name}_${tcparam.name}".toTypeName)
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if this generates two identical names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How could that happen? Both parameter lists use disjoint names.

Copy link
Contributor

@OlivierBlanvillain OlivierBlanvillain Feb 7, 2019

Choose a reason for hiding this comment

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

  • class Foo[A_B, A]
  • class Bar[_B_C, _C]

yields

  • A_B_B_C
  • A_B_C
  • A_B_C
  • A_C

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. We should use a reserved separator then. Maybe _$_?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do the change in the follow-up PR #5843

@odersky odersky merged commit f31a876 into scala:master Feb 8, 2019
@allanrenucci allanrenucci deleted the change-derive branch February 8, 2019 19:45
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.

2 participants