Added validating JSON parser mode to parser.

Added validating JSON parser mode to parser.

[Lasse Reichstein, 2010-01-29 17:08:17]

Review, plase.

[Mads Ager (chromium), 2010-02-01 08:42:19]

LGTM

http://codereview.chromium.org/549207/diff/1/6
File src/parser.cc (right):

http://codereview.chromium.org/549207/diff/1/6#newcode212
src/parser.cc:212: bool* is_simple_out,
Remove the _out prefix on these out params?

http://codereview.chromium.org/549207/diff/1/6#newcode218
src/parser.cc:218: bool* is_simple_out,
Remove _out on these as well?

http://codereview.chromium.org/549207/diff/1/6#newcode3955
src/parser.cc:3955: void Parser::BuildObjectLiteralConstantProperties(
This is used for normal parsing of object literals as well, right?  Move before
the JSON comment?

http://codereview.chromium.org/549207/diff/1/7
File src/parser.h (right):

http://codereview.chromium.org/549207/diff/1/7#newcode137
src/parser.h:137: bool is_json = false);
Can we use the validation state enum here instead to make calls to MakeAST
easier to read?

http://codereview.chromium.org/549207/diff/1/9
File src/scanner.h (right):

http://codereview.chromium.org/549207/diff/1/9#newcode424
src/scanner.h:424: Token::Value ScanJsonNumber();
For each of these Scan functions, please add a comment explaining (at least some
of the ways in which) JSON scanning differs from JavaScript scanning.  Also,
there are probably spec references that would make sense to add here for quick
reference.

[Rico, 2010-02-01 10:40:58]

http://codereview.chromium.org/549207/diff/1/8
File src/scanner.cc (right):

http://codereview.chromium.org/549207/diff/1/8#newcode692
src/scanner.cc:692: if (c0_ < '0' || c0_ > '9') return Token::ILLEGAL;
According to spec this can actually be 0 (ExponentPart is used here and allows
arbitrary DecimalDigits) - although I agree that this does not make much sense.

[Rico, 2010-02-01 11:38:58]

It seems that after applying this patch json objects are no longer recognized,
i.e., I can't do this:
JSON.parse('{foo: 42}'

[Rico, 2010-02-01 11:59:01]

Sorry, according to spec you are of course right that the above should not
compile since the name of the key in the key-value pair must be a JSONString
which must be enclosed in ""

On 2010/02/01 11:38:58, Rico wrote:
> It seems that after applying this patch json objects are no longer recognized,
> i.e., I can't do this:
> JSON.parse('{foo: 42}'

[Lasse Reichstein, 2010-02-01 12:17:30]

http://codereview.chromium.org/549207/diff/1/6
File src/parser.cc (right):

http://codereview.chromium.org/549207/diff/1/6#newcode212
src/parser.cc:212: bool* is_simple_out,
Done.

http://codereview.chromium.org/549207/diff/1/6#newcode218
src/parser.cc:218: bool* is_simple_out,
Done, too.

http://codereview.chromium.org/549207/diff/1/6#newcode3955
src/parser.cc:3955: void Parser::BuildObjectLiteralConstantProperties(
Done.

http://codereview.chromium.org/549207/diff/1/8
File src/scanner.cc (right):

http://codereview.chromium.org/549207/diff/1/8#newcode692
src/scanner.cc:692: if (c0_ < '0' || c0_ > '9') return Token::ILLEGAL;
This code does allow a zero digit after the "e".
As for sense ... I guess you can't rule it out in automatically generated code
JSON.

In retrospect, I should probably have used an IsDigit predicate for readablity.