[webkit-reviews] review granted: [Bug 26591] Support revivers in JSON.parse : [Attachment 31698] JSON.parse with reviver support

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Oliver Hunt <oliver at apple.com>'s
request for review:
Bug 26591: Support revivers in JSON.parse
https://bugs.webkit.org/show_bug.cgi?id=26591

Attachment 31698: JSON.parse with reviver support
https://bugs.webkit.org/attachment.cgi?id=31698&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   (JSC::):

Please remove this garbage line inserted by the script.

> +    JSValue callReviver(JSValue unfiltered, JSValue property) {

Please move this brace onto a separate line.

> +	   JSValue args[] = {property, unfiltered};

Could you add spaces inside the braces please?

I'm worried that the arguments here are in the opposite order from the actual
JavaScript function.

> +		   array->setIndex(indexStack.last(), callReviver(outValue,
jsString(m_exec, UString::from(indexStack.last()))));

Gosh that UString::from is going to be slow!

> +    Walker texasRanger(exec, asObject(function), callType, callData);
> +    return texasRanger.walk(unfiltered);

You can just put this on one line and so you won't have to apologize to our
friends from Texas.

You forgot to include the expected results.

r=me if you fix these things