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

Caj/type traits gmp #56

Closed
wants to merge 6 commits into from
Closed

Caj/type traits gmp #56

wants to merge 6 commits into from

Conversation

ChrisJefferson
Copy link

My current progress on #51 . It isn't designed to be merged just yet, but I wanted to check the state before continuing.

There are minimal changes to picojson.h, mainly just tidying up some places where the type traits weren't used (refering to array and object), and some (in my opinion) slight neatening. In particular, there is no explicit reference to my header, which is nice.

The only thing which caused me trouble is that my type must be storable in a union, so must be a pointer. I added a pair of traits(return_type and to_return_type) which givea way of storing a pointer in the union (like object/array), or the raw object (like double already is).

I would like to be able to run all the tests for my interface, but that is proving irritatingly fiddly. One option would be to pull the tests out into a seperate file?

I could also look at abstracting out the int64 in a similar way, which would get all int64 out of picojson.h (at the cost of changing the current int64 interface, and making int64s a bit less efficient because of another layer. Depends how much / if at all you would like it out..)

@ChrisJefferson
Copy link
Author

Just to say, using this I have built a binding of picojson for the GAP (A mathematics system) which uses both a custom large-int numeric type, and a custom stream type (which I wrapped as a simple input iterator).

This all worked very successfully!

@kazuho
Copy link
Owner

kazuho commented Nov 21, 2014

Thank you for the great work! It looks mostly fine to me. I especially liked the idea of using two different types (value_type and return_type), since it reduces the number of copies done internally by picojson.

That said, I have two suggestions.

  1. IMO we should better renaming return_type to value_type, and value_type to internal_type. The idea is that the former should have a familiar name since it is the type that is exposed to the users.
  2. At https://github.com/ChrisJefferson/picojson/blob/caj/type-traits-gmp/picojson.h#L173:
typedef TraitsT::number_traits::value_type value;

I would appreciate it if you could either a) do not typedef it or b) name it number_type since we might want to use traits for types other than number.

How do they sound?

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