Skip to content
This repository has been archived by the owner on Feb 20, 2019. It is now read-only.

Add support for specialized tuples #428

Merged
merged 2 commits into from
May 25, 2016

Conversation

jvican
Copy link
Member

@jvican jvican commented May 15, 2016

Add extendible way to pickle specialized classes

  • Reuse code in RuntimePicklerRegistry and sourcegen.
  • Create ClassMapper to delegate the tasks of checking class equality.

Add tests for Tuple2 (normal and specialized)

  • Now, we support pickling and unpickling specialized tuples. It's necessary
    to add a special method to check class equality since the specialized
    classes are implemented as subclasses and therefore have its own class
    name, failing a simple class equality.

Now, we support pickling and unpickling specialized tuples. It's
necessary to add a special method to check class equality since the
specialized classes are implemented as subclasses and therefore have its
own class name, failing a simple class equality.
@jvican jvican force-pushed the add-specialized-tuple-support branch from 9c9af01 to 809db6a Compare May 15, 2016 00:56
/* Map specialized classes to classes. Canonical use case: tuples.
* We map classes instead of strings to check at runtime that they exist. */
val specialMappingClasses: Map[Class[_], Class[_]] =
mapSpecializedTuplesFor("scala.Tuple2") // add also other special cases
Copy link
Contributor

Choose a reason for hiding this comment

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

We may be able to make this more generic, i forget the rules fo r specialization mangling

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing! Actually, I'd like to make it more generic, but first i think we should find another compelling use case 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

I've found out how specialization mangling works, but i think we shouldn't include it now and wait for a more compelling use case.

@jsuereth
Copy link
Contributor

I love the idea of this and how much cleaner it is! I think it'd be even cleaner if it were in the picklerregistry.

@jvican
Copy link
Member Author

jvican commented May 15, 2016

Definitely, thanks for the tip, will do that when i get some time!

* Reuse code in `RuntimePicklerRegistry` and `sourcegen`.

* Create `ClassMapper` to delegate the tasks of checking class equality.
@jvican jvican force-pushed the add-specialized-tuple-support branch 2 times, most recently from 4e0b03d to caf8730 Compare May 23, 2016 14:25
@jvican
Copy link
Member Author

jvican commented May 23, 2016

This PR has been updated with your suggestions @jsuereth

@@ -111,7 +117,8 @@ private[pickling] trait SourceGenerator extends Macro with tags.FastTypeTagMacro
case Some(b) =>
val parentTpe = x.parent.tpe[c.universe.type](c.universe)
val impl = generatePickleImplFromAst(b)
q"""if(classOf[$parentTpe] == picklee.getClass) $impl else $subclasses"""
val checkIfParent = q"$classMapper.areSameClasses(picklee.getClass, classOf[$parentTpe])"
q"""if($checkIfParent) $impl else $subclasses"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this already handled in the CaseDef?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but they're used for different use cases. The one that solves my problem is this one, but for the sake of correctness I decided to put it in the CaseDef as well. IIRC, this one takes care of the superclass, the case def of the subclasses.

@jsuereth
Copy link
Contributor

One quick question, otherwise LGTM

@jsuereth jsuereth merged commit d10ccee into scala:0.11.x May 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants