Floating-point NaN or Infinity values should be allowed as a feature

[Dani-Hub]

The JSON specification doesn't support special floating values such as any NaN value or +/-infinity. But many (if not most) Json libraries at least provide a special Flag to allow them. As an example see the flag serializeSpecialFloatingPointValues of the google-Gson library:

http://google-gson.googlecode.com/svn-history/trunk/gson/docs/javadocs/com/google/gson/GsonBuilder.html#serializeSpecialFloatingPointValues%28%29

It seems that the choice of representation chosen here (NaN for a NaN, Infinity and -Infinity for positive/negative infinity is quite popular and I recommend to use them when this feature is active.

Other JSON library with similar support:

https://github.com/aseemk/json5
https://simplejson.readthedocs.org/en/latest/

[cdunn2001]

Good idea. Would you like to implement this?

[Dani-Hub]

Sure, I will work on that.

[Dani-Hub]

While trying to start working, I'm wondering how to initialize a Writer object with Features. This support is needed to control the JSON output for infinities and NaNs.

According to the general documentation:

"This configuration object can be used to force the Reader or Writer to behave in a standard conforming way."

But only Reader objects can be constructed from a Features object.

What am I missing?

[cdunn2001]

Use neither Writer nor Features. Those are deprecated. (But if I add your macro, the compiler will get very noisy for our unit-tests.) Use StreamWriterBuilder. See the Advanced Usage section of the docs.

[Dani-Hub]

I would like to get some feedback regarding the design choices: The current trunk cannot handle NaN nor infinity for Visual Studio (presumably because of an implementation defect, because the gcc code writes +/-1e+9999 for infinities and null for NaN, but reads +/-infinity and 0, respectively) and a strict reading of the JSON spec means that these values are not valid. My suggestion would be that activating a special feature flag (such as "allowSpecialFloatingPointValues") will have the effect to output NaN, Infinity, -Infinity for these three value categories and will read them back to a floating point NaN or corresponding infinity values. But what should the behaviour be, if this flag is not set? Some libraries do not accept them while writing, some do not accept them while reading or writing and in such cases an exception is thrown. Or should this flag effectively allow three states: (a) The current behaviour (default), (b) exact mapping (as described above), or (c) hard error? I would like to collect some feedback to make acceptance of the intended feature more likely.

[cdunn2001]

We're changing 1e9999 to Infinity and null to NaN (for floats) only when the user has requested that feature. We will have a symmetrical feature in the Reader. We will not alter the current behavior when not set. And strict mode should follow the JSON spec.

For the feature's name, how about useSpecialFloats? It's a bit shorter.

Collecting feedback is a good idea! I hope you get some.

[Dani-Hub]

I completely agree with the first sentence, but I don't understand how to interpret "And strict mode should follow the JSON spec", because according to

http://www.ietf.org/rfc/rfc4627.txt

"Numeric values that cannot be represented as sequences of digits (such as Infinity and NaN) are not permitted."

My question was: How should that "not permitted" be implemented? This was the reason of my question related to exceptions.

[cdunn2001]

Strict mode will not include this new feature. In strict mode, the reader will reject any invalid symbol. Probably, it doesn't have to change at all.

• http://open-source-parsers.github.io/jsoncpp-docs/doxygen/class_json_1_1_char_reader_builder.html#a9c19e3c5475f9072d527810d4bf56749

As for how to reject, the reader returns false if it has a problem, and it shows the line of rejected input in the err string.

[Dani-Hub]

I'm still trying to figure out the architecture of the jsoncpp library to find the optimal way to implement this feature. I'm now stumbling across the valueToString functions, which seem to be public API, at least they can be referred to by the documentation:

http://open-source-parsers.github.io/jsoncpp-docs/doxygen/writer_8h.html

Given that these free function determine in a fixed way how primitive types (such as double) are converted into string forms, I don't understand how I could realize a different form of

std::string valueToString(double value);

that would produce the alternative output requested by this issue. I need some help to understand the choices that I would have for realizing an alternative double output (depending on a corresponding useSpecialFloats flag). Would it be feasible to provide a new function parameter with a default value such as:

std::string valueToString(double value, bool useSpecialFloats = false);

Why are these functions part of the public API anyway? Shouldn't they actually be moved to internal headers such as to header "json_tool.h"?

[cdunn2001]

valueToString() is public but should not be. (And we cannot remove it now.) It's ok to use a new internal-only alternative instead.

I'll be awk for a couple weeks, so take your time with your new ideas...

[Dani-Hub]

I'm following now all calls of valueToString(double) to potentially adapt them to invoke the correct output strategy (depending on the 'useSpecialFloats' flag) where appropriate (that is, excluding deprecated classes). This works quite well in StreamWriterBuilder where we have a settings_ member available, but it is completely unclear to me, how to read this setting flag within Json::Value, e.g. Value::asString(). Could you give me a hint, please? Thanks.

[cdunn2001]

Anyone calling Json::Value::asString() will be using a default writer. There is no way around that. (Not only would we have to modify Json::Value, which would break binary-compatibility, but we also would probably have to increase its size, which is a measurable penalty.)

That's fine. Just make it work for StreamWriterBuilder.

[Dani-Hub]

Somehow my commits have been registered under a previous issue (#214).

What is the AppVeyor error "Specify which project or solution file to use because the folder contains more than one project or solution file." supposed to mean?

From my point of view the implementation for #209 is completed, but I don't understand above mentioned errors

[cdunn2001]

Somehow my commits have been registered under a previous issue (#214).

You mentioned 209 in a comment for 214. Also, it's possible that one of your commit messages refers to 214. Not sure.

What is the AppVeyor error "Specify which project or solution file to use because the folder contains more than one project or solution file." supposed to mean?

Where are you seeing that? I recently registered an AppVeyor call-back. That should eventually run CI in AppVeyor in addition to Travis, but the appveyor.yml is not yet integrated. I don't see anything about appveyor in 209. Did you receive an e-mail? I'd like to see the exact message and how you received it.

In the meantime, I'll try a new PR of my own to see if it triggers a similar error.

From my point of view the implementation for #209 is completed...

If you want, open a new PR. Or at least, rebase to the tip of master and then git push -f ....

[Dani-Hub]

If you visit #217 and go down to the bottom of the page immediately after my last comment starting with "I was quite busy ..." you should see my very last commits. The last six of them are marked red and I don't understand the reason for this. Clicking on the details of the problem I'm referred to this page:

https://ci.appveyor.com/project/cdunn2001/jsoncpp/build/1.0.4

I don't understand the problems mentioned there. Locally - compiled against gcc mingw and Visual Studio I don't get any compile errors (but I see some cmake related errors which are not mine, since I have not touched any cmake files).

What do I need to do so make the last commits of mine available to you?

[Dani-Hub]

After a lengthy period being absent from this project I restarted and reset my fork to the head of the upstream project and retried with a new suggestion.

Essentially the implementation should be equal compared to my previous suggestion except that I no longer suggest a separate new (internal) header. Nonetheless I ensured that the two non-equivalent handling of snprintf within the reader and writer implementation is now equivalent (by copy and paste, because you didn't like the additional header). I verified that the preprocessor ifdeflogic regarding _MSC_VER does match VS 2015 (VC++ 14.0).

I hope my new start is now more readable in regard to git history line.

[Dani-Hub]

Shouldn't this issue be closed now?

[cdunn2001]

Yes, and thanks for your help.