Bug 26591 - Support revivers in JSON.parse
Summary: Support revivers in JSON.parse
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks: 26587
  Show dependency treegraph
 
Reported: 2009-06-21 16:10 PDT by Oliver Hunt
Modified: 2009-06-22 18:28 PDT (History)
0 users

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2009-06-21 16:10:03 PDT
As in title
Comment 1 Oliver Hunt 2009-06-22 18:11:25 PDT
Created attachment 31698 [details]
JSON.parse with reviver support
Comment 2 Darin Adler 2009-06-22 18:18:09 PDT
Comment on attachment 31698 [details]
JSON.parse with reviver support

> +        (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 Oliver Hunt 2009-06-22 18:28:21 PDT
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