Revision: 7409
Author:   kmilli...@chromium.org
Date:     Tue Mar 29 03:57:43 2011
Log:      [Arguments] Support setting properties on dictionary arguments.

Support setting properties that are not aliased by the parameters and that
are in a dictionary backing store.  This required teaching the function
ShouldConvertToFastElements about non-strict arguments objects.

R=a...@chromium.org

Review URL: http://codereview.chromium.org/6773005
http://code.google.com/p/v8/source/detail?r=7409

Modified:
 /branches/experimental/arguments/src/handles.cc
 /branches/experimental/arguments/src/objects.cc
 /branches/experimental/arguments/src/objects.h
 /branches/experimental/arguments/src/runtime.cc
 /branches/experimental/arguments/test/cctest/test-heap.cc

=======================================
--- /branches/experimental/arguments/src/handles.cc     Wed Mar 23 04:25:17 2011
+++ /branches/experimental/arguments/src/handles.cc     Tue Mar 29 03:57:43 2011
@@ -494,7 +494,8 @@
     }
   }
   CALL_HEAP_FUNCTION(object->GetIsolate(),
- object->SetElement(index, *value, strict_mode), Object);
+                     object->SetElement(index, *value, strict_mode, true),
+                     Object);
 }


=======================================
--- /branches/experimental/arguments/src/objects.cc     Tue Mar 29 00:56:46 2011
+++ /branches/experimental/arguments/src/objects.cc     Tue Mar 29 03:57:43 2011
@@ -7497,7 +7497,107 @@
   // Otherwise default to slow case.
   MaybeObject* result = NormalizeElements();
   if (result->IsFailure()) return result;
-  return SetElement(index, value, strict_mode, check_prototype);
+  return SetDictionaryElement(index, value, strict_mode, check_prototype);
+}
+
+
+MaybeObject* JSObject::SetDictionaryElement(uint32_t index,
+                                            Object* value,
+                                            StrictModeFlag strict_mode,
+                                            bool check_prototype) {
+  ASSERT(HasDictionaryElements() || HasDictionaryArgumentsElements());
+  Isolate* isolate = GetIsolate();
+  Heap* heap = isolate->heap();
+
+  // Insert element in the dictionary.
+  FixedArray* elements = FixedArray::cast(this->elements());
+  bool is_arguments =
+      (elements->map() == heap->non_strict_arguments_elements_map());
+  NumberDictionary* dictionary = NULL;
+  if (is_arguments) {
+    dictionary = NumberDictionary::cast(elements->get(1));
+  } else {
+    dictionary = NumberDictionary::cast(elements);
+  }
+
+  int entry = dictionary->FindEntry(index);
+  if (entry != NumberDictionary::kNotFound) {
+    Object* element = dictionary->ValueAt(entry);
+    PropertyDetails details = dictionary->DetailsAt(entry);
+    if (details.type() == CALLBACKS) {
+      return SetElementWithCallback(element, index, value, this);
+    } else {
+      dictionary->UpdateMaxNumberKey(index);
+      // If put fails in strict mode, throw an exception.
+ if (!dictionary->ValueAtPut(entry, value) && strict_mode == kStrictMode) { + Handle<Object> number = isolate->factory()->NewNumberFromUint(index);
+        Handle<Object> holder(this);
+        Handle<Object> args[2] = { number, holder };
+        Handle<Object> error =
+            isolate->factory()->NewTypeError("strict_read_only_property",
+                                             HandleVector(args, 2));
+        return isolate->Throw(*error);
+      }
+    }
+  } else {
+    // Index not already used. Look for an accessor in the prototype chain.
+    if (check_prototype) {
+      bool found;
+      MaybeObject* result =
+          // Strict mode not needed. No-setter case already handled.
+          SetElementWithCallbackSetterInPrototypes(index, value, &found);
+      if (found) return result;
+    }
+    // When we set the is_extensible flag to false we always force the
+    // element into dictionary mode (and force them to stay there).
+    if (!map()->is_extensible()) {
+      Handle<Object> number = isolate->factory()->NewNumberFromUint(index);
+      Handle<String> name = isolate->factory()->NumberToString(number);
+      Handle<Object> args[1] = { name };
+      Handle<Object> error =
+          isolate->factory()->NewTypeError("object_not_extensible",
+                                           HandleVector(args, 1));
+      return isolate->Throw(*error);
+    }
+    Object* new_dictionary;
+    MaybeObject* maybe = dictionary->AtNumberPut(index, value);
+    if (!maybe->ToObject(&new_dictionary)) return maybe;
+    if (dictionary != NumberDictionary::cast(new_dictionary)) {
+      if (is_arguments) {
+        elements->set(1, new_dictionary);
+      } else {
+        set_elements(HeapObject::cast(new_dictionary));
+      }
+      dictionary = NumberDictionary::cast(new_dictionary);
+    }
+  }
+
+  // Update the array length if this JSObject is an array.
+  if (IsJSArray()) {
+    MaybeObject* result =
+        JSArray::cast(this)->JSArrayUpdateLengthFromIndex(index, value);
+    if (result->IsFailure()) return result;
+  }
+
+  // Attempt to put this object back in fast case.
+  if (ShouldConvertToFastElements()) {
+    uint32_t new_length = 0;
+    if (IsJSArray()) {
+      CHECK(JSArray::cast(this)->length()->ToArrayIndex(&new_length));
+    } else {
+      new_length = dictionary->max_number_key() + 1;
+    }
+    MaybeObject* result =
+        SetFastElementsCapacityAndLength(new_length, new_length);
+    if (result->IsFailure()) return result;
+#ifdef DEBUG
+    if (FLAG_trace_normalization) {
+      PrintF("Object elements are fast case again:\n");
+      Print();
+    }
+#endif
+  }
+  return value;
 }


@@ -7547,8 +7647,9 @@
   Isolate* isolate = GetIsolate();
   switch (GetElementsKind()) {
     case FAST_ELEMENTS:
-      // Fast case.
       return SetFastElement(index, value, strict_mode, check_prototype);
+    case DICTIONARY_ELEMENTS:
+ return SetDictionaryElement(index, value, strict_mode, check_prototype);
     case EXTERNAL_PIXEL_ELEMENTS: {
       ExternalPixelArray* pixels = ExternalPixelArray::cast(elements());
       return pixels->SetValue(index, value);
@@ -7584,92 +7685,6 @@
       ExternalFloatArray* array = ExternalFloatArray::cast(elements());
       return array->SetValue(index, value);
     }
-    case DICTIONARY_ELEMENTS: {
-      // Insert element in the dictionary.
-      FixedArray* elms = FixedArray::cast(elements());
-      NumberDictionary* dictionary = NumberDictionary::cast(elms);
-
-      int entry = dictionary->FindEntry(index);
-      if (entry != NumberDictionary::kNotFound) {
-        Object* element = dictionary->ValueAt(entry);
-        PropertyDetails details = dictionary->DetailsAt(entry);
-        if (details.type() == CALLBACKS) {
-          return SetElementWithCallback(element, index, value, this);
-        } else {
-          dictionary->UpdateMaxNumberKey(index);
-          // If put fails instrict mode, throw exception.
-          if (!dictionary->ValueAtPut(entry, value) &&
-              strict_mode == kStrictMode) {
- Handle<Object> number(isolate->factory()->NewNumberFromUint(index));
-            Handle<Object> holder(this);
-            Handle<Object> args[2] = { number, holder };
-            return isolate->Throw(
- *isolate->factory()->NewTypeError("strict_read_only_property",
-                                                  HandleVector(args, 2)));
-          }
-        }
-      } else {
- // Index not already used. Look for an accessor in the prototype chain.
-        if (check_prototype) {
-          bool found;
-          MaybeObject* result =
-              // Strict mode not needed. No-setter case already handled.
- SetElementWithCallbackSetterInPrototypes(index, value, &found);
-          if (found) return result;
-        }
-        // When we set the is_extensible flag to false we always force
-        // the element into dictionary mode (and force them to stay there).
-        if (!map()->is_extensible()) {
- Handle<Object> number(isolate->factory()->NewNumberFromUint(index));
-          Handle<String> index_string(
-              isolate->factory()->NumberToString(number));
-          Handle<Object> args[1] = { index_string };
-          return isolate->Throw(
-              *isolate->factory()->NewTypeError("object_not_extensible",
-                                                HandleVector(args, 1)));
-        }
-        Object* result;
- { MaybeObject* maybe_result = dictionary->AtNumberPut(index, value);
-          if (!maybe_result->ToObject(&result)) return maybe_result;
-        }
-        if (elms != FixedArray::cast(result)) {
-          set_elements(FixedArray::cast(result));
-        }
-      }
-
-      // Update the array length if this JSObject is an array.
-      if (IsJSArray()) {
-        JSArray* array = JSArray::cast(this);
-        Object* return_value;
-        { MaybeObject* maybe_return_value =
-              array->JSArrayUpdateLengthFromIndex(index, value);
-          if (!maybe_return_value->ToObject(&return_value)) {
-            return maybe_return_value;
-          }
-        }
-      }
-
-      // Attempt to put this object back in fast case.
-      if (ShouldConvertToFastElements()) {
-        uint32_t new_length = 0;
-        if (IsJSArray()) {
-          CHECK(JSArray::cast(this)->length()->ToArrayIndex(&new_length));
-        } else {
- new_length = NumberDictionary::cast(elements())->max_number_key() + 1;
-        }
-        MaybeObject* result =
-            SetFastElementsCapacityAndLength(new_length, new_length);
-        if (result->IsFailure()) return result;
-#ifdef DEBUG
-        if (FLAG_trace_normalization) {
-          PrintF("Object elements are fast case again:\n");
-          Print();
-        }
-#endif
-      }
-      return value;
-    }
-
     case NON_STRICT_ARGUMENTS_ELEMENTS: {
       FixedArray* parameter_map = FixedArray::cast(elements());
       uint32_t length = parameter_map->length();
@@ -7685,8 +7700,8 @@
         // Object is not mapped, defer to the arguments.
         FixedArray* arguments = FixedArray::cast(parameter_map->get(1));
         if (arguments->IsDictionary()) {
-          UNIMPLEMENTED();
-          break;
+          return SetDictionaryElement(index, value, strict_mode,
+                                      check_prototype);
         } else {
return SetFastElement(index, value, strict_mode, check_prototype);
         }
@@ -8040,16 +8055,23 @@


 bool JSObject::ShouldConvertToFastElements() {
-  ASSERT(HasDictionaryElements());
-  NumberDictionary* dictionary = NumberDictionary::cast(elements());
+  ASSERT(HasDictionaryElements() || HasDictionaryArgumentsElements());
   // If the elements are sparse, we should not go back to fast case.
   if (!HasDenseElements()) return false;
-  // If an element has been added at a very high index in the elements
-  // dictionary, we cannot go back to fast case.
-  if (dictionary->requires_slow_elements()) return false;
   // An object requiring access checks is never allowed to have fast
   // elements.  If it had fast elements we would skip security checks.
   if (IsAccessCheckNeeded()) return false;
+
+  FixedArray* elements = FixedArray::cast(this->elements());
+  NumberDictionary* dictionary = NULL;
+  if (elements->map() == GetHeap()->non_strict_arguments_elements_map()) {
+    dictionary = NumberDictionary::cast(elements->get(1));
+  } else {
+    dictionary = NumberDictionary::cast(elements);
+  }
+  // If an element has been added at a very high index in the elements
+  // dictionary, we cannot go back to fast case.
+  if (dictionary->requires_slow_elements()) return false;
   // If the dictionary backing storage takes up roughly half as much
   // space as a fast-case backing storage would the array should have
   // fast elements.
=======================================
--- /branches/experimental/arguments/src/objects.h      Tue Mar 29 00:56:46 2011
+++ /branches/experimental/arguments/src/objects.h      Tue Mar 29 03:57:43 2011
@@ -1587,14 +1587,18 @@
   MUST_USE_RESULT MaybeObject* SetFastElement(uint32_t index,
                                               Object* value,
                                               StrictModeFlag strict_mode,
-                                              bool check_prototype = true);
+                                              bool check_prototype);
+  MUST_USE_RESULT MaybeObject* SetDictionaryElement(uint32_t index,
+                                                    Object* value,
+ StrictModeFlag strict_mode,
+                                                    bool check_prototype);

   // Set the index'th array element.
   // A Failure object is returned if GC is needed.
   MUST_USE_RESULT MaybeObject* SetElement(uint32_t index,
                                           Object* value,
                                           StrictModeFlag strict_mode,
-                                          bool check_prototype = true);
+                                          bool check_prototype);

   // Returns the index'th element.
   // The undefined object if index is out of bounds.
=======================================
--- /branches/experimental/arguments/src/runtime.cc     Thu Mar 24 04:22:39 2011
+++ /branches/experimental/arguments/src/runtime.cc     Tue Mar 29 03:57:43 2011
@@ -4063,7 +4063,7 @@
   Handle<String> name = Handle<String>::cast(converted);

   if (name->AsArrayIndex(&index)) {
-    return js_object->SetElement(index, *value, strict_mode);
+    return js_object->SetElement(index, *value, strict_mode, true);
   } else {
     return js_object->SetProperty(*name, *value, attr, strict_mode);
   }
@@ -4091,12 +4091,12 @@
       return *value;
     }

