Reviver can modify holder object, like inserting new object / array, which breaks invariant of parsing information

[Constellation]

Let's have this example,

print(JSON.stringify(JSON.parse("[1, 2]", function (name, value, context) {
    if (name === "0")
        this[1] = [ "Hello" ];
    return this[name];
})));

JSON.stringify's result is [1,["Hello",["Hello"]]] before this proposal because reviver modified array[1] at runtime and we visit this newly inserted ["Hello"] array. But for this array, we do not have correct corresponding parse information since this is not coming from text. source information for this property is still 2's information. Thus,

i. Assert: typedValNode is an ArrayLiteral.

This will crash since this newly inserted array does not have right source information. Source information for this property says 2 (it is coming from the original source).

[Constellation]

We observed invariant failures in JSC's prototype implementation, and we found that V8 crashes with the above example with --harmony-json-parse-with-source

[syg]

This looks like ultimately the same root cause as #35, which was also noticed during V8's implementation.

@gibson042 @mathiasbynens Needless to say this and #35 is a hard blocker for shipping.

[gibson042]

Nice find, @Constellation! This is a counterexample to our claim that bottom-up invocation renders moot the concern about divergence between live values and the source text from which they are derived, and will require a normative change to fix. I'll make a PR to take a shallow snapshot of arrays and other objects and suppress source in the context passed to the reviver for entries that have diverged and raise it for approval at the next TC39 plenary. We'll also want to cover this with test cases:

• [array and plain object] overwrite of not-yet-processed entry should result in absence of source for that entry
  • primitive to non-primitive
  • non-primitive to primitive
  • non-primitive to a clone (same date but different identity)
  • primitive to a different primitive
• [array and plain object] overwrite and restoration of not-yet-processed entry should preserve source for that entry
  • primitive
  • non-primitive
• [array] splicing out an item should result in absence of source from the affected index forward
  • ...but restoring items at their original index should preserve source

[mhofman]

What about deletion and recreation of an already processed object property? Values would be the same but order wouldn't.

[mhofman]

Would it be web compatible to use the snapshot of objects and arrays for the entries passed to the reviver, and ignore mutations past snapshot time?

I mean which legitimate code mutates the object through the reviver?

[gibson042]

What about deletion and recreation of an already processed object property? Values would be the same but order wouldn't.

My plan is to preserve source in that case:

const reorderedSource = JSON.parse(
  '{ "foo": 0, "bar": 1, "baz": 2 }',
  function reviver(key, value, {source}) {
    if (key === "foo") {
      // Reposition the "bar" property.
      const barVal = this.bar;
      delete this.bar;
      this.bar = barVal;
    }
    return key ? source : value;
  },
);
assert.strictEqual(reorderedSource.bar, "1");

Would it be web compatible to use the snapshot of objects and arrays for the entries passed to the reviver, and ignore mutations past snapshot time?

I mean which legitimate code mutates the object through the reviver?

I don't think it would be (as evidenced in part by these two issues), and at any rate such a change is out of scope for this proposal.