[ES5] Arguments object should inherit from Array

https://bugs.webkit.org/show_bug.cgi?id=28298

Reviewed by Gavin Barraclough

Make the Arguments object conform to the behaviour specified in ES5.
The simple portion of this is to make Arguments use Array.prototype
as its prototype rather than Object.prototype.

The spec then requires us to set instance.constructor to the pristine
Object constructor, and instance.toString and instance.toLocaleString
to the pristine versions from Object.prototype.  To do this we now
make the ObjectPrototype constructor return its toString and
toLocaleString functions (similar to the call and apply functions
from FunctionPrototype).

Oddly enough this reports itself as a slight win, but given the code
isn't hit in the tests that claim to have improved I put this down to
code motion.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@47292 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 5211117..a7673f7 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -131,6 +131,45 @@
 
 2009-08-14  Oliver Hunt  <oliver@apple.com>
 
+        Reviewed by Gavin Barraclough.
+
+        [ES5] Arguments object should inherit from Array
+        https://bugs.webkit.org/show_bug.cgi?id=28298
+
+        Make the Arguments object conform to the behaviour specified in ES5.
+        The simple portion of this is to make Arguments use Array.prototype
+        as its prototype rather than Object.prototype.
+
+        The spec then requires us to set instance.constructor to the pristine
+        Object constructor, and instance.toString and instance.toLocaleString
+        to the pristine versions from Object.prototype.  To do this we now 
+        make the ObjectPrototype constructor return its toString and
+        toLocaleString functions (similar to the call and apply functions
+        from FunctionPrototype).
+
+        Oddly enough this reports itself as a slight win, but given the code
+        isn't hit in the tests that claim to have improved I put this down to
+        code motion.
+
+        * runtime/Arguments.h:
+        (JSC::Arguments::Arguments):
+        (JSC::Arguments::initializeStandardProperties):
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::reset):
+        (JSC::JSGlobalObject::markChildren):
+        * runtime/JSGlobalObject.h:
+        (JSC::JSGlobalObject::JSGlobalObjectData::JSGlobalObjectData):
+        (JSC::JSGlobalObject::objectConstructor):
+        (JSC::JSGlobalObject::objectToStringFunction):
+        (JSC::JSGlobalObject::objectToLocaleStringFunction):
+        * runtime/ObjectPrototype.cpp:
+        (JSC::ObjectPrototype::ObjectPrototype):
+        * runtime/ObjectPrototype.h:
+        * tests/mozilla/ecma_3/Function/arguments-001.js:
+          Update test to new es5 behaviour
+
+2009-08-14  Oliver Hunt  <oliver@apple.com>
+
         Reviewed by NOBODY (Build fix).
 
         Remove MarkStack::drain from the JSC exports file

diff --git a/JavaScriptCore/runtime/Arguments.h b/JavaScriptCore/runtime/Arguments.h
index 79fe720..bfc8f00 100644 (file)
--- a/JavaScriptCore/runtime/Arguments.h
+++ b/JavaScriptCore/runtime/Arguments.h
@@ -28,6 +28,7 @@
 #include "JSFunction.h"
 #include "JSGlobalObject.h"
 #include "Interpreter.h"
