Remove bogus writability check in DefineGetterSetter.

R=rossberg@chromium.org
TEST=test262

Review URL: http://codereview.chromium.org/8951013

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

diff --git a/src/objects.cc b/src/objects.cc
index 2b6f235..e193188 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -4257,10 +4257,10 @@ void JSObject::LookupCallback(String* name, LookupResult* result) {
 
 
 // Search for a getter or setter in an elements dictionary and update its
-// attributes.  Returns either undefined if the element is read-only, or the
-// getter/setter pair (fixed array) if there is an existing one, or the hole
-// value if the element does not exist or is a normal non-getter/setter data
-// element.
+// attributes.  Returns either undefined if the element is non-deletable, or
+// the getter/setter pair (fixed array) if there is an existing one, or the
+// hole value if the element does not exist or is a normal non-getter/setter
+// data element.
 static Object* UpdateGetterSetterInDictionary(NumberDictionary* dictionary,
                                               uint32_t index,
                                               PropertyAttributes attributes,
@@ -4269,7 +4269,8 @@ static Object* UpdateGetterSetterInDictionary(NumberDictionary* dictionary,
   if (entry != NumberDictionary::kNotFound) {
     Object* result = dictionary->ValueAt(entry);
     PropertyDetails details = dictionary->DetailsAt(entry);
-    if (details.IsReadOnly()) return heap->undefined_value();
+    // TODO(mstarzinger): We should check for details.IsDontDelete() here once
+    // we only call into the runtime once to set both getter and setter.
     if (details.type() == CALLBACKS && result->IsFixedArray()) {
       if (details.attributes() != attributes) {
         dictionary->DetailsAtPut(entry,
@@ -4353,7 +4354,8 @@ MaybeObject* JSObject::DefineGetterSetter(String* name,
     LookupResult result(heap->isolate());
     LocalLookup(name, &result);
     if (result.IsProperty()) {
-      if (result.IsReadOnly()) return heap->undefined_value();
+      // TODO(mstarzinger): We should check for result.IsDontDelete() here once
+      // we only call into the runtime once to set both getter and setter.
       if (result.type() == CALLBACKS) {
         Object* obj = result.GetCallbackObject();
         // Need to preserve old getters/setters.

diff --git a/src/runtime.cc b/src/runtime.cc
index 07825d2..b0e1a05 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -4248,27 +4248,14 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DefineOrRedefineAccessorProperty) {
   CONVERT_CHECKED(String, name, args[1]);
   CONVERT_CHECKED(Smi, flag_setter, args[2]);
   Object* fun = args[3];
-  RUNTIME_ASSERT(fun->IsSpecFunction() || fun->IsUndefined());
   CONVERT_CHECKED(Smi, flag_attr, args[4]);
+
   int unchecked = flag_attr->value();
   RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) == 0);
-  RUNTIME_ASSERT(!obj->IsNull());
-  LookupResult result(isolate);
-  obj->LocalLookupRealNamedProperty(name, &result);
-
   PropertyAttributes attr = static_cast<PropertyAttributes>(unchecked);
-  // If an existing property is either FIELD, NORMAL or CONSTANT_FUNCTION
-  // delete it to avoid running into trouble in DefineAccessor, which
-  // handles this incorrectly if the property is readonly (does nothing)
-  if (result.IsProperty() &&
-      (result.type() == FIELD || result.type() == NORMAL
-       || result.type() == CONSTANT_FUNCTION)) {
-    Object* ok;
-    { MaybeObject* maybe_ok =
-          obj->DeleteProperty(name, JSReceiver::NORMAL_DELETION);
-      if (!maybe_ok->ToObject(&ok)) return maybe_ok;
-    }
-  }
+
+  RUNTIME_ASSERT(!obj->IsNull());
+  RUNTIME_ASSERT(fun->IsSpecFunction() || fun->IsUndefined());
   return obj->DefineAccessor(name, flag_setter->value() == 0, fun, attr);
 }
 
@@ -4284,11 +4271,10 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DefineOrRedefineDataProperty) {
   CONVERT_ARG_CHECKED(JSObject, js_object, 0);
   CONVERT_ARG_CHECKED(String, name, 1);
   Handle<Object> obj_value = args.at<Object>(2);
-
   CONVERT_CHECKED(Smi, flag, args[3]);
+
   int unchecked = flag->value();
   RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) == 0);
-
   PropertyAttributes attr = static_cast<PropertyAttributes>(unchecked);
 
   // Check if this is an element.

diff --git a/test/test262/test262.status b/test/test262/test262.status
index e42aa30..5fa0ba8 100644 (file)
--- a/test/test262/test262.status
+++ b/test/test262/test262.status
@@ -205,21 +205,6 @@ S15.1.1.3_A2_T2: FAIL_OK  # undefined
 S15.4.4.2_A2_T1: FAIL_OK
 S15.4.4.3_A2_T1: FAIL_OK
 
-######################### UNANALYZED FAILURES ##########################
-
-# Bug? ES5 Attributes - Updating indexed data property 'P' whose attributes are
-#      [[Writable]]: false, [[Enumerable]]: true, [[Configurable]]: true to an
-#      accessor property, 'A' is an Array object (8.12.9 - step 9.b.i)
-15.2.3.6-4-360-1: FAIL
-# Bug? ES5 Attributes - Updating indexed data property 'P' whose attributes are
-#      [[Writable]]: false, [[Enumerable]]: true, [[Configurable]]: true to an
-#      accessor property, 'O' is an Arguments object (8.12.9 - step 9.b.i)
-15.2.3.6-4-360-6: FAIL
-# Bug? ES5 Attributes - Updating indexed data property 'P' whose attributes are
-#      [[Writable]]: false, [[Enumerable]]: true, [[Configurable]]: true to an
-#      accessor property, 'O' is the global object (8.12.9 - step 9.b.i)
-15.2.3.6-4-360-7: FAIL
-
 ############################ SKIPPED TESTS #############################
 
 # These tests take a looong time to run in debug mode.