Skip to content

Tests to Explore Typeclass Derivation #5497

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 13 commits into from
Nov 27, 2018

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Nov 22, 2018

Two different typeclass derivation schemes in typeclass-derivation{1,2}.scala. The second scheme is preferable: it generates more efficient code and needs fewer (generated or hand-written) instance definitions.

This is a first stab at the problem; there are lots of opportunities for optimizations.

They are redundant, and can cause a cyclic reference when trying to
evaluate a lazy body annotation. We did filter them before, but not
everywhere.
 - package case bindings with reduced expression
 - don't map wildcards to TypeTrees
This one resembles Magnolia, in that it uses some degree of reflection
@odersky odersky changed the title Some inliner fixes Tests to Explore Typeclass Derivation Nov 23, 2018
... and some polishings
We could not reduce a pattern with wildcard type arguments because
these were not treated as GADT bound variables but as abstract types.
@odersky
Copy link
Contributor Author

odersky commented Nov 23, 2018

@milessabin: https://github.com/dotty-staging/dotty/tree/inline-for-derive/tests/run/typeclass-derivation2.scala is a test case for first stab at typeclass derivation. It would be good to have your feedback on this.

It can happen that what is really a constant type is still in the form of
an unreduced MatchType. We need to normalize the type to reveal the constant.

The new version typeclass-derivation2.scala test needs this fix to compile
without errors.
@odersky
Copy link
Contributor Author

odersky commented Nov 26, 2018

@nicolasstucki The PR contains some smallish fixes to the inliner that makes the tests compile correctly. Can you do a quick review of these?

@nicolasstucki
Copy link
Contributor

The changes in the Inliner look good.

Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

The changes in the compiler look good. I'll le @milessabin give feedback on the derivation test cases/example.

@milessabin
Copy link
Contributor

@odersky I think this looks good as a proof of concept ... and it satisfies me that inline/match gives us a good set of primitives to work with. I have some issues with the encodings you've used (both forms), but I think, as you say, it's a good starting point.

I'm going to take a stab at eliminating (or at least reducing the number of) handwritten folds, seeing if we can make (as much as possible of) the intermediate representation erasable, and generalizing to type classes indexed by higher kinded types (eg. Functor).

@milessabin
Copy link
Contributor

@odersky I'm happy for this to be merged as-is, given that it contains needed fixes.

@milessabin
Copy link
Contributor

Ahh ... I'm the one assigned ... I'll merge :-)

@milessabin milessabin merged commit 1ba082a into scala:master Nov 27, 2018
@liufengyun liufengyun deleted the inline-for-derive branch November 27, 2018 15:33
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