Make integer indexed properties ("elements") work for proxies.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 609f66ae1d5291f3bccb020428497467aa21bca9..7c3d9660abdcd9f82fb7aec036be9ea765895948 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -234,17 +234,31 @@ MaybeObject* JSProxy::GetPropertyWithHandler(Object* receiver_raw,
 
 MaybeObject* JSProxy::GetElementWithHandler(Object* receiver,
                                             uint32_t index) {
-  Heap* heap = GetHeap();
-  Object* number;
-  MaybeObject* maybe = heap->NumberFromUint32(index);
-  if (!maybe->To<Object>(&number)) return maybe;
-  maybe = heap->NumberToString(number);
   String* name;
+  MaybeObject* maybe = GetHeap()->Uint32ToString(index);
   if (!maybe->To<String>(&name)) return maybe;
   return GetPropertyWithHandler(receiver, name);
 }
 
 
+MaybeObject* JSProxy::SetElementWithHandler(uint32_t index,
+                                            Object* value,
+                                            StrictModeFlag strict_mode) {
+  String* name;
+  MaybeObject* maybe = GetHeap()->Uint32ToString(index);
+  if (!maybe->To<String>(&name)) return maybe;
+  return SetPropertyWithHandler(name, value, NONE, strict_mode);
+}
+
+
+bool JSProxy::HasElementWithHandler(uint32_t index) {
+  String* name;
+  MaybeObject* maybe = GetHeap()->Uint32ToString(index);
+  if (!maybe->To<String>(&name)) return maybe;
+  return HasPropertyWithHandler(name);
+}
+
+
 MaybeObject* Object::GetPropertyWithDefinedGetter(Object* receiver,
                                                   JSFunction* getter) {
   HandleScope scope;
@@ -1926,6 +1940,16 @@ MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(
   for (Object* pt = GetPrototype();
        pt != heap->null_value();
        pt = pt->GetPrototype()) {
+    if (pt->IsJSProxy()) {
+      String* name;
+      MaybeObject* maybe = GetHeap()->Uint32ToString(index);
+      if (!maybe->To<String>(&name)) {
+        *found = true;  // Force abort
+        return maybe;
+      }
+      return JSProxy::cast(pt)->SetPropertyWithHandlerDefiningSetter(
+          name, value, NONE, strict_mode, found);
+    }
     if (!JSObject::cast(pt)->HasDictionaryElements()) {
       continue;
     }
@@ -2261,6 +2285,64 @@ MUST_USE_RESULT MaybeObject* JSProxy::SetPropertyWithHandler(
 }
 
 
+MUST_USE_RESULT MaybeObject* JSProxy::SetPropertyWithHandlerDefiningSetter(
+    String* name,
+    Object* value,
+    PropertyAttributes attributes,
+    StrictModeFlag strict_mode,
+    bool* found) {
+  *found = true;  // except where defined otherwise...
+  Isolate* isolate = GetHeap()->isolate();
+  Handle<JSProxy> proxy(this);
+  Handle<String> hname(name);
+  Handle<Object> hvalue(value);
+  Handle<Object> args[] = { hname };
+  Handle<Object> result = proxy->CallTrap(
+    "getOwnPropertyDescriptor", Handle<Object>(), ARRAY_SIZE(args), args);

[Rico, 2011/09/12 11:21:18]

4 space indention 

[rossberg, 2011/09/13 16:10:25]

Done.

+  if (isolate->has_pending_exception()) return Failure::Exception();
+
+  if (!result->IsUndefined()) {
+    // The proxy handler cares about this property.
+    // Check whether it is virtualized as an accessor.
+    Handle<String> getter_name =

[Rico, 2011/09/12 11:21:18]

We might as well lookup the setter first, and then only lookup the getter if the
setter is not found

[rossberg, 2011/09/13 16:10:25]

Done.

I also forgot to invoke ToCompletePropertyDescriptor, as the Proxy spec
requires.

+        isolate->factory()->LookupAsciiSymbol("get");
+    Handle<Object> getter(
+        v8::internal::GetProperty(result, getter_name));
+    if (isolate->has_pending_exception()) return Failure::Exception();
+    Handle<String> setter_name =
+        isolate->factory()->LookupAsciiSymbol("set");
+    Handle<Object> setter(
+        v8::internal::GetProperty(result, setter_name));
+    if (isolate->has_pending_exception()) return Failure::Exception();
+
+    if (!setter->IsUndefined()) {
+      // We have a setter -- invoke it.
+      if (setter->IsJSFunction()) {
+        return proxy->SetPropertyWithDefinedSetter(
+            JSFunction::cast(*setter), *hvalue);
+      }
+      Handle<Object> args[] = { setter };
+      Handle<Object> error = isolate->factory()->NewTypeError(
+         "setter_must_be_callable", HandleVector(args, ARRAY_SIZE(args)));

[Rico, 2011/09/12 11:21:18]

4 space indention

[rossberg, 2011/09/13 16:10:25]

Done.

+      return isolate->Throw(*error);
+    } else if (!getter->IsUndefined()) {
+      // We have a getter but no setter -- the property may not be
+      // written. In strict mode, throw an error.
+      if (strict_mode == kNonStrictMode) return *hvalue;
+      Handle<Object> args[] = { hname, proxy };
+      Handle<Object> error = isolate->factory()->NewTypeError(
+          "no_setter_in_callback", HandleVector(args, ARRAY_SIZE(args)));
+      return isolate->Throw(*error);
+    }
+    // Fall-through.
+  }
+
+  // The proxy does not define the property as an accessor.
+  *found = false;
+  return value;
+}
+
+
 MUST_USE_RESULT MaybeObject* JSProxy::DeletePropertyWithHandler(
     String* name_raw, DeleteMode mode) {
   Isolate* isolate = GetIsolate();
@@ -2286,6 +2368,15 @@ MUST_USE_RESULT MaybeObject* JSProxy::DeletePropertyWithHandler(
 }
 
 
+MUST_USE_RESULT MaybeObject* JSProxy::DeleteElementWithHandler(
+    uint32_t index, DeleteMode mode) {

[Rico, 2011/09/12 11:21:18]

parameters on separate lines to be consistent

[rossberg, 2011/09/13 16:10:25]

Done.

+  String* name;
+  MaybeObject* maybe = GetHeap()->Uint32ToString(index);
+  if (!maybe->To<String>(&name)) return maybe;
+  return JSProxy::DeletePropertyWithHandler(name, mode);
+}
+
+
 MUST_USE_RESULT PropertyAttributes JSProxy::GetPropertyAttributeWithHandler(
     JSReceiver* receiver_raw,
     String* name_raw) {
@@ -2327,6 +2418,16 @@ MUST_USE_RESULT PropertyAttributes JSProxy::GetPropertyAttributeWithHandler(
 }
 
 
+MUST_USE_RESULT PropertyAttributes JSProxy::GetElementAttributeWithHandler(
+    JSReceiver* receiver,
+    uint32_t index) {
+  String* name;
+  MaybeObject* maybe = GetHeap()->Uint32ToString(index);
+  if (!maybe->To<String>(&name)) return NONE;

[Rico, 2011/09/12 11:21:18]

Why do we return NONE here if allocation fails?

[rossberg, 2011/09/13 16:10:25]

Fixed by introducing a factory method.

+  return GetPropertyAttributeWithHandler(receiver, name);
+}
+
+
 void JSProxy::Fix() {
   Isolate* isolate = GetIsolate();
   HandleScope scope(isolate);
@@ -2424,6 +2525,18 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* result,
       } else if (accessor_result.type() == HANDLER) {
         // There is a proxy in the prototype chain. Invoke its
         // getOwnPropertyDescriptor trap.
+        bool found = false;
+        Handle<JSObject> self(this);
+        Handle<String> hname(name);
+        Handle<Object> hvalue(value);
+        MaybeObject* result =
+            accessor_result.proxy()->SetPropertyWithHandlerDefiningSetter(
+                name, value, attributes, strict_mode, &found);
+        if (found) return result;
+        // The proxy does not define the property as an accessor.
+        // Consequently, it has no effect on setting the receiver.
+        return self->AddProperty(*hname, *hvalue, attributes, strict_mode);
+/*

[Rico, 2011/09/12 11:21:18]

If this code is not needed anymore please delete it

[rossberg, 2011/09/13 16:10:25]

Oops.

         Isolate* isolate = heap->isolate();
         Handle<JSObject> self(this);
         Handle<String> hname(name);
@@ -2471,6 +2584,7 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* result,
           // Consequently, it has no effect on setting the receiver.
           return self->AddProperty(*hname, *hvalue, attributes, strict_mode);
         }
+*/
       }
     }
   }
@@ -2717,9 +2831,8 @@ PropertyAttributes JSReceiver::GetPropertyAttributeWithReceiver(
       String* key) {
   uint32_t index = 0;
   if (IsJSObject() && key->AsArrayIndex(&index)) {
-    if (JSObject::cast(this)->HasElementWithReceiver(receiver, index))
-      return NONE;
-    return ABSENT;
+    return JSObject::cast(this)->HasElementWithReceiver(receiver, index)
+        ? NONE : ABSENT;
   }
   // Named property.
   LookupResult result;
@@ -3252,9 +3365,16 @@ MaybeObject* JSObject::DeleteElement(uint32_t index, DeleteMode mode) {
 MaybeObject* JSReceiver::DeleteProperty(String* name, DeleteMode mode) {
   if (IsJSProxy()) {
     return JSProxy::cast(this)->DeletePropertyWithHandler(name, mode);
-  } else {
-    return JSObject::cast(this)->DeleteProperty(name, mode);
   }
+  return JSObject::cast(this)->DeleteProperty(name, mode);
+}
+
+
+MaybeObject* JSReceiver::DeleteElement(uint32_t index, DeleteMode mode) {
+  if (IsJSProxy()) {
+    return JSProxy::cast(this)->DeleteElementWithHandler(index, mode);
+  }
+  return JSObject::cast(this)->DeleteElement(index, mode);
 }
 
 
@@ -7887,6 +8007,11 @@ bool JSObject::HasElementPostInterceptor(JSReceiver* receiver, uint32_t index) {
 
   Object* pt = GetPrototype();
   if (pt->IsNull()) return false;
+  if (pt->IsJSProxy()) {
+    // We need to follow the spec and simulate a call to [[GetOwnProperty]].
+    return JSProxy::cast(pt)->GetElementAttributeWithHandler(
+        receiver, index) != ABSENT;
+  }
   return JSObject::cast(pt)->HasElementWithReceiver(receiver, index);
 }
 
@@ -8143,6 +8268,11 @@ bool JSObject::HasElementWithReceiver(JSReceiver* receiver, uint32_t index) {
 
   Object* pt = GetPrototype();
   if (pt->IsNull()) return false;
+  if (pt->IsJSProxy()) {
+    // We need to follow the spec and simulate a call to [[GetOwnProperty]].
+    return JSProxy::cast(pt)->GetElementAttributeWithHandler(
+        receiver, index) != ABSENT;
+  }
   return JSObject::cast(pt)->HasElementWithReceiver(receiver, index);
 }
 
@@ -8575,6 +8705,16 @@ MUST_USE_RESULT MaybeObject* JSObject::SetFastDoubleElement(
 }
 
 
+MaybeObject* JSReceiver::SetElement(uint32_t index,
+                                    Object* value,
+                                    StrictModeFlag strict_mode,
+                                    bool check_proto) {
+  return IsJSProxy()
+     ? JSProxy::cast(this)->SetElementWithHandler(index, value, strict_mode)

[Rico, 2011/09/12 11:21:18]

4 space indention 

[rossberg, 2011/09/13 16:10:25]

Done.

+     : JSObject::cast(this)->SetElement(index, value, strict_mode, check_proto);
+}
+
+
 MaybeObject* JSObject::SetElement(uint32_t index,
                                   Object* value,
                                   StrictModeFlag strict_mode,