Stage 3 Specification Review: Waldemar Horwat about proposal-json-parse-with-source

[waldemarhorwat]

Here's my review. I just have a couple editorial comments.

The ShallowestContained algorithm doesn't work in general. It may happen to work for the JSON subset, but it forms an attractive nuisance that hides land mines for others who might call it in the future. For example,

ShallowestContained called on the parse of "class C {x=4; [true]=8}" with types «NumericLiteral» doesn't find any numeric literals. But if we add searching for an unrelated nonterminal, it suddenly starts to find them:
ShallowestContained called on the parse of "class C {x=4; [true]=8}" with types «NumericLiteral, BooleanLiteral» finds the numeric literal 4.

ArrayElementList is a bit broken in general too. If elisions are present, the length of the list generated by ArrayElementList is not necessarily equal to the array's length because a single Elision can contain multiple commas but ArrayElementList treats it as a single element. However, the JSON syntax doesn't allow elisions, so it happens to work on JSON arrays.

Possible solutions are to either fix these so they work in general or rename them so it's clear they are only usable on JSON objects.

[gibson042]

The ShallowestContained algorithm doesn't work in general. It may happen to work for the JSON subset, but it forms an attractive nuisance that hides land mines for others who might call it in the future. For example,

ShallowestContained called on the parse of "class C {x=4; [true]=8}" with types «NumericLiteral» doesn't find any numeric literals. But if we add searching for an unrelated nonterminal, it suddenly starts to find them: ShallowestContained called on the parse of "class C {x=4; [true]=8}" with types «NumericLiteral, BooleanLiteral» finds the numeric literal 4.

Thank you, this is an excellent point. Please review #30, which renames the operation to ShallowestContainedJSONValue and narrows its use to JSON.parse.

ArrayElementList is a bit broken in general too. If elisions are present, the length of the list generated by ArrayElementList is not necessarily equal to the array's length because a single Elision can contain multiple commas but ArrayElementList treats it as a single element. However, the JSON syntax doesn't allow elisions, so it happens to work on JSON arrays.

This behavior is intentional, although the length assertion is indeed specific to JSON.parse. For clarity, I have renamed the operation to ArrayLiteralContentNodes in #31.