[es6] fix IsConcatSpreadable() algorithm in runtime-array.cc

The ordering of the "IsArray()" check for IsConcatSpreadable() was incorrect previously --- IsArray() is only used if Get(O, @@isConcatSpreadable) is undefined. Without this fix, it's not possible for Array subclasses to opt out of spreading

22.1.3.1.1 http://www.ecma-international.org/ecma-262/6.0/#sec-isconcatspreadable

BUG=v8:3764
LOG=N
R=arv@chromium.org, dslomov@chromium.org, rossberg@chromium.org

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

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

diff --git a/src/runtime/runtime-array.cc b/src/runtime/runtime-array.cc
index c468673..806af12 100644 (file)
--- a/src/runtime/runtime-array.cc
+++ b/src/runtime/runtime-array.cc
@@ -724,17 +724,18 @@ static bool IterateElements(Isolate* isolate, Handle<JSObject> receiver,
 static bool IsConcatSpreadable(Isolate* isolate, Handle<Object> obj) {
   HandleScope handle_scope(isolate);
   if (!obj->IsSpecObject()) return false;
-  if (obj->IsJSArray()) return true;
   if (FLAG_harmony_arrays) {
     Handle<Symbol> key(isolate->factory()->is_concat_spreadable_symbol());
     Handle<Object> value;
     MaybeHandle<Object> maybeValue =
         i::Runtime::GetObjectProperty(isolate, obj, key);
     if (maybeValue.ToHandle(&value)) {
-      return value->BooleanValue();
+      if (!value->IsUndefined()) {
+        return value->BooleanValue();
+      }
     }
   }
-  return false;
+  return obj->IsJSArray();
 }
 
 

diff --git a/test/mjsunit/harmony/array-concat.js b/test/mjsunit/harmony/array-concat.js
index c1ff92c..b623a5c 100644 (file)
--- a/test/mjsunit/harmony/array-concat.js
+++ b/test/mjsunit/harmony/array-concat.js
@@ -194,6 +194,15 @@ assertThrows(function() {
 
 (function testConcatArraySubclass() {
   "use strict";
+  // If @@isConcatSpreadable is not used, the value of IsArray(O)
+  // is used to determine the spreadable property.
+  class A extends Array {}
+  var obj = [].concat(new A(1, 2, 3), new A(4, 5, 6), new A(7, 8, 9));
+  assertEquals(9, obj.length);
+  for (var i = 0; i < obj.length; ++i) {
+    assertEquals(i + 1, obj[i]);
+  }
+
   // TODO(caitp): when concat is called on instances of classes which extend
   // Array, they should:
   //
@@ -203,6 +212,19 @@ assertThrows(function() {
 })();
 
 
+(function testConcatArraySubclassOptOut() {
+  "use strict";
+  class A extends Array {
+    get [Symbol.isConcatSpreadable]() { return false; }
+  }
+  var obj = [].concat(new A(1, 2, 3), new A(4, 5, 6), new A(7, 8, 9));
+  assertEquals(3, obj.length);
+  assertEquals(3, obj[0].length);
+  assertEquals(3, obj[1].length);
+  assertEquals(3, obj[2].length);
+})();
+
+
 (function testConcatNonArray() {
   "use strict";
   class NonArray {