SimpleSlice now calls [[Get]] before [[Has]] when generating copy

SparseSlice does not need this (non-optimal) reordering since its
callers guarantee that [[Get]] has no side effects on the passed-in array.

BUG=v8:3643
LOG=n
R=mstarzinger@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#24884}
git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24884 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/array.js b/src/array.js
index a4e681c..55dd797 100644 (file)
--- a/src/array.js
+++ b/src/array.js
@@ -284,11 +284,8 @@ function SparseMove(array, start_i, del_count, len, num_additional_args) {
 function SimpleSlice(array, start_i, del_count, len, deleted_elements) {
   for (var i = 0; i < del_count; i++) {
     var index = start_i + i;
-    // The spec could also be interpreted such that %HasOwnProperty
-    // would be the appropriate test.  We follow KJS in consulting the
-    // prototype.
-    var current = array[index];
-    if (!IS_UNDEFINED(current) || index in array) {
+    if (index in array) {
+      var current = array[index];
       // The spec requires [[DefineOwnProperty]] here, %AddElement is close
       // enough (in that it ignores the prototype).
       %AddElement(deleted_elements, i, current, NONE);

diff --git a/test/mjsunit/regress/regress-3643.js b/test/mjsunit/regress/regress-3643.js
new file mode 100644 (file)
index 0000000..cc61a1c
--- /dev/null
+++ b/test/mjsunit/regress/regress-3643.js
@@ -0,0 +1,21 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+var a = [1, 2, 3];
+Object.defineProperty(a, '1', {
+  get: function() { delete this[1]; return undefined; },
+  configurable: true
+});
+var s = a.slice(1);
+assertTrue('0' in s);
+
+// Sparse case should hit the same code as above due to presence of the getter.
+a = [1, 2, 3];
+a[0xffff] = 4;
+Object.defineProperty(a, '1', {
+  get: function() { delete this[1]; return undefined; },
+  configurable: true
+});
+s = a.slice(1);
+assertTrue('0' in s);