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

Add tests that show #409 #410

Merged
merged 1 commit into from
Apr 18, 2016

Conversation

jvican
Copy link
Member

@jvican jvican commented Apr 13, 2016

No description provided.

@jvican jvican force-pushed the double-pickler-generation-traversable branch 2 times, most recently from 6082667 to 77c1a0d Compare April 13, 2016 19:11
@jvican
Copy link
Member Author

jvican commented Apr 13, 2016

Apparently, both the pickler and unpickler that are previously returned from lookup{Un}Pickler are created by a generator stored in genPicklerMap. I'm actually debugging why this is happening in the first place and where we put that generator (which is a generator of List). Also, I guess that these generators are used only because of reflection and we do use them also for generating static picklers, but we shouldn't. And if we do, we should cache them correctly and make them visible so that implicitly[Pickler[List[String]]] doesn't trigger the standard generation. I'm trying to find a slick way to solve this issue.

@jvican
Copy link
Member Author

jvican commented Apr 14, 2016

My diagnosis: when we look a pickler up, it doesn't only look it up in the picklerMap but the genPicklerMap. Before querying the last one, we extract the top level type and use it as the key, e.g. from List[String] we get List. Then, the lookup succeeds and returns the generator for List, which doesn't correspond to Pickler[List[String]]].

To solve this problem it's necessary to register the picklers/unpicklers that are created from the generators. Therefore, next time someone looks them up they will be in picklerMap. Also, I've added two new methods in DefaultPicklerRegistry that only query picklerMap/unpicklerMap, ignoring the generators.

@jvican jvican force-pushed the double-pickler-generation-traversable branch from 77c1a0d to 65c79d5 Compare April 16, 2016 13:59
@jvican
Copy link
Member Author

jvican commented Apr 16, 2016

Tests passing thanks to #411. Review by @jsuereth.

@jsuereth jsuereth merged commit c66aaf5 into scala:0.11.x Apr 18, 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.

2 participants