Bug 26591 - Support revivers in JSON.parse
: Support revivers in JSON.parse
Status: RESOLVED FIXED
: WebKit
JavaScriptCore
: 528+ (Nightly build)
: Macintosh Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
: 26587
  Show dependency treegraph
 
Reported: 2009-06-21 16:10 PST by
Modified: 2009-06-22 18:28 PST (History)


Attachments
JSON.parse with reviver support (14.45 KB, patch)
2009-06-22 18:11 PST, Oliver Hunt
darin: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-06-21 16:10:03 PST
As in title
------- Comment #1 From 2009-06-22 18:11:25 PST -------
Created an attachment (id=31698) [details]
JSON.parse with reviver support
------- Comment #2 From 2009-06-22 18:18:09 PST -------
(From update of attachment 31698 [details])
> +        (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
------- Comment #3 From 2009-06-22 18:28:21 PST -------
Committing to http://svn.webkit.org/repository/webkit/trunk ...
    M    JavaScriptCore/ChangeLog
    M    JavaScriptCore/runtime/JSONObject.cpp
    M    LayoutTests/ChangeLog
    M    LayoutTests/fast/js/JSON-parse-expected.txt
    M    LayoutTests/fast/js/resources/JSON-parse.js
Committed r44968