-    return js_object->SetElement(index, *value, kNonStrictMode);
+    return js_object->SetElement(index, *value, kNonStrictMode, true);
   }

   if (key->IsString()) {
     if (Handle<String>::cast(key)->AsArrayIndex(&index)) {
-      return js_object->SetElement(index, *value, kNonStrictMode);
+      return js_object->SetElement(index, *value, kNonStrictMode, true);
     } else {
       Handle<String> key_string = Handle<String>::cast(key);
       key_string->TryFlatten();
@@ -4113,7 +4113,7 @@
   Handle<String> name = Handle<String>::cast(converted);

   if (name->AsArrayIndex(&index)) {
-    return js_object->SetElement(index, *value, kNonStrictMode);
+    return js_object->SetElement(index, *value, kNonStrictMode, true);
   } else {
return js_object->SetLocalPropertyIgnoreAttributes(*name, *value, attr);
   }
@@ -8608,8 +8608,8 @@
   }
   Object* obj;
// Strict not needed. Used for cycle detection in Array join implementation.
-  { MaybeObject* maybe_obj = array->SetFastElement(length, element,
-                                                   kNonStrictMode);
+  { MaybeObject* maybe_obj =
+        array->SetFastElement(length, element, kNonStrictMode, true);
     if (!maybe_obj->ToObject(&obj)) return maybe_obj;
   }
   return isolate->heap()->true_value();
=======================================
--- /branches/experimental/arguments/test/cctest/test-heap.cc Wed Mar 23 04:25:17 2011 +++ /branches/experimental/arguments/test/cctest/test-heap.cc Tue Mar 29 03:57:43 2011
@@ -675,7 +675,7 @@
   CHECK(array->HasFastElements());  // Must be in fast mode.

   // array[length] = name.
-  ok = array->SetElement(0, *name, kNonStrictMode)->ToObjectChecked();
+ ok = array->SetElement(0, *name, kNonStrictMode, true)->ToObjectChecked();
   CHECK_EQ(Smi::FromInt(1), array->length());
   CHECK_EQ(array->GetElement(0), *name);

@@ -690,7 +690,8 @@
   CHECK(array->HasDictionaryElements());  // Must be in slow mode.

   // array[length] = name.
- ok = array->SetElement(int_length, *name, kNonStrictMode)->ToObjectChecked();
+  ok = array->SetElement(
+      int_length, *name, kNonStrictMode, true)->ToObjectChecked();
   uint32_t new_int_length = 0;
   CHECK(array->length()->ToArrayIndex(&new_int_length));
   CHECK_EQ(static_cast<double>(int_length), new_int_length - 1);
@@ -717,9 +718,10 @@
   obj->SetProperty(
       *second, Smi::FromInt(2), NONE, kNonStrictMode)->ToObjectChecked();

- Object* ok = obj->SetElement(0, *first, kNonStrictMode)->ToObjectChecked();
-
-  ok = obj->SetElement(1, *second, kNonStrictMode)->ToObjectChecked();
+  Object* ok =
+      obj->SetElement(0, *first, kNonStrictMode, true)->ToObjectChecked();
+
+ ok = obj->SetElement(1, *second, kNonStrictMode, true)->ToObjectChecked();

   // Make the clone.
   Handle<JSObject> clone = Copy(obj);
@@ -737,8 +739,8 @@
   clone->SetProperty(
       *second, Smi::FromInt(1), NONE, kNonStrictMode)->ToObjectChecked();

-  ok = clone->SetElement(0, *second, kNonStrictMode)->ToObjectChecked();
-  ok = clone->SetElement(1, *first, kNonStrictMode)->ToObjectChecked();
+ ok = clone->SetElement(0, *second, kNonStrictMode, true)->ToObjectChecked(); + ok = clone->SetElement(1, *first, kNonStrictMode, true)->ToObjectChecked();

   CHECK_EQ(obj->GetElement(1), clone->GetElement(0));
   CHECK_EQ(obj->GetElement(0), clone->GetElement(1));

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

Reply via email to