Unify ToArrayLength conversions

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

Cr-Commit-Position: refs/heads/master@{#31310}

src/accessors.cc

@@ -198,18 +198,6 @@ void Accessors::ArrayLengthGetter(
 }
 
 
-// Tries to non-observably convert |value| to a valid array length.
-// Returns false if it fails.
-static bool FastAsArrayLength(Isolate* isolate, Handle<Object> value,
-                              uint32_t* length) {
-  if (value->ToArrayLength(length)) return true;
-  // We don't support AsArrayLength, so use AsArrayIndex for now. This just
-  // misses out on kMaxUInt32.
-  if (value->IsString()) return String::cast(*value)->AsArrayIndex(length);
-  return false;
-}
-
-
 void Accessors::ArrayLengthSetter(
     v8::Local<v8::Name> name,
     v8::Local<v8::Value> val,
@@ -222,28 +210,11 @@ void Accessors::ArrayLengthSetter(
   Handle<Object> length_obj = Utils::OpenHandle(*val);
 
   uint32_t length = 0;
-  if (!FastAsArrayLength(isolate, length_obj, &length)) {
-    Handle<Object> uint32_v;
-    if (!Object::ToUint32(isolate, length_obj).ToHandle(&uint32_v)) {
+  if (!JSArray::AnythingToArrayLength(isolate, length_obj, &length)) {
     isolate->OptionalRescheduleException(false);
     return;
   }
 
-    Handle<Object> number_v;
-    if (!Object::ToNumber(length_obj).ToHandle(&number_v)) {
-      isolate->OptionalRescheduleException(false);
-      return;
-    }
-
-    if (uint32_v->Number() != number_v->Number()) {
-      Handle<Object> exception = isolate->factory()->NewRangeError(
-          MessageTemplate::kInvalidArrayLength);
-      return isolate->ScheduleThrow(*exception);
-    }
-
-    CHECK(uint32_v->ToArrayLength(&length));
-  }
-
   if (JSArray::ObservableSetLength(array, length).is_null()) {
     isolate->OptionalRescheduleException(false);
   }
@@ -260,7 +231,6 @@ Handle<AccessorInfo> Accessors::ArrayLengthInfo(
 }
 
 
-
 //
 // Accessors::StringLength
 //

src/conversions-inl.h

@@ -70,7 +70,7 @@ inline unsigned int FastD2UI(double x) {
 
 
 inline float DoubleToFloat32(double x) {
-  // TODO(yanggou): This static_cast is implementation-defined behaviour in C++,
+  // TODO(yangguo): This static_cast is implementation-defined behaviour in C++,
   // so we may need to do the conversion manually instead to match the spec.
   volatile float f = static_cast<float>(x);
   return f;

src/objects-inl.h

@@ -2365,27 +2365,11 @@ void Struct::InitializeBody(int object_size) {
 }
 
 
-bool Object::ToArrayLength(uint32_t* index) {
-  if (IsSmi()) {
-    int value = Smi::cast(this)->value();
-    if (value < 0) return false;
-    *index = value;
-    return true;
-  }
-  if (IsHeapNumber()) {
-    double value = HeapNumber::cast(this)->value();
-    uint32_t uint_value = static_cast<uint32_t>(value);
-    if (value == static_cast<double>(uint_value)) {
-      *index = uint_value;
-      return true;
-    }
-  }
-  return false;
-}
+bool Object::ToArrayLength(uint32_t* index) { return Object::ToUint32(index); }
 
 
 bool Object::ToArrayIndex(uint32_t* index) {
-  return ToArrayLength(index) && *index != kMaxUInt32;
+  return Object::ToUint32(index) && *index != kMaxUInt32;
 }
 
 

src/objects.cc

@@ -715,15 +715,16 @@ bool Object::ToInt32(int32_t* value) {
 bool Object::ToUint32(uint32_t* value) {
   if (IsSmi()) {
     int num = Smi::cast(this)->value();
-    if (num >= 0) {
+    if (num < 0) return false;
     *value = static_cast<uint32_t>(num);
     return true;
   }
-  }
   if (IsHeapNumber()) {
     double num = HeapNumber::cast(this)->value();
-    if (num >= 0 && FastUI2D(FastD2UI(num)) == num) {
-      *value = FastD2UI(num);
+    if (num < 0) return false;
+    uint32_t uint_value = FastD2UI(num);
+    if (FastUI2D(uint_value) == num) {
+      *value = uint_value;
       return true;
     }
   }
@@ -6393,26 +6394,28 @@ bool JSArray::DefineOwnProperty(Isolate* isolate, Handle<JSArray> o,
 }
 
 
-// TODO(jkummerow): Consider unification with ArrayLengthSetter in accessors.cc.
-bool AnythingToArrayLength(Isolate* isolate, Handle<Object> length_obj,
+// Part of ES6 9.4.2.4 ArraySetLength.
+// static
+bool JSArray::AnythingToArrayLength(Isolate* isolate,
+                                    Handle<Object> length_object,
                                     uint32_t* output) {
   // Fast path: check numbers and strings that can be converted directly
   // and unobservably.
-  if (length_obj->ToUint32(output)) return true;
-  if (length_obj->IsString() &&
-      Handle<String>::cast(length_obj)->AsArrayIndex(output)) {
+  if (length_object->ToArrayLength(output)) return true;
+  if (length_object->IsString() &&
+      Handle<String>::cast(length_object)->AsArrayIndex(output)) {
     return true;
   }
   // Slow path: follow steps in ES6 9.4.2.4 "ArraySetLength".
   // 3. Let newLen be ToUint32(Desc.[[Value]]).
   Handle<Object> uint32_v;
-  if (!Object::ToUint32(isolate, length_obj).ToHandle(&uint32_v)) {
+  if (!Object::ToUint32(isolate, length_object).ToHandle(&uint32_v)) {
     // 4. ReturnIfAbrupt(newLen).
     return false;
   }
   // 5. Let numberLen be ToNumber(Desc.[[Value]]).
   Handle<Object> number_v;
-  if (!Object::ToNumber(length_obj).ToHandle(&number_v)) {
+  if (!Object::ToNumber(length_object).ToHandle(&number_v)) {
     // 6. ReturnIfAbrupt(newLen).
     return false;
   }
@@ -6423,7 +6426,8 @@ bool AnythingToArrayLength(Isolate* isolate, Handle<Object> length_obj,
     isolate->Throw(*exception);
     return false;
   }
-  return uint32_v->ToArrayLength(output);
+  CHECK(uint32_v->ToArrayLength(output));
+  return true;
 }
 
 

src/objects.h

@@ -10076,6 +10076,9 @@ class JSArray: public JSObject {
                                 Handle<Object> name, PropertyDescriptor* desc,
                                 ShouldThrow should_throw);
 
+  static bool AnythingToArrayLength(Isolate* isolate,
+                                    Handle<Object> length_object,
+                                    uint32_t* output);
   static bool ArraySetLength(Isolate* isolate, Handle<JSArray> a,
                              PropertyDescriptor* desc,
                              ShouldThrow should_throw);