Reviewers: rossberg, Vyacheslav Egorov,

Description:
Added no argument constructor support to external arrays.
ArrayType.call({}), ArrayType.apply({}) and new ArrayType() are all supported
now.

Modified existing no arg constructor test to assertDoesNotThrow().
Added number of new different tests.

Original issue: http://code.google.com/p/v8/issues/detail?id=1497

Contributed by [email protected]

Please review this at https://chromiumcodereview.appspot.com/9429015/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
  M     src/d8.cc
  M     test/mjsunit/external-array.js


Index: src/d8.cc
===================================================================
--- src/d8.cc   (revision 10785)
+++ src/d8.cc   (working copy)
@@ -330,16 +330,14 @@
   }
   ASSERT(element_size == 1 || element_size == 2 || element_size == 4 ||
          element_size == 8);
-  if (args.Length() == 0) {
-    return ThrowException(
-        String::New("Array constructor must have at least one "
-                    "parameter."));
-  }
+
   bool first_arg_is_array_buffer =
+      args.Length() > 0 &&
       args[0]->IsObject() &&
       args[0]->ToObject()->Get(
           String::New(kArrayBufferMarkerPropName))->IsTrue();
   // Currently, only the following constructors are supported:
+  //   TypedArray()
   //   TypedArray(unsigned long length)
   //   TypedArray(ArrayBuffer buffer,
   //              optional unsigned long byteOffset,
@@ -347,16 +345,21 @@
   if (args.Length() > 3) {
     return ThrowException(
         String::New("Array constructor from ArrayBuffer must "
-                    "have 1-3 parameters."));
+                    "have 0-3 parameters."));
   }

-  Local<Value> length_value = (args.Length() < 3)
-      ? (first_arg_is_array_buffer
-         ? args[0]->ToObject()->Get(String::New("length"))
-         : args[0])
-      : args[2];
-  size_t length = convertToUint(length_value, &try_catch);
-  if (try_catch.HasCaught()) return try_catch.Exception();
+  size_t length;
+  if (args.Length() > 0) {
+    Local<Value> length_value = (args.Length() < 3)
+        ? (first_arg_is_array_buffer
+           ? args[0]->ToObject()->Get(String::New("length"))
+           : args[0])
+        : args[2];
+    length = convertToUint(length_value, &try_catch);
+    if (try_catch.HasCaught()) return try_catch.Exception();
+  } else {
+    length = 0;
+  }

   void* data = NULL;
   size_t offset = 0;
@@ -428,7 +431,7 @@
   Persistent<Object> persistent_array = Persistent<Object>::New(array);
   persistent_array.MakeWeak(data, ExternalArrayWeakCallback);
   persistent_array.MarkIndependent();
-  if (data == NULL && length != 0) {
+  if (data == NULL) {
     data = calloc(length, element_size);
     if (data == NULL) {
       return ThrowException(String::New("Memory allocation failed."));
Index: test/mjsunit/external-array.js
===================================================================
--- test/mjsunit/external-array.js      (revision 10785)
+++ test/mjsunit/external-array.js      (working copy)
@@ -43,11 +43,11 @@
 assertEquals(0, a[0]);
 assertEquals(0, a[1]);

-// No-parameter constructor should fail right now.
+// No-parameter constructor shouldn't fail
 function abfunc1() {
   return new ArrayBuffer();
 }
-assertThrows(abfunc1);
+assertDoesNotThrow(abfunc1);

 // Test derivation from an ArrayBuffer
 var ab = new ArrayBuffer(12);
@@ -351,3 +351,37 @@
 %OptimizeFunctionOnNextCall(store_float64_undefined);
 store_float64_undefined(float64_array);
 assertTrue(isNaN(float64_array[0]));
+
+// test external arrays call() and apply() methods with no arguments
+function testNoArgCallApply(type) {
+  assertDoesNotThrow(type.call({}));
+  assertDoesNotThrow(type.apply({}));
+}
+
+// compare type.call() and new type() own property names
+function testNoArgCallAndNewOwnPropertyNames(type) {
+  var o1 = type.call({});
+  var o2 = new type();
+
+  assertPropertiesEqual(
+    Object.getOwnPropertyNames(o1),
+    Object.getOwnPropertyNames(o2)
+  );
+}
+
+// compare type.apply() and new type() own property names
+function testNoArgApplyAndNewOwnPropertyNames(type) {
+  var o1 = type.apply({});
+  var o2 = new type();
+
+  assertPropertiesEqual(
+    Object.getOwnPropertyNames(o1),
+    Object.getOwnPropertyNames(o2)
+  );
+}
+
+for (var t = 0; t < types.length; t++) {
+  testNoArgCallApply(types[t]);
+  testNoArgCallAndNewOwnPropertyNames(types[t]);
+  testNoArgApplyAndNewOwnPropertyNames(types[t]);
+}


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to