Allow negative hexadecimal literals again

[aseemk]

I just hooked up Travis CI support, which is great for testing across multiple versions of Node, and indeed, one of our test cases fails on Node 0.9:

https://travis-ci.org/aseemk/json5/jobs/4405590

The test case is parsing -0xC8, a negative hexadecimal literal. I narrowed the problem down to these lines in the parser:

https://github.com/aseemk/json5/blob/v0.1.0/lib/json5.js#L154-L156

number = +string;
if (!isFinite(number)) {
    error("Bad number");

The problem is that converting the string -0xC8 to a number via the unary + operator used to work, but now returns NaN. This changed from Node 0.8 to 0.9.

I investigated, and indeed, this is a breaking change in V8, but a fix technically:

http://code.google.com/p/v8/issues/detail?id=2240

I'm not sure yet what this means for JSON5, but we have one failing test today on Node 0.9. We don't document that negative hexadecimal literals are supported anywhere, so I think we're fine there, and the parser correctly rejects them, but we should change the test. Not sure how to do this without causing test failures on Node 0.6 and 0.8.

/cc @MaxNanasy FYI =)

[aseemk]

As commit 9cf34ad's message shows, I was able to test this easily since ES5 doesn't throw an error on negative hexadecimal literals -- it just evaluates them to NaN. Fixed.

[aseemk]

Released in 0.2.0 FYI!

https://github.com/aseemk/json5/blob/develop/CHANGELOG.md#v020-code-diff

[rlidwka]

I don't entirely understand why signed hexadecimals are not valid.

This seem to work just fine for me:

> eval('(function(){"use strict";return -0xC8})()')
-200

[jordanbtucker]

I wanted to open this back up for discussion, clarify some things, and see if we want to change this behavior.

TL;DR
This issue was originally a bug because of a change in V8. Instead of fixing the bug, the feature was removed even though the feature didn't really have anything to do with the V8 bug. It was the implementation of the feature that needed a fix.

Signed numbers vs numeric literals

Signed numbers, whether decimal or hexadecimal, are not treated as literals when parsed by ES5. They are parsed as two separate tokens: a Punctuator (+ or - in this case) and a NumericLiteral (an unsigned decimal or hexadecimal number).

That sequence produces a UnaryExpression using the + or - operators. In either case, the text of the NumericLiteral is converted to a number using ES5's ToNumber operation, and in the case of the - operator, the mathematical value is negated.

The discrepancy between signed decimals and signed hexadecimals occurs within the ToNumber(String) operation. It allows signed decimal numbers but prohibits signed hexadecimal numbers, returning NaN instead. Compare the following. (The global Number function uses the ToNumber operation):

> [  eval('1'),   eval('0x1'),   eval('-1'),   eval('-0x1')]
[ 1, 1, -1, -1 ]

> [Number('1'), Number('0x1'), Number('-1'), Number('-0x1')]
[ 1, 1, -1, NaN ]

The V8 bug

The + unary operator with a String argument (i.e. +'1.234') also applies the ToNumber operation, which is what the V8 bug was about. Expressions like +'-0xF' and Number('-0xF') were resulting in -15 instead of NaN as called for in the spec. Note that this is separate from the actual unary expression -0xF; this is the string '-0xF' being converted to a number.

Conclusion

Signed hexadecimals aren't evil, nor are they disallowed in ES5. They just can't be converted from strings for whatever reason the ES5 authors had. This really has nothing to do with JSON5.

JSON5 has freed us from the restrictions of JSON so that we can have a lightweight data-interchange format that is even easier for humans to read and write. Why disallow signed hexadecimals just because ES5 doesn't convert them from strings?

[aseemk]

Great argument @jordanbtucker, and fantastically thorough explanation! Thanks.

I'm convinced: let's re-add support for negative hexadecimals. Updated the issue title.

Now we just have to figure out how, right? =)

[aseemk]

Fixed by @jordanbtucker in pull #74.

[aseemk]

Oops, I'd forgotten to close this.