Propagate exceptions thrown when setting elements.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 775487a0a6be95205874632c74075da4fe6c7889..b92350c47ea49fe6130ca20c8fd60eb12e55c202 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1707,8 +1707,8 @@ void JSObject::LookupCallbackSetterInPrototypes(String* name,
 }
 
 
-bool JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index,
-                                                        Object* value) {
+MaybeObject* JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index,
+                                                                Object* value) {
   for (Object* pt = GetPrototype();
        pt != Heap::null_value();
        pt = pt->GetPrototype()) {
@@ -1718,15 +1718,14 @@ bool JSObject::SetElementWithCallbackSetterInPrototypes(uint32_t index,
     NumberDictionary* dictionary = JSObject::cast(pt)->element_dictionary();
     int entry = dictionary->FindEntry(index);
     if (entry != NumberDictionary::kNotFound) {
-      Object* element = dictionary->ValueAt(entry);
       PropertyDetails details = dictionary->DetailsAt(entry);
       if (details.type() == CALLBACKS) {
-        SetElementWithCallback(element, index, value, JSObject::cast(pt));
-        return true;
+        return SetElementWithCallback(
+            dictionary->ValueAt(entry), index, value, JSObject::cast(pt));
       }
     }
   }
-  return false;
+  return Heap::the_hole_value();
 }
 
 
@@ -6962,9 +6961,10 @@ MaybeObject* JSObject::SetFastElement(uint32_t index,
   uint32_t elms_length = static_cast<uint32_t>(elms->length());
 
   if (check_prototype &&
-      (index >= elms_length || elms->get(index)->IsTheHole()) &&
-      SetElementWithCallbackSetterInPrototypes(index, value)) {
-    return value;
+      (index >= elms_length || elms->get(index)->IsTheHole())) {
+    MaybeObject* result =
+        SetElementWithCallbackSetterInPrototypes(index, value);
+    if (result != Heap::the_hole_value()) return result;

[Lasse Reichstein, 2011/02/08 14:17:56]

Check if the result is a failure instead.
You are comparing pointers of different type here (which is slightly icky, even
if it's true).

[antonm, 2011/02/08 14:30:21]

Let me double check.  I cannot use plain IsFailure as I need to be able to
distinguish between three outcomes: no setter in proto chain, setting threw,
setting went fine.  I used this hole to signify no setter case.

I can first check it's not a failure and when compare Object to the hole.  I can
too pass a pointer to a flag.

Which one do you prefer?

   }
 
 
@@ -7096,9 +7096,10 @@ MaybeObject* JSObject::SetElementWithoutInterceptor(uint32_t index,
         }
       } else {
         // Index not already used. Look for an accessor in the prototype chain.
-        if (check_prototype &&
-            SetElementWithCallbackSetterInPrototypes(index, value)) {
-          return value;
+        if (check_prototype) {
+          MaybeObject* result =
+              SetElementWithCallbackSetterInPrototypes(index, value);
+          if (result != Heap::the_hole_value()) return result;

[Lasse Reichstein, 2011/02/08 14:17:56]

Same here. Check result->IsFailure()

[antonm, 2011/02/08 14:30:21]

See above.

         }
         // When we set the is_extensible flag to false we always force
         // the element into dictionary mode (and force them to stay there).