Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Json constructors for int64 and uint32 / uint64 types? #16

Open
lehni opened this issue Oct 11, 2014 · 12 comments
Open

Json constructors for int64 and uint32 / uint64 types? #16

lehni opened this issue Oct 11, 2014 · 12 comments

Comments

@lehni
Copy link

lehni commented Oct 11, 2014

First of all, congrats on this great little library! I've been looking for a lightweight modern implementation for a while now, really happy to finally have found it.

The only thing that I was a bit irritated by was that there isn't any support for int64 and uint32 / 64 types.

Json doesn't make that distinction, but if you want a non-float value that is larger than 32 bits, or an unsigned int without loosing the values from 1 << 15 and beyond, then you currently have to go through double, which seems odd.

The simple solution could be to internally store all such values as long (in a new JsonLong class), not int, and then do the casting in int_value(), long_value() uint_value() (or unsigned_int_value() if you like it verbose), ulong_value(), ...) But I guess this would still cause troubles when exporting, as it will have to be known if a value was signed or unsigned.

@skabbes
Copy link
Contributor

skabbes commented Nov 16, 2014

I believe that the reasoning here is that JSON can't actually support anything larger that 53 bit integers. If your use case is timestamps, double has plenty of precision as clarified in this comment

json11/json11.hpp

Lines 24 to 28 in 7103522

* Fortunately, double-precision IEEE754 ('double') can precisely store any integer in the
* range +/-2^53, which includes every 'int' on most systems. (Timestamps often use int64
* or long long to avoid the Y2038K problem; a double storing microseconds since some epoch
* will be exact for +/- 275 years.)
*/

Storing as a 32 bit integer (as either signed or unsigned) will be fine, since doubles have more than enough precision to represent this.

uint32_t value = static_cast<uint32_t>( json_thing.number_value() )

will always be safe, but casting to a 64 bit integer type will cause issues with values larger than 53 bits.

So, in my opinion, 64 bit types are a non-goal of this library, and 32 bits types are simply syntactic sugar (which would be argued for either way).

@lehni
Copy link
Author

lehni commented Nov 16, 2014

Alright, that all makes sense, thanks for clarifying. The syntactic sugar for unsigned 32 bit could still be nice though?

@skabbes
Copy link
Contributor

skabbes commented Nov 18, 2014

I would probably agree, but I'm not the library maintainer :)

@skifire1983
Copy link

skifire1983 commented May 16, 2016

@skabbes and @lehni made a pull request #61
(also signed the SLA).

@taozhijiang
Copy link

Yes, I store uint64 session_id, but can not store with the precise. I am wondering whether there are any other methods to improve this.

string str1 = R"({"message_id":105308320612483198,"msg_type":3,"order":0,"ques_id":0,"session_id":105308187502051928,"site_id":122062,"text":"XXXXXXX","timestamp":1471140271,"visitor_id":9941658010949867158,"worker_id":133746})";
string err;
auto json = json11::Json::parse(str1, err);
cout << (unsigned long long) (json["message_id"].number_value()) << endl;

the value is 105308320612483200
maybe I can ignore some bits....

@artwyman
Copy link
Contributor

If you need fidelity for a full 64 bits, your best bet is to encode your IDs as strings rather than numbers. Or modify your own fork with something like #61 if you can accept the cross-language compatibility concerns discussed there.

@taozhijiang
Copy link

@artwyman thanks.
I checked, the #61 implement may have some problem, It can not reach int64_max, int64_min, uint64_max limits. I forked and enhanced at https://github.com/taozhijiang/json11

With out the consider like javascript, I think int64_t and uint_64 are very important at most cases. I suggest create another branch may a better solution. ;-)

@artwyman
Copy link
Contributor

Maintaining a separate branch would add some maintenance overhead, though admittedly less confusingly than multiple forks. It might be better to provide both options controlled by ifdefs, or as distinct types in the same library, though I'd be interested to hear @j4cbo's opinion on the matter.

@fosterbrereton
Copy link

fosterbrereton commented Sep 1, 2016

I understand json11 is limited to 53-bit integers because it might round-trip them through double - but that is a limitation of the implementation, not of the JSON specification. (If I am mistaken here and missed something in the JSON spec, please let me know. Otherwise...)

I think a proper implementation would use intmax_t to store integer values and separate integer storage from double. Support for implicit casting between intmax_t and double should still be available, with the implied understanding that there would be cases of loss in precision (which is already the case, only going from double -> int.)

@artwyman
Copy link
Contributor

artwyman commented Sep 2, 2016

@fosterbrereton That's an intentional limitation for compatibility, as described here:

json11/json11.hpp

Lines 16 to 22 in 7103522

* A note on numbers - JSON specifies the syntax of number formatting but not its semantics,
* so some JSON implementations distinguish between integers and floating-point numbers, while
* some don't. In json11, we choose the latter. Because some JSON implementations (namely
* Javascript itself) treat all numbers as the same type, distinguishing the two leads
* to JSON that will be *silently* changed by a round-trip through those implementations.
* Dangerous! To avoid that risk, json11 stores all numbers as double internally, but also
* provides integer helpers.

It's not about the JSON spec (which doesn't say anything about numeric sizes) but about compatibility with all major implementations, including those such as JavaScript which treat all numbers as doubles. We designed json11 for network traffic, and want it to produce and consume JSON which can be exchanged with any other implementation.

@fosterbrereton
Copy link

If everything has to be limited to what can round-trip through a double, then, why add all the extra code to support int? The library would be cleaner and more correct with respect to the intent of supporting double for JavaScript's sake.

Separating storage for int and double implies they should be treated differently to take advantage of each's benefits (which would be great). It's no surprise, then, that library users might misinterpret the library's intent and file bug reports and/or feature requests in kind.

@rektide
Copy link

rektide commented Jan 16, 2017

It's not about the JSON spec (which doesn't say anything about numeric sizes) but about compatibility with all major implementations, including those such as JavaScript which treat all numbers as doubles.

But JavaScript will implicity convert to double! Ignoring the spec and lowering precision deliberately because one implementation has a funky numerical typing system is really a frustrating resolution to this ticket. Further, in the future JS is expected to have 64 bit ints & uints (https://github.com/tc39/ecmascript_simd &c)! Exercising this nuclear option now will could significantly harm the usability of this library in that future.

uint64_t support would be really really really nice, and it should totally happen. JavaScript will work just fine if json11 does this, it'll just lose precision, like JavaScript does when you are > MAX_SAFE_INTEGER. Which is fine and well known and JS developers (raise hand) know they have to cope with this. Please don't limit this lib artificially.

This would be very useful for implementing PowerDNS/pdns#4915 for example.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants