Parse NaN from object like {"n": NaN}

[ajdavis]

Parsing is case-insensitive. Closes #24.

[ajdavis]

This is ready for an initial review. "make check" passes, but there's not yet any test for the new NaN parsing. The next steps are to control this NaN parsing with a compile-time or runtime option, and to add a test for it that expects success or failure depending on whether NaN parsing is enabled.

Would you like to do those steps, or tell me how I should do them?

[mnunberg]

else if

[ajdavis]

Actually it's two "!=" statements, so CUR_CHAR might be neither 'a', 'N', nor 'n', I think we have to check both if statements.

[mnunberg]

FYI this table is generated by a Perl script. Better to modify it there.

[mnunberg]

likewise

[mnunberg]

JSONSL_PARSE_NAN would be the compiler directive.

Regarding tests - all existing parse tests load from a file. You'd make a new file and simply feed a [nan] or similar, verifying it parses successfully. This reminds me that I should add better tests for type-specific valdiation (right now all our tests do is ensure that things parse successfully (or not)).

[mnunberg]

oops. didn't mean to close

[ajdavis]

I see the tests didn't work on Travis as they did on my machine, investigating....

[ajdavis]

OK, this pull request is ready for review.

[mnunberg]

Can you squash these into a single commit?

Also, I'm still a bit uneasy about having the NaN parsing around there as dead code even if it's not being compiled that way. I have another idea. How about making a proxy variable.. e.g.

(in the character table for special_begin):

JSONSL_SPECIALf_NULL|JSONSL__NAN_PROXY

Then..

#ifdef JSONSL_PARSE_NAN
#define JSONSL__NAN_PROXY JSONSL_SPECIALf_NAN
#else
#define JSONSL__NAN_PROXY 0
#endif

You can even go one step further and only define the enum value if PARSE_NAN is defined (there's no reason for the NAN value to be there otherwise).. though I define the macros using the xmacro pattern.. so I'm not sure tht's easily modifiable.

This way, the current (pre-nan) code can remain the same, and newer code/branches can be added on only if PARSE_NAN is defined. The way you have it currently, it will run through some cycles determining NAN stuff even if nan parsing is disabled. My desire to have this be a compile time option is less about one of configuration and more about not impacting performance (even theoretically) for less common situations (granted, null isn't super common either..).

So basically:

#ifdef JSONSL_PARSE_NAN
else if (state->special_flags & JSONSL_SPECIALf_NAN) {
    // ...
}
#endif

Because _NAN_PROXY is 0 if PARSE_NAN is not defined, the character table will only yield JSONSL_SPECIALf_NULL on n and nothing on N.

[ajdavis]

Done, let me know if I followed your instructions correctly.

[mnunberg]

yep! exactly what I had in mind. thanks for the contribution!