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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4738 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/macros.py b/src/macros.py
index 1533741..7d97918 100644 (file)
--- a/src/macros.py
+++ b/src/macros.py
@@ -159,3 +159,13 @@ macro LAST_INPUT(array) = ((array)[2]);
 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;

diff --git a/src/runtime.cc b/src/runtime.cc
index f627a26..25260b0 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -569,6 +569,18 @@ static void GetOwnPropertyImplementation(JSObject* obj,
 }
 
 
+// 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 void GetOwnPropertyImplementation(JSObject* obj,
 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 @@ static Object* Runtime_GetOwnProperty(Arguments args) {
       // 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;
 }
 

diff --git a/src/v8natives.js b/src/v8natives.js
index ed392e2..1d47eb7 100644 (file)
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -492,23 +492,23 @@ PropertyDescriptor.prototype.hasSetter = function() {
 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.setValue(props[VALUE_INDEX]);
+    desc.setWritable(props[WRITABLE_INDEX]);
   }
-  desc.setEnumerable(props[3]);
-  desc.setConfigurable(props[4]);
+  desc.setEnumerable(props[ENUMERABLE_INDEX]);
+  desc.setConfigurable(props[CONFIGURABLE_INDEX]);
 
   return desc;
 }

diff --git a/test/es5conform/es5conform.status b/test/es5conform/es5conform.status
index 5225c32..9d9dc3c 100644 (file)
--- a/test/es5conform/es5conform.status
+++ b/test/es5conform/es5conform.status
@@ -200,11 +200,6 @@ chapter15/15.2/15.2.3/15.2.3.4/15.2.3.4-4-34: FAIL_OK
 # 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 @@ chapter15/15.4/15.4.4/15.4.4.19/15.4.4.19-5-1: FAIL_OK
 # 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

diff --git a/test/mjsunit/get-own-property-descriptor.js b/test/mjsunit/get-own-property-descriptor.js
index 79172c8..ceb7715 100644 (file)
--- a/test/mjsunit/get-own-property-descriptor.js
+++ b/test/mjsunit/get-own-property-descriptor.js
@@ -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};
+// 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
 
-var obj = {x:1};
+function get() { return x; }
+function set(x) { this.x = x; }
+
+var obj = {x: 1};
 obj.__defineGetter__("accessor", get);
 obj.__defineSetter__("accessor", set);
+var a = new Array();
+a[1] = 42;
+obj[1] = 42;
 
-
-var descIsData = Object.getOwnPropertyDescriptor(obj,'x');
+var descIsData = Object.getOwnPropertyDescriptor(obj, 'x');
 assertTrue(descIsData.enumerable);
 assertTrue(descIsData.writable);
 assertTrue(descIsData.configurable);
@@ -49,3 +56,50 @@ assertTrue(descIsNotData == undefined);
 
 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);