Keyed stores to super with numeric keys.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 3eedfe92faf08b10e71b512049509a7e197ceb26..5890728d501bec3a1379067add109ba780660d94 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -802,6 +802,82 @@ MaybeHandle<Object> Object::GetElementWithReceiver(Isolate* isolate,
 }
 
 
+MaybeHandle<Object> Object::SetElementWithReceiver(
+    Isolate* isolate, Handle<Object> object, Handle<Object> receiver,
+    uint32_t index, Handle<Object> value, StrictMode strict_mode) {
+  // Iterate up the prototype chain until an element is found or the null
+  // prototype is encountered.
+  bool done = false;
+  for (PrototypeIterator iter(isolate, object,
+                              object->IsJSProxy() || object->IsJSObject()
+                                  ? PrototypeIterator::START_AT_RECEIVER
+                                  : PrototypeIterator::START_AT_PROTOTYPE);
+       !iter.IsAtEnd() && !done; iter.Advance()) {
+    if (PrototypeIterator::GetCurrent(iter)->IsJSProxy()) {
+      // TODO(dslomov): implement.
+      isolate->ThrowIllegalOperation();
+      return MaybeHandle<Object>();
+    }
+
+    Handle<JSObject> js_object =
+        Handle<JSObject>::cast(PrototypeIterator::GetCurrent(iter));
+
+    // Check access rights if needed.
+    if (js_object->IsAccessCheckNeeded()) {
+      if (!isolate->MayIndexedAccess(js_object, index, v8::ACCESS_SET)) {
+        isolate->ReportFailedAccessCheck(js_object, v8::ACCESS_SET);
+        RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
+        return isolate->factory()->undefined_value();
+      }
+    }
+
+    if (js_object->HasIndexedInterceptor()) {
+      Maybe<PropertyAttributes> from_interceptor =
+          JSObject::GetElementAttributeFromInterceptor(js_object, receiver,
+                                                       index);
+      if (!from_interceptor.has_value) return MaybeHandle<Object>();
+      if ((from_interceptor.value & READ_ONLY) != 0) {
+        return WriteToReadOnlyElement(isolate, receiver, index, value,
+                                      strict_mode);
+      }
+      done = from_interceptor.value != ABSENT;
+    }
+
+    if (!done &&
+        js_object->elements() != isolate->heap()->empty_fixed_array()) {
+      ElementsAccessor* accessor = js_object->GetElementsAccessor();
+      PropertyAttributes attrs =
+          accessor->GetAttributes(receiver, js_object, index);
+      if ((attrs & READ_ONLY) != 0) {
+        return WriteToReadOnlyElement(isolate, receiver, index, value,
+                                      strict_mode);
+      }
+      Handle<Object> element_structure;
+      if (accessor->GetStructure(receiver, js_object, index)

[Toon Verwaest, 2014/10/15 08:31:28]

Come to think about it, you also only want to call the setter on the prototype
chain if it's an AccessorPair (or currently ExecutableAccessorInfo).
ExecutableAccessorInfos are changing because they are incorrect, they are
supposed to behave like data properties. I'm pretty sure we never have those
anyway as elements. So I'm not sure it's really worthwhile to replace
GetAccessorPair by GetStructure.

It also causes confusion for the others where you changed it. You aren't
supposed to blacklist non-AccessorPair callbacks.

+              .ToHandle(&element_structure)) {
+        return JSObject::SetElementWithCallback(
+            receiver, element_structure, index, value, js_object, strict_mode);
+      } else {
+        done = attrs != ABSENT;
+      }
+    }
+  }
+
+  if (!receiver->IsJSObject()) {
+    return WriteToReadOnlyElement(isolate, receiver, index, value, strict_mode);
+  }
+  Handle<JSObject> target = Handle<JSObject>::cast(receiver);
+  ElementsAccessor* accessor = target->GetElementsAccessor();
+  PropertyAttributes attrs = accessor->GetAttributes(receiver, target, index);
+  if ((attrs & READ_ONLY) != 0) {
+    return WriteToReadOnlyElement(isolate, receiver, index, value, strict_mode);
+  }
+  PropertyAttributes new_attrs = attrs != ABSENT ? attrs : NONE;
+  return JSObject::SetElement(target, index, value, new_attrs, strict_mode,
+                              false);
+}
+
+
 Map* Object::GetRootMap(Isolate* isolate) {
   DisallowHeapAllocation no_alloc;
   if (IsSmi()) {
@@ -2930,6 +3006,21 @@ MaybeHandle<Object> Object::WriteToReadOnlyProperty(LookupIterator* it,
 }
 
 
+MaybeHandle<Object> Object::WriteToReadOnlyElement(Isolate* isolate,
+                                                   Handle<Object> receiver,
+                                                   uint32_t index,
+                                                   Handle<Object> value,
+                                                   StrictMode strict_mode) {
+  if (strict_mode != STRICT) return value;
+
+  Handle<Object> args[] = {isolate->factory()->NewNumberFromUint(index),
+                           receiver};
+  THROW_NEW_ERROR(isolate, NewTypeError("strict_read_only_property",
+                                        HandleVector(args, arraysize(args))),
+                  Object);
+}
+
+
 Handle<Object> Object::SetDataProperty(LookupIterator* it,
                                        Handle<Object> value) {
   // Proxies are handled on the WithHandler path. Other non-JSObjects cannot
@@ -4057,6 +4148,21 @@ Maybe<PropertyAttributes> JSObject::GetElementAttributeWithInterceptor(
   // callbacks or interceptor calls.
   AssertNoContextChange ncc(isolate);
 
+  Maybe<PropertyAttributes> from_interceptor =
+      GetElementAttributeFromInterceptor(object, receiver, index);
+  if (!from_interceptor.has_value) return Maybe<PropertyAttributes>();
+  if (from_interceptor.value != ABSENT) return maybe(from_interceptor.value);
+
+  return GetElementAttributeWithoutInterceptor(object, receiver, index,
+                                               check_prototype);
+}
+
+
+Maybe<PropertyAttributes> JSObject::GetElementAttributeFromInterceptor(
+    Handle<JSObject> object, Handle<Object> receiver, uint32_t index) {
+  Isolate* isolate = object->GetIsolate();
+  AssertNoContextChange ncc(isolate);
+
   Handle<InterceptorInfo> interceptor(object->GetIndexedInterceptor());
   PropertyCallbackArguments args(
       isolate, interceptor->data(), *receiver, *object);
@@ -4077,9 +4183,8 @@ Maybe<PropertyAttributes> JSObject::GetElementAttributeWithInterceptor(
     v8::Handle<v8::Value> result = args.Call(getter, index);
     if (!result.IsEmpty()) return maybe(NONE);
   }
-
-  return GetElementAttributeWithoutInterceptor(
-       object, receiver, index, check_prototype);
+  RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate, Maybe<PropertyAttributes>());
+  return maybe(ABSENT);
 }
 
 
@@ -4868,7 +4973,7 @@ MaybeHandle<Object> JSObject::DeleteElement(Handle<JSObject> object,
     if (!maybe.has_value) return MaybeHandle<Object>();
     should_enqueue_change_record = maybe.value;
     if (should_enqueue_change_record) {
-      if (!GetOwnElementAccessorPair(object, index).is_null()) {

[Toon Verwaest, 2014/10/15 08:31:28]

These cases actually only blacklist accessor pairs. The other accessors look
like data properties to JS, so have to be read.
(I don't think we ever have non-accessorpair indexed callbacks, but for
completeness ...)

+      if (!GetOwnElementStructure(object, index).is_null()) {
         old_value = Handle<Object>::cast(factory->the_hole_value());
       } else {
         old_value = Object::GetElement(
@@ -6130,7 +6235,7 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
         return isolate->factory()->undefined_value();
       }
       preexists = maybe.value;
-      if (preexists && GetOwnElementAccessorPair(object, index).is_null()) {
+      if (preexists && GetOwnElementStructure(object, index).is_null()) {
         old_value =
             Object::GetElement(isolate, object, index).ToHandleChecked();
       }
@@ -11115,7 +11220,7 @@ static bool GetOldValue(Isolate* isolate,
   DCHECK(maybe.value != ABSENT);
   if (maybe.value == DONT_DELETE) return false;
   Handle<Object> value;
-  if (!JSObject::GetOwnElementAccessorPair(object, index).is_null()) {
+  if (!JSObject::GetOwnElementStructure(object, index).is_null()) {
     value = Handle<Object>::cast(isolate->factory()->the_hole_value());
   } else {
     value = Object::GetElement(isolate, object, index).ToHandleChecked();
@@ -11764,21 +11869,20 @@ void JSObject::EnsureCanContainElements(Handle<JSObject> object,
 }
 
 
-MaybeHandle<AccessorPair> JSObject::GetOwnElementAccessorPair(
-    Handle<JSObject> object,
-    uint32_t index) {
+MaybeHandle<Object> JSObject::GetOwnElementStructure(Handle<JSObject> object,
+                                                     uint32_t index) {
   if (object->IsJSGlobalProxy()) {
     PrototypeIterator iter(object->GetIsolate(), object);
     if (iter.IsAtEnd()) return MaybeHandle<AccessorPair>();
     DCHECK(PrototypeIterator::GetCurrent(iter)->IsJSGlobalObject());
-    return GetOwnElementAccessorPair(
+    return GetOwnElementStructure(
         Handle<JSObject>::cast(PrototypeIterator::GetCurrent(iter)), index);
   }
 
   // Check for lookup interceptor.
   if (object->HasIndexedInterceptor()) return MaybeHandle<AccessorPair>();
 
-  return object->GetElementsAccessor()->GetAccessorPair(object, object, index);
+  return object->GetElementsAccessor()->GetStructure(object, object, index);
 }
 
 
@@ -11871,13 +11975,10 @@ MaybeHandle<Object> JSObject::GetElementWithCallback(
 }
 
 
-MaybeHandle<Object> JSObject::SetElementWithCallback(Handle<JSObject> object,
-                                                     Handle<Object> structure,
-                                                     uint32_t index,
-                                                     Handle<Object> value,
-                                                     Handle<JSObject> holder,
-                                                     StrictMode strict_mode) {
-  Isolate* isolate = object->GetIsolate();
+MaybeHandle<Object> JSObject::SetElementWithCallback(
+    Handle<Object> object, Handle<Object> structure, uint32_t index,
+    Handle<Object> value, Handle<JSObject> holder, StrictMode strict_mode) {
+  Isolate* isolate = holder->GetIsolate();
 
   // We should never get here to initialize a const with the hole
   // value since a const declaration would conflict with the setter.
@@ -11893,7 +11994,7 @@ MaybeHandle<Object> JSObject::SetElementWithCallback(Handle<JSObject> object,
     if (call_fun == NULL) return value;
     Handle<Object> number = isolate->factory()->NewNumberFromUint(index);
     Handle<String> key(isolate->factory()->NumberToString(number));
-    LOG(isolate, ApiNamedPropertyAccess("store", *object, *key));
+    LOG(isolate, ApiNamedPropertyAccess("store", *holder, *key));
     PropertyCallbackArguments
         args(isolate, data->data(), *object, *holder);
     args.Call(call_fun,
@@ -12413,7 +12514,7 @@ MaybeHandle<Object> JSObject::SetElement(Handle<JSObject> object,
   Handle<Object> new_length_handle;
 
   if (old_attributes != ABSENT) {
-    if (GetOwnElementAccessorPair(object, index).is_null()) {
+    if (GetOwnElementStructure(object, index).is_null()) {
       old_value = Object::GetElement(isolate, object, index).ToHandleChecked();
     }
   } else if (object->IsJSArray()) {