Handlification of JSArray::SetElementsLength().

R=verwaest@chromium.org, yangguo@chromium.org

Review URL: https://codereview.chromium.org/203333004

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

diff --git a/src/accessors.cc b/src/accessors.cc
index 99ea25a..dda1dee 100644 (file)
--- a/src/accessors.cc
+++ b/src/accessors.cc
@@ -213,7 +213,9 @@ MaybeObject* Accessors::ArraySetLength(Isolate* isolate,
   if (has_exception) return Failure::Exception();
 
   if (uint32_v->Number() == number_v->Number()) {
-    return array_handle->SetElementsLength(*uint32_v);
+    Handle<Object> result = JSArray::SetElementsLength(array_handle, uint32_v);
+    RETURN_IF_EMPTY_HANDLE(isolate, result);
+    return *result;
   }
   return isolate->Throw(
       *isolate->factory()->NewRangeError("invalid_array_length",

diff --git a/src/elements.cc b/src/elements.cc
index 0a9a40f..2e9bc32 100644 (file)
--- a/src/elements.cc
+++ b/src/elements.cc
@@ -737,6 +737,15 @@ class ElementsAccessorBase : public ElementsAccessor {
     return NULL;
   }
 
+  // TODO(ishell): Temporary wrapper until handlified.
+  MUST_USE_RESULT virtual Handle<Object> SetLength(
+      Handle<JSArray> array,
+      Handle<Object> length) {
+    CALL_HEAP_FUNCTION(array->GetIsolate(),
+                       SetLength(*array, *length),
+                       Object);
+  }
+
   MUST_USE_RESULT virtual MaybeObject* SetLength(JSArray* array,
                                                  Object* length) {
     return ElementsAccessorSubclass::SetLengthImpl(

diff --git a/src/elements.h b/src/elements.h
index bfca9c1..135e091 100644 (file)
--- a/src/elements.h
+++ b/src/elements.h
@@ -109,6 +109,9 @@ class ElementsAccessor {
   // changing array sizes as defined in EcmaScript 5.1 15.4.5.2, i.e. array that
   // have non-deletable elements can only be shrunk to the size of highest
   // element that is non-deletable.
+  MUST_USE_RESULT virtual Handle<Object> SetLength(
+      Handle<JSArray> holder,
+      Handle<Object> new_length) = 0;
   MUST_USE_RESULT virtual MaybeObject* SetLength(JSArray* holder,
                                                  Object* new_length) = 0;
 

diff --git a/src/ic.cc b/src/ic.cc
index e174904..399be0e 100644 (file)
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1813,11 +1813,11 @@ RUNTIME_FUNCTION(MaybeObject*, StoreIC_MissFromStubFailure) {
 
 
 RUNTIME_FUNCTION(MaybeObject*, StoreIC_ArrayLength) {
-  SealHandleScope shs(isolate);
+  HandleScope scope(isolate);
 
   ASSERT(args.length() == 2);
-  JSArray* receiver = JSArray::cast(args[0]);
-  Object* len = args[1];
+  Handle<JSArray> receiver = args.at<JSArray>(0);
+  Handle<Object> len = args.at<Object>(1);
 
   // The generated code should filter out non-Smis before we get here.
   ASSERT(len->IsSmi());
@@ -1829,11 +1829,9 @@ RUNTIME_FUNCTION(MaybeObject*, StoreIC_ArrayLength) {
   ASSERT(debug_lookup.IsPropertyCallbacks() && !debug_lookup.IsReadOnly());
 #endif
 
-  Object* result;
-  MaybeObject* maybe_result = receiver->SetElementsLength(len);
-  if (!maybe_result->To(&result)) return maybe_result;
-
-  return len;
+  RETURN_IF_EMPTY_HANDLE(isolate,
+                         JSArray::SetElementsLength(receiver, len));
+  return *len;
 }
 
 

diff --git a/src/objects.cc b/src/objects.cc
index 1bbb8c8..897c52a 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -11401,77 +11401,64 @@ static void EndPerformSplice(Handle<JSArray> object) {
 }
 
 
-// TODO(ishell): Temporary wrapper until handlified.
-// static
 Handle<Object> JSArray::SetElementsLength(Handle<JSArray> array,
-                                          Handle<Object> length) {
-  CALL_HEAP_FUNCTION(array->GetIsolate(),
-                     array->SetElementsLength(*length),
-                     Object);
-}
-
-
-MaybeObject* JSArray::SetElementsLength(Object* len) {
+                                          Handle<Object> new_length_handle) {
   // We should never end in here with a pixel or external array.
-  ASSERT(AllowsSetElementsLength());
-  if (!map()->is_observed())
-    return GetElementsAccessor()->SetLength(this, len);
+  ASSERT(array->AllowsSetElementsLength());
+  if (!array->map()->is_observed()) {
+    return array->GetElementsAccessor()->SetLength(array, new_length_handle);
+  }
 
-  Isolate* isolate = GetIsolate();
-  HandleScope scope(isolate);
-  Handle<JSArray> self(this);
+  Isolate* isolate = array->GetIsolate();
   List<uint32_t> indices;
   List<Handle<Object> > old_values;
-  Handle<Object> old_length_handle(self->length(), isolate);
-  Handle<Object> new_length_handle(len, isolate);
+  Handle<Object> old_length_handle(array->length(), isolate);
   uint32_t old_length = 0;
   CHECK(old_length_handle->ToArrayIndex(&old_length));
   uint32_t new_length = 0;
-  if (!new_length_handle->ToArrayIndex(&new_length))
-    return Failure::InternalError();
+  CHECK(new_length_handle->ToArrayIndex(&new_length));
 
   static const PropertyAttributes kNoAttrFilter = NONE;
-  int num_elements = self->NumberOfLocalElements(kNoAttrFilter);
+  int num_elements = array->NumberOfLocalElements(kNoAttrFilter);
   if (num_elements > 0) {
     if (old_length == static_cast<uint32_t>(num_elements)) {
       // Simple case for arrays without holes.
       for (uint32_t i = old_length - 1; i + 1 > new_length; --i) {
-        if (!GetOldValue(isolate, self, i, &old_values, &indices)) break;
+        if (!GetOldValue(isolate, array, i, &old_values, &indices)) break;
       }
     } else {
       // For sparse arrays, only iterate over existing elements.
       // TODO(rafaelw): For fast, sparse arrays, we can avoid iterating over
       // the to-be-removed indices twice.
       Handle<FixedArray> keys = isolate->factory()->NewFixedArray(num_elements);
-      self->GetLocalElementKeys(*keys, kNoAttrFilter);
+      array->GetLocalElementKeys(*keys, kNoAttrFilter);
       while (num_elements-- > 0) {
         uint32_t index = NumberToUint32(keys->get(num_elements));
         if (index < new_length) break;
-        if (!GetOldValue(isolate, self, index, &old_values, &indices)) break;
+        if (!GetOldValue(isolate, array, index, &old_values, &indices)) break;
       }
     }
   }
 
-  MaybeObject* result =
-      self->GetElementsAccessor()->SetLength(*self, *new_length_handle);
-  Handle<Object> hresult;
-  if (!result->ToHandle(&hresult, isolate)) return result;
+  Handle<Object> hresult =
+      array->GetElementsAccessor()->SetLength(array, new_length_handle);
+  RETURN_IF_EMPTY_HANDLE_VALUE(isolate, hresult, hresult);
 
-  CHECK(self->length()->ToArrayIndex(&new_length));
-  if (old_length == new_length) return *hresult;
+  CHECK(array->length()->ToArrayIndex(&new_length));
+  if (old_length == new_length) return hresult;
 
-  BeginPerformSplice(self);
+  BeginPerformSplice(array);
 
   for (int i = 0; i < indices.length(); ++i) {
     JSObject::EnqueueChangeRecord(
-        self, "delete", isolate->factory()->Uint32ToString(indices[i]),
+        array, "delete", isolate->factory()->Uint32ToString(indices[i]),
         old_values[i]);
   }
   JSObject::EnqueueChangeRecord(
-      self, "update", isolate->factory()->length_string(),
+      array, "update", isolate->factory()->length_string(),
       old_length_handle);
 
-  EndPerformSplice(self);
+  EndPerformSplice(array);
 
   uint32_t index = Min(old_length, new_length);
   uint32_t add_count = new_length > old_length ? new_length - old_length : 0;
@@ -11488,9 +11475,9 @@ MaybeObject* JSArray::SetElementsLength(Object* len) {
                 NONE, SLOPPY);
   }
 
-  EnqueueSpliceRecord(self, index, deleted, add_count);
+  EnqueueSpliceRecord(array, index, deleted, add_count);
 
-  return *hresult;
+  return hresult;
 }
 
 

diff --git a/src/objects.h b/src/objects.h
index 8508727..41c0d2a 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -10028,7 +10028,6 @@ class JSArray: public JSObject {
   // Can cause GC.
   static Handle<Object> SetElementsLength(Handle<JSArray> array,
                                           Handle<Object> length);
-  MUST_USE_RESULT MaybeObject* SetElementsLength(Object* length);
 
   // Set the content of the array to the content of storage.
   MUST_USE_RESULT inline MaybeObject* SetContent(FixedArrayBase* storage);

diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 8f3e4c8..ee96837 100644 (file)
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -759,7 +759,7 @@ TEST(JSArray) {
   JSArray::Initialize(array, 0);
 
   // Set array length to 0.
-  array->SetElementsLength(Smi::FromInt(0))->ToObjectChecked();
+  *JSArray::SetElementsLength(array, handle(Smi::FromInt(0), isolate));
   CHECK_EQ(Smi::FromInt(0), array->length());
   // Must be in fast mode.
   CHECK(array->HasFastSmiOrObjectElements());
@@ -772,7 +772,7 @@ TEST(JSArray) {
   // Set array length with larger than smi value.
   Handle<Object> length =
       factory->NewNumberFromUint(static_cast<uint32_t>(Smi::kMaxValue) + 1);
-  array->SetElementsLength(*length)->ToObjectChecked();
+  *JSArray::SetElementsLength(array, length);
 
   uint32_t int_length = 0;
   CHECK(length->ToArrayIndex(&int_length));