Revision: 4738
Author: [email protected]
Date: Thu May 27 00:43:43 2010
Log: Add support for getOwnPropertyDescriptor on array indices (fixes issue
599).
This fix adds support for retriving a property descriptor on elements. The
new version supports both fast and slow case elements. In the fast case
we always default configurable, writable, enumerable to true (we don't have
PropertyDetails for fast elements).
A few new tests are added to get-own-property-descriptor.js, I will
add a lot more to object-define-property when I add support for indices in
Object.defineProperty.
Review URL: http://codereview.chromium.org/2278002
http://code.google.com/p/v8/source/detail?r=4738
Modified:
/branches/bleeding_edge/src/macros.py
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/v8natives.js
/branches/bleeding_edge/test/es5conform/es5conform.status
/branches/bleeding_edge/test/mjsunit/get-own-property-descriptor.js
=======================================
--- /branches/bleeding_edge/src/macros.py Mon May 10 01:58:41 2010
+++ /branches/bleeding_edge/src/macros.py Thu May 27 00:43:43 2010
@@ -159,3 +159,13 @@
macro CAPTURE(index) = (3 + (index));
const CAPTURE0 = 3;
const CAPTURE1 = 4;
+
+# PropertyDescriptor return value indices - must match
+# PropertyDescriptorIndices in runtime.cc.
+const IS_ACCESSOR_INDEX = 0;
+const VALUE_INDEX = 1;
+const GETTER_INDEX = 2;
+const SETTER_INDEX = 3;
+const WRITABLE_INDEX = 4;
+const ENUMERABLE_INDEX = 5;
+const CONFIGURABLE_INDEX = 6;
=======================================
--- /branches/bleeding_edge/src/runtime.cc Wed May 26 09:11:30 2010
+++ /branches/bleeding_edge/src/runtime.cc Thu May 27 00:43:43 2010
@@ -569,6 +569,18 @@
}
+// Enumerator used as indices into the array returned from GetOwnProperty
+enum PropertyDescriptorIndices {
+ IS_ACCESSOR_INDEX,
+ VALUE_INDEX,
+ GETTER_INDEX,
+ SETTER_INDEX,
+ WRITABLE_INDEX,
+ ENUMERABLE_INDEX,
+ CONFIGURABLE_INDEX,
+ DESCRIPTOR_SIZE
+};
+
// Returns an array with the property description:
// if args[1] is not a property on args[0]
// returns undefined
@@ -579,18 +591,63 @@
static Object* Runtime_GetOwnProperty(Arguments args) {
ASSERT(args.length() == 2);
HandleScope scope;
- Handle<FixedArray> elms = Factory::NewFixedArray(5);
+ Handle<FixedArray> elms = Factory::NewFixedArray(DESCRIPTOR_SIZE);
Handle<JSArray> desc = Factory::NewJSArrayWithElements(elms);
LookupResult result;
CONVERT_CHECKED(JSObject, obj, args[0]);
CONVERT_CHECKED(String, name, args[1]);
+
+ // This could be an element.
+ uint32_t index;
+ if (name->AsArrayIndex(&index)) {
+ if (!obj->HasLocalElement(index)) {
+ return Heap::undefined_value();
+ }
+
+ // Special handling of string objects according to ECMAScript 5
15.5.5.2.
+ // Note that this might be a string object with elements other than the
+ // actual string value. This is covered by the subsequent cases.
+ if (obj->IsStringObjectWithCharacterAt(index)) {
+ JSValue* js_value = JSValue::cast(obj);
+ String* str = String::cast(js_value->value());
+ elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
+ elms->set(VALUE_INDEX, str->SubString(index, index+1));
+ elms->set(WRITABLE_INDEX, Heap::false_value());
+ elms->set(ENUMERABLE_INDEX, Heap::false_value());
+ elms->set(CONFIGURABLE_INDEX, Heap::false_value());
+ return *desc;
+ }
+
+ // This can potentially be an element in the elements dictionary or
+ // a fast element.
+ if (obj->HasDictionaryElements()) {
+ NumberDictionary* dictionary = obj->element_dictionary();
+ int entry = dictionary->FindEntry(index);
+ PropertyDetails details = dictionary->DetailsAt(entry);
+ elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
+ elms->set(VALUE_INDEX, dictionary->ValueAt(entry));
+ elms->set(WRITABLE_INDEX, Heap::ToBoolean(!details.IsDontDelete()));
+ elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!details.IsDontEnum()));
+ elms->set(CONFIGURABLE_INDEX,
Heap::ToBoolean(!details.IsReadOnly()));
+ return *desc;
+ } else {
+ // Elements that are stored as array elements always has:
+ // writable: true, configurable: true, enumerable: true.
+ elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
+ elms->set(VALUE_INDEX, obj->GetElement(index));
+ elms->set(WRITABLE_INDEX, Heap::true_value());
+ elms->set(ENUMERABLE_INDEX, Heap::true_value());
+ elms->set(CONFIGURABLE_INDEX, Heap::true_value());
+ return *desc;
+ }
+ }
// Use recursive implementation to also traverse hidden prototypes
GetOwnPropertyImplementation(obj, name, &result);
- if (!result.IsProperty())
+ if (!result.IsProperty()) {
return Heap::undefined_value();
-
+ }
if (result.type() == CALLBACKS) {
Object* structure = result.GetCallbackObject();
if (structure->IsProxy() || structure->IsAccessorInfo()) {
@@ -598,25 +655,25 @@
// an API defined callback.
Object* value = obj->GetPropertyWithCallback(
obj, structure, name, result.holder());
- elms->set(0, Heap::false_value());
- elms->set(1, value);
- elms->set(2, Heap::ToBoolean(!result.IsReadOnly()));
+ elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
+ elms->set(VALUE_INDEX, value);
+ elms->set(WRITABLE_INDEX, Heap::ToBoolean(!result.IsReadOnly()));
} else if (structure->IsFixedArray()) {
// __defineGetter__/__defineSetter__ callback.
- elms->set(0, Heap::true_value());
- elms->set(1, FixedArray::cast(structure)->get(0));
- elms->set(2, FixedArray::cast(structure)->get(1));
+ elms->set(IS_ACCESSOR_INDEX, Heap::true_value());
+ elms->set(GETTER_INDEX, FixedArray::cast(structure)->get(0));
+ elms->set(SETTER_INDEX, FixedArray::cast(structure)->get(1));
} else {
return Heap::undefined_value();
}
} else {
- elms->set(0, Heap::false_value());
- elms->set(1, result.GetLazyValue());
- elms->set(2, Heap::ToBoolean(!result.IsReadOnly()));
+ elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
+ elms->set(VALUE_INDEX, result.GetLazyValue());
+ elms->set(WRITABLE_INDEX, Heap::ToBoolean(!result.IsReadOnly()));
}
- elms->set(3, Heap::ToBoolean(!result.IsDontEnum()));
- elms->set(4, Heap::ToBoolean(!result.IsDontDelete()));
+ elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!result.IsDontEnum()));
+ elms->set(CONFIGURABLE_INDEX, Heap::ToBoolean(!result.IsDontDelete()));
return *desc;
}
=======================================
--- /branches/bleeding_edge/src/v8natives.js Wed May 26 01:31:57 2010
+++ /branches/bleeding_edge/src/v8natives.js Thu May 27 00:43:43 2010
@@ -492,23 +492,23 @@
function GetOwnProperty(obj, p) {
var desc = new PropertyDescriptor();
- // An array with:
- // obj is a data property [false, value, Writeable, Enumerable,
Configurable]
- // obj is an accessor [true, Get, Set, Enumerable, Configurable]
+ // GetOwnProperty returns an array indexed by the constants
+ // defined in macros.py.
+ // If p is not a property on obj undefined is returned.
var props = %GetOwnProperty(ToObject(obj), ToString(p));
if (IS_UNDEFINED(props)) return void 0;
// This is an accessor
- if (props[0]) {
- desc.setGet(props[1]);
- desc.setSet(props[2]);
+ if (props[IS_ACCESSOR_INDEX]) {
+ desc.setGet(props[GETTER_INDEX]);
+ desc.setSet(props[SETTER_INDEX]);
} else {
- desc.setValue(props[1]);
- desc.setWritable(props[2]);
- }
- desc.setEnumerable(props[3]);
- desc.setConfigurable(props[4]);
+ desc.setValue(props[VALUE_INDEX]);
+ desc.setWritable(props[WRITABLE_INDEX]);
+ }
+ desc.setEnumerable(props[ENUMERABLE_INDEX]);
+ desc.setConfigurable(props[CONFIGURABLE_INDEX]);
return desc;
}
=======================================
--- /branches/bleeding_edge/test/es5conform/es5conform.status Wed Apr 28
01:04:39 2010
+++ /branches/bleeding_edge/test/es5conform/es5conform.status Thu May 27
00:43:43 2010
@@ -200,11 +200,6 @@
# SUBSETFAIL
chapter15/15.2/15.2.3/15.2.3.4/15.2.3.4-4-35: FAIL_OK
-# getOwnPropertyDescriptor not implemented on array indices
-chapter15/15.2/15.2.3/15.2.3.4/15.2.3.4-4-b-1: FAIL_OK
-
-
-
# We fail this because Object.keys returns numbers for element indices
# rather than strings.
@@ -260,9 +255,6 @@
# Same as 15.4.4.16-7-7
chapter15/15.4/15.4.4/15.4.4.19/15.4.4.19-8-7: FAIL_OK
-# Uses a array index number as a property
-chapter15/15.4/15.4.4/15.4.4.19/15.4.4.19-8-c-iii-1: FAIL_OK
-
chapter15/15.5: UNIMPLEMENTED
chapter15/15.6: UNIMPLEMENTED
=======================================
--- /branches/bleeding_edge/test/mjsunit/get-own-property-descriptor.js Wed
Jan 13 04:10:57 2010
+++ /branches/bleeding_edge/test/mjsunit/get-own-property-descriptor.js Thu
May 27 00:43:43 2010
@@ -25,15 +25,22 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-function get(){return x}
-function set(x){this.x=x};
-
-var obj = {x:1};
+// This file only tests very simple descriptors that always have
+// configurable, enumerable, and writable set to true.
+// A range of more elaborate tests are performed in
+// object-define-property.js
+
+function get() { return x; }
+function set(x) { this.x = x; }
+
+var obj = {x: 1};
obj.__defineGetter__("accessor", get);
obj.__defineSetter__("accessor", set);
-
-
-var descIsData = Object.getOwnPropertyDescriptor(obj,'x');
+var a = new Array();
+a[1] = 42;
+obj[1] = 42;
+
+var descIsData = Object.getOwnPropertyDescriptor(obj, 'x');
assertTrue(descIsData.enumerable);
assertTrue(descIsData.writable);
assertTrue(descIsData.configurable);
@@ -49,3 +56,50 @@
var descIsNotAccessor =
Object.getOwnPropertyDescriptor(obj, 'not-accessor');
assertTrue(descIsNotAccessor == undefined);
+
+var descArray = Object.getOwnPropertyDescriptor(a, '1');
+assertTrue(descArray.enumerable);
+assertTrue(descArray.configurable);
+assertTrue(descArray.writable);
+assertEquals(descArray.value, 42);
+
+var descObjectElement = Object.getOwnPropertyDescriptor(obj, '1');
+assertTrue(descObjectElement.enumerable);
+assertTrue(descObjectElement.configurable);
+assertTrue(descObjectElement.writable);
+assertEquals(descObjectElement.value, 42);
+
+// String objects.
+var a = new String('foobar');
+for (var i = 0; i < a.length; i++) {
+ var descStringObject = Object.getOwnPropertyDescriptor(a, i);
+ assertFalse(descStringObject.enumerable);
+ assertFalse(descStringObject.configurable);
+ assertFalse(descStringObject.writable);
+ assertEquals(descStringObject.value, a.substring(i, i+1));
+}
+
+// Support for additional attributes on string objects.
+a.x = 42;
+a[10] = 'foo';
+var descStringProperty = Object.getOwnPropertyDescriptor(a, 'x');
+assertTrue(descStringProperty.enumerable);
+assertTrue(descStringProperty.configurable);
+assertTrue(descStringProperty.writable);
+assertEquals(descStringProperty.value, 42);
+
+var descStringElement = Object.getOwnPropertyDescriptor(a, '10');
+assertTrue(descStringElement.enumerable);
+assertTrue(descStringElement.configurable);
+assertTrue(descStringElement.writable);
+assertEquals(descStringElement.value, 'foo');
+
+// Test that elements in the prototype chain is not returned.
+var proto = {};
+proto[10] = 42;
+
+var objWithProto = new Array();
+objWithProto.prototype = proto;
+objWithProto[0] = 'bar';
+var descWithProto = Object.getOwnPropertyDescriptor(objWithProto, '10');
+assertEquals(undefined, descWithProto);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev