Bug 26249 – Support JSON.stringify

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Oliver Hunt]

Bug for tracking the stringify portion of bug# 20031

[Oliver Hunt]

Created attachment 31039 [details]
First pass at JSON.stringify

[Sam Weinig]

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.

[Oliver Hunt]

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

[Oliver Hunt]

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