Re: [Jsoncpp-devel] Backwards incompatible change in jsoncpp 64-bit support

[Baptiste L. <bap...@gm...>]

2011/5/23 Aaron Jacobs <ja...@go...>

> On Mon, May 23, 2011 at 9:50 PM, Baptiste Lepilleur
> <bap...@gm...> wrote:
> > If I understood your problem correctly, this should solve your backward
> > compatibility issue. Is that correct?
>
> Almost but not quite, I think. Now the numbers in [2^32, 2^64) will be
> parsed as uint64s, and the code above will always return kError
> because both isUInt and isDouble return false, even though it used to
> work for those values. I think maybe the only choice here is to have
> each of the isFoo methods return true iff asFoo will work, whether
> that's the best way to represent the number or not. For example:
>
> 0.7 -> isDouble
> 1 -> isUInt, isInt, isUInt64, isInt64, isDouble
> 2^32 -> isUInt, isUInt64, isDouble
> 2^40 -> isUInt64, isDouble
> 2^64 - 1 -> isDouble
> 2^64 -> isDouble
>
> I can imagine that this change would cause problems for other existing
> code, but I'm not sure. What do you think?
>

This is what I had in mind when I said that "isUInt() should succeed if the
value is a positive signed int Value". I think it moves the API in the right
direction: there should always be a way to know if a function will throw
before calling it.

In term of backward compatibility I would not expect too many issue. The
most impacting change will likely be that 1 will return true for all
numeric isUInt, isInt, isUInt64, isInt64, isDouble while before it only
returned true for either isUInt or isInt depending on how it was
constructed. When you look at the current behavior, it really sound buggy...

I'm curious has existing code that would break with such change to the
semantic of isInt() and isUInt()...


>
> Perhaps the best way would have been to have users explicitly opt in
> for 64-bit support. That is, add an option to the constructor of
> Json::Reader that defaults to false, and if you set it to true then
> you're asserting that your code doesn't suffer from the problem
> mentioned above. I guess it's too late for that, though.
>

It is not too late. It should not be difficult to add a data member to
Features instance passed to the Reader to control its behavior that would
indicate if integer greater than 32 bits should be stored as double or int
in the Json::Value. Feel free to add this behavior, it should not be too
difficult.

That being said, when I look back at your code sample:
   unsigned int value =
       value.isUInt() ? value.asUInt() :
           (value.isDouble() ? value.asDouble() : kError);

I can not help but feel that something is wrong. If value does not fit on a
32 bits integer, then you will get mostly a random value after conversion to
double followed by a cast to unsigned int. Am I missing something?

Baptiste.