Reviewers: ,

https://codereview.chromium.org/799853003/diff/1/src/runtime/runtime-array.cc
File src/runtime/runtime-array.cc (right):

https://codereview.chromium.org/799853003/diff/1/src/runtime/runtime-array.cc#newcode464
src/runtime/runtime-array.cc:464: // TODO(caitp): ToLength(Get(receiver,
"length")) should be a single op.
I'm not sure how big of a problem this is, but it seems simpler to just
call the JS implementation of ToLength() rather than write it in C++.
The perf-impact should only affect uses of Symbol.isConcatSpreadable, so
it probably won't matter much in most cases.

Description:
Use proper ToLength() operation in %ArrayConcat()

LOG=N
R=
BUG=

Please review this at https://codereview.chromium.org/799853003/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+52, -1 lines):
  M src/bootstrapper.cc
  M src/contexts.h
  M src/execution.h
  M src/execution.cc
  M src/runtime/runtime-array.cc
  M test/mjsunit/harmony/array-concat.js


Index: src/bootstrapper.cc
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index 1ec8afe9abaaa353cbe365bcfb70f5f0928ae9c5..24652c73036761fc7a0e0173f980c97db58c423d 100644
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -1525,6 +1525,7 @@ void Genesis::InstallNativeFunctions() {
   INSTALL_NATIVE(JSFunction, "ToInteger", to_integer_fun);
   INSTALL_NATIVE(JSFunction, "ToUint32", to_uint32_fun);
   INSTALL_NATIVE(JSFunction, "ToInt32", to_int32_fun);
+  INSTALL_NATIVE(JSFunction, "ToLength", to_length_fun);

   INSTALL_NATIVE(JSFunction, "GlobalEval", global_eval_fun);
   INSTALL_NATIVE(JSFunction, "Instantiate", instantiate_fun);
Index: src/contexts.h
diff --git a/src/contexts.h b/src/contexts.h
index ecc3151097e551865d48d73ec7d5d2b0015f6c56..cd3ff1421177fa9e41b691c5b0030ae91b195dfc 100644
--- a/src/contexts.h
+++ b/src/contexts.h
@@ -98,6 +98,7 @@ enum BindingFlags {
V(TO_INTEGER_FUN_INDEX, JSFunction, to_integer_fun) \ V(TO_UINT32_FUN_INDEX, JSFunction, to_uint32_fun) \ V(TO_INT32_FUN_INDEX, JSFunction, to_int32_fun) \ + V(TO_LENGTH_FUN_INDEX, JSFunction, to_length_fun) \ V(GLOBAL_EVAL_FUN_INDEX, JSFunction, global_eval_fun) \ V(INSTANTIATE_FUN_INDEX, JSFunction, instantiate_fun) \ V(CONFIGURE_INSTANCE_FUN_INDEX, JSFunction, configure_instance_fun) \
@@ -416,6 +417,7 @@ class Context: public FixedArray {
     ARRAY_VALUES_ITERATOR_INDEX,
     SCRIPT_CONTEXT_TABLE_INDEX,
     MAP_CACHE_INDEX,
+    TO_LENGTH_FUN_INDEX,

     // Properties from here are treated as weak references by the full GC.
     // Scavenge treats them as strong references.
Index: src/execution.cc
diff --git a/src/execution.cc b/src/execution.cc
index cfe61f963a87d41e86e153d2104bdc0791783c63..b4d19872fb46d77a31419856e31c706d6a027828 100644
--- a/src/execution.cc
+++ b/src/execution.cc
@@ -545,6 +545,12 @@ MaybeHandle<Object> Execution::ToInt32(
 }


+MaybeHandle<Object> Execution::ToLength(
+    Isolate* isolate, Handle<Object> obj) {
+  RETURN_NATIVE_CALL(to_length, { obj });
+}
+
+
 MaybeHandle<Object> Execution::NewDate(Isolate* isolate, double time) {
   Handle<Object> time_obj = isolate->factory()->NewNumber(time);
   RETURN_NATIVE_CALL(create_date, { time_obj });
Index: src/execution.h
diff --git a/src/execution.h b/src/execution.h
index 89175cd906d80a9533a47013ba2f0ec129b67fdf..ae263bddd7c42bd12e486b7890f2e741bc0a24f3 100644
--- a/src/execution.h
+++ b/src/execution.h
@@ -69,6 +69,11 @@ class Execution FINAL : public AllStatic {
   MUST_USE_RESULT static MaybeHandle<Object> ToUint32(
       Isolate* isolate, Handle<Object> obj);

+
+  // ES6, draft 10-14-14, section 7.1.15
+  MUST_USE_RESULT static MaybeHandle<Object> ToLength(
+      Isolate* isolate, Handle<Object> obj);
+
   // ECMA-262 9.8
   MUST_USE_RESULT static MaybeHandle<Object> ToString(
       Isolate* isolate, Handle<Object> obj);
Index: src/runtime/runtime-array.cc
diff --git a/src/runtime/runtime-array.cc b/src/runtime/runtime-array.cc
index 1df42c424c33f9ee06112b0e80d241b3ca0390f4..77c522f78c4643ebe9cb12e5dab20ff63f8cd8e2 100644
--- a/src/runtime/runtime-array.cc
+++ b/src/runtime/runtime-array.cc
@@ -461,7 +461,10 @@ static bool IterateElements(Isolate* isolate, Handle<JSObject> receiver,
     Handle<Object> key(isolate->heap()->length_string(), isolate);
     ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, val,
         Runtime::GetObjectProperty(isolate, receiver, key), false);
-    // TODO(caitp): implement ToLength() abstract operation for C++
+ // TODO(caitp): ToLength(Get(receiver, "length")) should be a single op.
+    ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, val,
+        Execution::ToLength(isolate, val), false);
+    // TODO(caitp): Support larger element indexes (up to 2^53-1).
     val->ToUint32(&length);
   }

Index: test/mjsunit/harmony/array-concat.js
diff --git a/test/mjsunit/harmony/array-concat.js b/test/mjsunit/harmony/array-concat.js index 738c81a45753d44653f0e0e6aabf6c4fbde4e4d0..1ab2dc28ac41b7d8db35d3f6d437ff52e818954f 100644
--- a/test/mjsunit/harmony/array-concat.js
+++ b/test/mjsunit/harmony/array-concat.js
@@ -40,6 +40,40 @@
 })();


+(function testConcatArrayLikeStringLength() {
+  "use strict";
+  var obj = {
+    "length": "6",
+    "1": "A",
+    "3": "B",
+    "5": "C"
+  };
+  obj[Symbol.isConcatSpreadable] = true;
+  var obj2 = { length: 3, "0": "0", "1": "1", "2": "2" };
+  var arr = ["X", "Y", "Z"];
+  assertEquals([void 0, "A", void 0, "B", void 0, "C",
+               { "length": 3, "0": "0", "1": "1", "2": "2" },
+ "X", "Y", "Z"], Array.prototype.concat.call(obj, obj2, arr));
+})();
+
+
+(function testConcatArrayLikeToLengthThrows() {
+  "use strict";
+  var obj = {
+    "length": {valueOf: null, toString: null},
+    "1": "A",
+    "3": "B",
+    "5": "C"
+  };
+  obj[Symbol.isConcatSpreadable] = true;
+  var obj2 = { length: 3, "0": "0", "1": "1", "2": "2" };
+  var arr = ["X", "Y", "Z"];
+  assertThrows(function() {
+    Array.prototype.concat.call(obj, obj2, arr);
+  }, TypeError);
+})();
+
+
 (function testConcatHoleyArray() {
   "use strict";
   var arr = [];


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to