Bug 26249 - Support JSON.stringify
Summary: Support JSON.stringify
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on: 26640
Blocks: 20031
  Show dependency treegraph
 
Reported: 2009-06-07 21:53 PDT by Oliver Hunt
Modified: 2009-06-22 22:12 PDT (History)
0 users

See Also:


Attachments
First pass at JSON.stringify (76.63 KB, patch)
2009-06-07 22:48 PDT, Oliver Hunt
sam: 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-07 21:53:01 PDT
Bug for tracking the stringify portion of bug# 20031
Comment 1 Oliver Hunt 2009-06-07 22:48:55 PDT
Created attachment 31039 [details]
First pass at JSON.stringify
Comment 2 Sam Weinig 2009-06-08 22:00:48 PDT
Comment on attachment 31039 [details]
First pass at JSON.stringify

The code looks good, but I think it could use a little sprucing up in the way of whitespace/paragraphing.  If you would, please go through and add some whitespace to make it more in line with the rest of the code.  A few specifics inline.

> +
> +class Stringifier {
> +    typedef UString StringBuilder;
> +    // Basically the maximum nesting accepted by firefox.

Perhaps given the complexity of this Stringifier class, we should put it in its own header file.

> +    }
> +    class StringKeyGenerator {
> +    public:
> +        StringKeyGenerator(const Identifier& property) : m_property(property) {}
> +        JSValue getKey(ExecState* exec) { if (!m_key) m_key = jsString(exec, m_property.ustring()); return m_key;  }
> +    private:
> +        Identifier m_property;
> +        JSValue m_key;
> +    };
> +    class IntKeyGenerator {
> +    public:
> +        IntKeyGenerator(uint32_t property) : m_property(property) {}
> +        JSValue getKey(ExecState* exec) { if (!m_key) m_key = jsNumber(exec, m_property); return m_key;  }
> +    private:
> +        uint32_t m_property;
> +        JSValue m_key;
> +    };

A little whitespace in and around these classes could help paragraph them a little better.

> +            return jsValue;
> +
> +        JSValue list[] = { jsKey.getKey(m_exec) };        
> +        ArgList args(list, sizeof(list) / sizeof(JSValue));
> +        return call(m_exec, object,callType, callData, jsValue, args);
Missing space after object,.


> +        return true;
> +    }
> +    bool stringifyObject(StringBuilder& builder, JSObject* object) {

{ on the wrong line.  Please put whitespace between functions as well.

> +
> +// ECMA-262 v5 15.12.3
> +JSValue JSC_HOST_CALL JSONProtoFuncStringify(ExecState* exec, JSObject*, JSValue thisValue, const ArgList& args)
> +{
> +    UNUSED_PARAM(thisValue);

I believe you can just elide the name thisValue in the prototype and avoid the need for this UNUSED_PARAM

r=me.
Comment 3 Oliver Hunt 2009-06-08 23:04:45 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/DerivedSources.make
	M	JavaScriptCore/GNUmakefile.am
	M	JavaScriptCore/JavaScriptCore.pri
	M	JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj
	M	JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
	M	JavaScriptCore/interpreter/CallFrame.h
	M	JavaScriptCore/runtime/CommonIdentifiers.h
	M	JavaScriptCore/runtime/JSGlobalData.cpp
	M	JavaScriptCore/runtime/JSGlobalData.h
	M	JavaScriptCore/runtime/JSGlobalObject.cpp
	A	JavaScriptCore/runtime/JSONObject.cpp
	A	JavaScriptCore/runtime/JSONObject.h
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/js/JSON-stringify-expected.txt
	A	LayoutTests/fast/js/JSON-stringify.html
	A	LayoutTests/fast/js/resources/JSON-stringify.js
	A	LayoutTests/fast/js/resources/json2-es5-compat.js
Committed r44521
Comment 4 Oliver Hunt 2009-06-09 17:37:48 PDT
Recommitting following rollout.  Should now work on windows, etc

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/DerivedSources.make
	M	JavaScriptCore/GNUmakefile.am
	M	JavaScriptCore/JavaScriptCore.pri
	M	JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj
	M	JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
	M	JavaScriptCore/interpreter/CallFrame.h
	M	JavaScriptCore/runtime/CommonIdentifiers.h
	M	JavaScriptCore/runtime/JSGlobalData.cpp
	M	JavaScriptCore/runtime/JSGlobalData.h
	M	JavaScriptCore/runtime/JSGlobalObject.cpp
	A	JavaScriptCore/runtime/JSONObject.cpp
	A	JavaScriptCore/runtime/JSONObject.h
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/js/JSON-stringify-expected.txt
	A	LayoutTests/fast/js/JSON-stringify.html
	A	LayoutTests/fast/js/resources/JSON-stringify.js
	A	LayoutTests/fast/js/resources/json2-es5-compat.js
Committed r44550