+#include "ObjectConstructor.h"
 
 namespace JSC {
 
@@ -98,6 +99,7 @@ namespace JSC {
         virtual const ClassInfo* classInfo() const { return &info; }
 
         void init(CallFrame*);
+        void initializeStandardProperties(CallFrame*);
 
         OwnPtr<ArgumentsData> d;
     };
@@ -131,6 +133,7 @@ namespace JSC {
         : JSObject(callFrame->lexicalGlobalObject()->argumentsStructure())
         , d(new ArgumentsData)
     {
+        initializeStandardProperties(callFrame);
         JSFunction* callee;
         ptrdiff_t firstParameterIndex;
         Register* argv;
@@ -169,7 +172,8 @@ namespace JSC {
         , d(new ArgumentsData)
     {
         ASSERT(!callFrame->callee()->body()->parameterCount());
-
+        
+        initializeStandardProperties(callFrame);
         unsigned numArguments = callFrame->argumentCount() - 1;
 
         d->numParameters = 0;
@@ -237,6 +241,13 @@ namespace JSC {
         return asArguments(jsValue());
     }
     
+    
+    inline void Arguments::initializeStandardProperties(CallFrame* callFrame)
+    {
+        putDirectFunction(callFrame->propertyNames().constructor, callFrame->lexicalGlobalObject()->objectConstructor(), DontEnum);
+        putDirectFunction(callFrame->propertyNames().toString, callFrame->lexicalGlobalObject()->objectToStringFunction(), DontEnum);
+        putDirectFunction(callFrame->propertyNames().toLocaleString, callFrame->lexicalGlobalObject()->objectToLocaleStringFunction(), DontEnum);
+    }
 
 } // namespace JSC
 

diff --git a/JavaScriptCore/runtime/JSGlobalObject.cpp b/JavaScriptCore/runtime/JSGlobalObject.cpp
index eece719..4268a54 100644 (file)
--- a/JavaScriptCore/runtime/JSGlobalObject.cpp
+++ b/JavaScriptCore/runtime/JSGlobalObject.cpp
@@ -210,18 +210,22 @@ void JSGlobalObject::reset(JSValue prototype)
     d()->functionPrototype->addFunctionProperties(exec, d()->prototypeFunctionStructure.get(), &callFunction, &applyFunction);
     d()->callFunction = callFunction;
     d()->applyFunction = applyFunction;
-    d()->objectPrototype = new (exec) ObjectPrototype(exec, ObjectPrototype::createStructure(jsNull()), d()->prototypeFunctionStructure.get());
+    NativeFunctionWrapper* objectToStringFunction = 0;
+    NativeFunctionWrapper* objectToLocaleStringFunction = 0;
+    d()->objectPrototype = new (exec) ObjectPrototype(exec, ObjectPrototype::createStructure(jsNull()), d()->prototypeFunctionStructure.get(), &objectToStringFunction, &objectToLocaleStringFunction);
+    d()->objectToStringFunction = objectToStringFunction;
+    d()->objectToLocaleStringFunction = objectToLocaleStringFunction;
     d()->functionPrototype->structure()->setPrototypeWithoutTransition(d()->objectPrototype);
 
     d()->emptyObjectStructure = d()->objectPrototype->inheritorID();
 
     d()->functionStructure = JSFunction::createStructure(d()->functionPrototype);
     d()->callbackFunctionStructure = JSCallbackFunction::createStructure(d()->functionPrototype);
-    d()->argumentsStructure = Arguments::createStructure(d()->objectPrototype);
     d()->callbackConstructorStructure = JSCallbackConstructor::createStructure(d()->objectPrototype);
     d()->callbackObjectStructure = JSCallbackObject<JSObject>::createStructure(d()->objectPrototype);
 
     d()->arrayPrototype = new (exec) ArrayPrototype(ArrayPrototype::createStructure(d()->objectPrototype));
+    d()->argumentsStructure = Arguments::createStructure(d()->arrayPrototype);
     d()->arrayStructure = JSArray::createStructure(d()->arrayPrototype);
     d()->regExpMatchesArrayStructure = RegExpMatchesArray::createStructure(d()->arrayPrototype);
 
@@ -256,7 +260,7 @@ void JSGlobalObject::reset(JSValue prototype)
 
     // Constructors
 
-    JSCell* objectConstructor = new (exec) ObjectConstructor(exec, ObjectConstructor::createStructure(d()->functionPrototype), d()->objectPrototype, d()->prototypeFunctionStructure.get());
+    ObjectConstructor* objectConstructor = new (exec) ObjectConstructor(exec, ObjectConstructor::createStructure(d()->functionPrototype), d()->objectPrototype, d()->prototypeFunctionStructure.get());
     JSCell* functionConstructor = new (exec) FunctionConstructor(exec, FunctionConstructor::createStructure(d()->functionPrototype), d()->functionPrototype);
     JSCell* arrayConstructor = new (exec) ArrayConstructor(exec, ArrayConstructor::createStructure(d()->functionPrototype), d()->arrayPrototype, d()->prototypeFunctionStructure.get());
     JSCell* stringConstructor = new (exec) StringConstructor(exec, StringConstructor::createStructure(d()->functionPrototype), d()->prototypeFunctionStructure.get(), d()->stringPrototype);
@@ -270,6 +274,7 @@ void JSGlobalObject::reset(JSValue prototype)
 
     RefPtr<Structure> nativeErrorStructure = NativeErrorConstructor::createStructure(d()->functionPrototype);
 
+    d()->objectConstructor = objectConstructor;
     d()->evalErrorConstructor = new (exec) NativeErrorConstructor(exec, nativeErrorStructure, evalErrorPrototype);
     d()->rangeErrorConstructor = new (exec) NativeErrorConstructor(exec, nativeErrorStructure, rangeErrorPrototype);
     d()->referenceErrorConstructor = new (exec) NativeErrorConstructor(exec, nativeErrorStructure, referenceErrorPrototype);
@@ -368,7 +373,8 @@ void JSGlobalObject::markChildren(MarkStack& markStack)
     RegisterFile& registerFile = globalData()->interpreter->registerFile();
     if (registerFile.globalObject() == this)
         registerFile.markGlobals(markStack, &globalData()->heap);
-
+    
+    markIfNeeded(markStack, d()->objectConstructor);
     markIfNeeded(markStack, d()->regExpConstructor);
     markIfNeeded(markStack, d()->errorConstructor);
     markIfNeeded(markStack, d()->evalErrorConstructor);
@@ -381,6 +387,8 @@ void JSGlobalObject::markChildren(MarkStack& markStack)
     markIfNeeded(markStack, d()->evalFunction);
     markIfNeeded(markStack, d()->callFunction);
     markIfNeeded(markStack, d()->applyFunction);
+    markIfNeeded(markStack, d()->objectToStringFunction);
+    markIfNeeded(markStack, d()->objectToLocaleStringFunction);
 
     markIfNeeded(markStack, d()->objectPrototype);
     markIfNeeded(markStack, d()->functionPrototype);

diff --git a/JavaScriptCore/runtime/JSGlobalObject.h b/JavaScriptCore/runtime/JSGlobalObject.h
index cda49bd..4dcaa76 100644 (file)
--- a/JavaScriptCore/runtime/JSGlobalObject.h
+++ b/JavaScriptCore/runtime/JSGlobalObject.h
@@ -40,6 +40,7 @@ namespace JSC {
     class FunctionPrototype;
     class GlobalEvalFunction;
     class NativeErrorConstructor;
+    class ObjectConstructor;
     class ProgramCodeBlock;
     class PrototypeFunction;
     class RegExpConstructor;
@@ -60,6 +61,7 @@ namespace JSC {
                 : JSVariableObjectData(&symbolTable, 0)
                 , registerArraySize(0)
                 , globalScopeChain(NoScopeChain())
+                , objectConstructor(0)
                 , regExpConstructor(0)
                 , errorConstructor(0)
                 , evalErrorConstructor(0)
@@ -71,6 +73,8 @@ namespace JSC {
                 , evalFunction(0)
                 , callFunction(0)
                 , applyFunction(0)
+                , objectToStringFunction(0)
+                , objectToLocaleStringFunction(0)
                 , objectPrototype(0)
                 , functionPrototype(0)
                 , arrayPrototype(0)
@@ -99,6 +103,7 @@ namespace JSC {
 
             int recursion;
 
+            ObjectConstructor* objectConstructor;
             RegExpConstructor* regExpConstructor;
             ErrorConstructor* errorConstructor;
             NativeErrorConstructor* evalErrorConstructor;
@@ -111,6 +116,8 @@ namespace JSC {
             GlobalEvalFunction* evalFunction;
             NativeFunctionWrapper* callFunction;
             NativeFunctionWrapper* applyFunction;
+            NativeFunctionWrapper* objectToStringFunction;
+            NativeFunctionWrapper* objectToLocaleStringFunction;
 
             ObjectPrototype* objectPrototype;
             FunctionPrototype* functionPrototype;
@@ -183,6 +190,7 @@ namespace JSC {
         // The following accessors return pristine values, even if a script 
         // replaces the global object's associated property.
 
+        ObjectConstructor* objectConstructor() const { return d()->objectConstructor; }
         RegExpConstructor* regExpConstructor() const { return d()->regExpConstructor; }
 
         ErrorConstructor* errorConstructor() const { return d()->errorConstructor; }
@@ -204,6 +212,9 @@ namespace JSC {
         DatePrototype* datePrototype() const { return d()->datePrototype; }
         RegExpPrototype* regExpPrototype() const { return d()->regExpPrototype; }
 
+        NativeFunctionWrapper* objectToStringFunction() const { return d()->objectToStringFunction; }
+        NativeFunctionWrapper* objectToLocaleStringFunction() const { return d()->objectToLocaleStringFunction; }
+
         JSObject* methodCallDummy() const { return d()->methodCallDummy; }
 
         Structure* argumentsStructure() const { return d()->argumentsStructure.get(); }

diff --git a/JavaScriptCore/runtime/ObjectPrototype.cpp b/JavaScriptCore/runtime/ObjectPrototype.cpp
index 98e4713..42b0013 100644 (file)
--- a/JavaScriptCore/runtime/ObjectPrototype.cpp
+++ b/JavaScriptCore/runtime/ObjectPrototype.cpp
@@ -40,11 +40,15 @@ static JSValue JSC_HOST_CALL objectProtoFuncLookupSetter(ExecState*, JSObject*,
 static JSValue JSC_HOST_CALL objectProtoFuncPropertyIsEnumerable(ExecState*, JSObject*, JSValue, const ArgList&);
 static JSValue JSC_HOST_CALL objectProtoFuncToLocaleString(ExecState*, JSObject*, JSValue, const ArgList&);
 
-ObjectPrototype::ObjectPrototype(ExecState* exec, PassRefPtr<Structure> stucture, Structure* prototypeFunctionStructure)
+ObjectPrototype::ObjectPrototype(ExecState* exec, PassRefPtr<Structure> stucture, Structure* prototypeFunctionStructure, NativeFunctionWrapper** toStringFunction, NativeFunctionWrapper** toLocaleStringFunction)
     : JSObject(stucture)
 {
-    putDirectFunctionWithoutTransition(exec, new (exec) NativeFunctionWrapper(exec, prototypeFunctionStructure, 0, exec->propertyNames().toString, objectProtoFuncToString), DontEnum);
-    putDirectFunctionWithoutTransition(exec, new (exec) NativeFunctionWrapper(exec, prototypeFunctionStructure, 0, exec->propertyNames().toLocaleString, objectProtoFuncToLocaleString), DontEnum);
+    NativeFunctionWrapper* toString = new (exec) NativeFunctionWrapper(exec, prototypeFunctionStructure, 0, exec->propertyNames().toString, objectProtoFuncToString);
+    NativeFunctionWrapper* toLocaleString = new (exec) NativeFunctionWrapper(exec, prototypeFunctionStructure, 0, exec->propertyNames().toLocaleString, objectProtoFuncToLocaleString);
+    *toStringFunction = toString;
+    *toLocaleStringFunction = toLocaleString;
+    putDirectFunctionWithoutTransition(exec, toString, DontEnum);
+    putDirectFunctionWithoutTransition(exec, toLocaleString, DontEnum);
     putDirectFunctionWithoutTransition(exec, new (exec) NativeFunctionWrapper(exec, prototypeFunctionStructure, 0, exec->propertyNames().valueOf, objectProtoFuncValueOf), DontEnum);
     putDirectFunctionWithoutTransition(exec, new (exec) NativeFunctionWrapper(exec, prototypeFunctionStructure, 1, exec->propertyNames().hasOwnProperty, objectProtoFuncHasOwnProperty), DontEnum);
     putDirectFunctionWithoutTransition(exec, new (exec) NativeFunctionWrapper(exec, prototypeFunctionStructure, 1, exec->propertyNames().propertyIsEnumerable, objectProtoFuncPropertyIsEnumerable), DontEnum);

diff --git a/JavaScriptCore/runtime/ObjectPrototype.h b/JavaScriptCore/runtime/ObjectPrototype.h
index 7790ae0..ea0f0de 100644 (file)
--- a/JavaScriptCore/runtime/ObjectPrototype.h
+++ b/JavaScriptCore/runtime/ObjectPrototype.h
@@ -27,7 +27,7 @@ namespace JSC {
 
     class ObjectPrototype : public JSObject {
     public:
-        ObjectPrototype(ExecState*, PassRefPtr<Structure>, Structure* prototypeFunctionStructure);
+        ObjectPrototype(ExecState*, PassRefPtr<Structure>, Structure* prototypeFunctionStructure, NativeFunctionWrapper** toStringFunction, NativeFunctionWrapper** toLocaleStringFunction);
     };
 
     JSValue JSC_HOST_CALL objectProtoFuncToString(ExecState*, JSObject*, JSValue, const ArgList&);

diff --git a/JavaScriptCore/tests/mozilla/ecma_3/Function/arguments-001.js b/JavaScriptCore/tests/mozilla/ecma_3/Function/arguments-001.js
index 98aca18..56803d0 100644 (file)
--- a/JavaScriptCore/tests/mozilla/ecma_3/Function/arguments-001.js
+++ b/JavaScriptCore/tests/mozilla/ecma_3/Function/arguments-001.js
@@ -49,11 +49,11 @@ expect = true;
 addThis();
 
 actual = a instanceof Array;
-expect = false;
+expect = true;
 addThis();
 
 actual = a.length;
-expect = undefined;
+expect = 0;
 addThis();
 
 
@@ -65,11 +65,11 @@ expect = true;
 addThis();
 
 actual = a instanceof Array;
-expect = false;
+expect = true;
 addThis();
 
 actual = a.length;
-expect = undefined;
+expect = 0;
 addThis();
 
 actual = a[0];

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index d0959db..52e1592 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,23 @@
+2009-08-14  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Gavin Barraclough.
+
+        [ES5] Arguments object should inherit from Array
+        https://bugs.webkit.org/show_bug.cgi?id=28298
+
+        Tests that an arguments object is created with the correct
+        properties and prototype (per ES5), and that the are always
+        the initial "pristine" versions.
+
+        * fast/js/arguments-expected.txt:
+        * fast/js/resources/arguments.js:
+        (shouldBe.getArguments):
+        (Object):
+        (Array):
+        (originalObject.prototype.toString):
+        (originalObject.prototype.toLocaleString):
+        (Array.prototype):
+
 2009-08-14  Eric Carlson  <eric.carlson@apple.com>
 
         Reviewed by Adam Roben.

diff --git a/LayoutTests/fast/js/arguments-expected.txt b/LayoutTests/fast/js/arguments-expected.txt
index 0970334..3ac3f28 100644 (file)
--- a/LayoutTests/fast/js/arguments-expected.txt
+++ b/LayoutTests/fast/js/arguments-expected.txt
@@ -129,6 +129,21 @@ PASS argumentsFunctionConstructorParam(true) is true
 FAIL argumentsVarUndefined() should be undefined. Was [object Arguments]
 FAIL argumentsConstUndefined() should be undefined. Was [object Arguments]
 PASS argumentCalleeInException() is argumentCalleeInException
+PASS getArguments() instanceof Object is true
+PASS getArguments() instanceof Array is true
+PASS getArguments().constructor is Object
+PASS Object.getPrototypeOf(getArguments()) is Array.prototype
+PASS getArguments().toString is Object.prototype.toString
+PASS getArguments().toLocaleString is Object.prototype.toLocaleString
+PASS getArguments() instanceof originalObject is true
+PASS getArguments() instanceof originalArray is true
+PASS getArguments().constructor is originalObject
+PASS originalObject.getPrototypeOf(getArguments()) is originalArray.prototype
+PASS getArguments().toString is originalObject.prototype.toString
+PASS getArguments().toLocaleString is originalObject.prototype.toLocaleString
+PASS originalObject.getPrototypeOf(getArguments()) is originalArrayPrototype
+PASS getArguments().toString is originalObjectToString
+PASS getArguments().toLocaleString is originalObjectToLocaleString
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/LayoutTests/fast/js/resources/arguments.js b/LayoutTests/fast/js/resources/arguments.js
index 5810af7..41e955b 100644 (file)
--- a/LayoutTests/fast/js/resources/arguments.js
+++ b/LayoutTests/fast/js/resources/arguments.js
@@ -534,4 +534,36 @@ function argumentCalleeInException() {
 }
 shouldBe("argumentCalleeInException()", "argumentCalleeInException")
 
+function getArguments() {
+    return arguments;
+}
+
+shouldBeTrue("getArguments() instanceof Object");
+shouldBeTrue("getArguments() instanceof Array");
+shouldBe("getArguments().constructor", "Object");
+shouldBe("Object.getPrototypeOf(getArguments())", "Array.prototype");
+shouldBe("getArguments().toString", "Object.prototype.toString");
+shouldBe("getArguments().toLocaleString", "Object.prototype.toLocaleString");
+
+var originalObject = Object;
+Object = function NewObject() {};
+var originalArray = Array;
+Array = function NewArray() {};
+shouldBeTrue("getArguments() instanceof originalObject");
+shouldBeTrue("getArguments() instanceof originalArray");
+shouldBe("getArguments().constructor", "originalObject");
+shouldBe("originalObject.getPrototypeOf(getArguments())", "originalArray.prototype");
+shouldBe("getArguments().toString", "originalObject.prototype.toString");
+shouldBe("getArguments().toLocaleString", "originalObject.prototype.toLocaleString");
+
+var originalObjectToString = originalObject.prototype.toString;
+originalObject.prototype.toString = function NewObjectPrototypeToString() {};
+var originalObjectToLocaleString = originalObject.prototype.toLocaleString;
+originalObject.prototype.toLocaleString = function NewObjectPrototypeToLocaleString() {};
+var originalArrayPrototype = originalArray.prototype;
+Array.prototype = function NewArrayPrototype() {};
+shouldBe("originalObject.getPrototypeOf(getArguments())", "originalArrayPrototype");
+shouldBe("getArguments().toString", "originalObjectToString");
+shouldBe("getArguments().toLocaleString", "originalObjectToLocaleString");
+
 var successfullyParsed = true;