Array.prototype.{reduce, reduceRight}: Wrong order of operations when determining initial value.

src/array.js

 // Copyright 2012 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.
 
 "use strict";
 
 // This file relies on the fact that the following declarations have been made
 // in runtime.js:
 // var $Array = global.Array;
 
@@ skipping 1376 matching lines @@
   CHECK_OBJECT_COERCIBLE(this, "Array.prototype.reduce");
 
   // Pull out the length so that modifications to the length in the
   // loop will not affect the looping and side effects are visible.
   var array = ToObject(this);
   var length = ToUint32(array.length);
 
   if (!IS_SPEC_FUNCTION(callback)) {
     throw MakeTypeError('called_non_callable', [callback]);
   }
+  if (length == 0 && IS_UNDEFINED(current)) {
+    throw MakeTypeError('reduce_no_initial', []);
+  }

[wingo, 2014/10/07 11:04:31]

I think the test should be %_ArgumentsLength() < 2 instead of
IS_UNDEFINED(current); explicitly passing undefined is fine.

[arv (Not doing code reviews), 2014/10/07 14:16:02]

Yup. The spec says: "If initialValue is present, then" so an args length check
is needed.

https://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.reduce

[Diego Pino, 2014/10/09 10:18:57]

What I intended to code with that line was step 7 of the algorithm.

7. If len is 0 and initialValue is not present, throw a TypeError exception.
(https://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.reduce).

However I realized is not needed actually, because the code below that searches
for initialValue, when is not defined (%_ArgumentsLength < 2), covers that case
too. There's already a test for this (array length equals 0 and current equals
undefined).

 
   var i = 0;
   find_initial: if (%_ArgumentsLength() < 2) {
     for (; i < length; i++) {
-      current = array[i];
-      if (!IS_UNDEFINED(current) || i in array) {
-        i++;
+      if (i in array) {
+        current = array[i++];
         break find_initial;
       }
     }
     throw MakeTypeError('reduce_no_initial', []);
   }
 
   var receiver = %GetDefaultReceiver(callback);
   var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(callback);
   for (; i < length; i++) {
     if (i in array) {
@@ skipping 10 matching lines @@
   CHECK_OBJECT_COERCIBLE(this, "Array.prototype.reduceRight");
 
   // Pull out the length so that side effects are visible before the
   // callback function is checked.
   var array = ToObject(this);
   var length = ToUint32(array.length);
 
   if (!IS_SPEC_FUNCTION(callback)) {
     throw MakeTypeError('called_non_callable', [callback]);
   }
+  if (length == 0 && IS_UNDEFINED(current)) {
+    throw MakeTypeError('reduce_no_initial', []);
+  }

[wingo, 2014/10/07 11:04:31]

likewise

[arv (Not doing code reviews), 2014/10/07 14:16:02]

Yup.
https://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.red...

 
   var i = length - 1;
   find_initial: if (%_ArgumentsLength() < 2) {
     for (; i >= 0; i--) {
-      current = array[i];
-      if (!IS_UNDEFINED(current) || i in array) {
-        i--;
+      if (i in array) {
+        current = array[i--];
         break find_initial;
       }
     }
     throw MakeTypeError('reduce_no_initial', []);
   }
 
   var receiver = %GetDefaultReceiver(callback);
   var stepping = DEBUG_IS_ACTIVE && %DebugCallbackSupportsStepping(callback);
   for (; i >= 0; i--) {
     if (i in array) {
@@ skipping 96 matching lines @@
   ));
 
   SetUpLockedPrototype(InternalPackedArray, $Array(), $Array(
     "join", getFunction("join", ArrayJoin),
     "pop", getFunction("pop", ArrayPop),
     "push", getFunction("push", ArrayPush)
   ));
 }
 
 SetUpArray();