Bug 1216379 - Throw less cryptic error message when using a non-iterable in a for...of loop. r=jorendorff
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 08 Dec 2015 15:28:23 -0500
changeset 310063 7d5dfdfe0150615d9ad01d2178ca43db0bf3645d
parent 310062 38fbb8418ffcddc43061ec9b1e347efaede8e72f
child 310064 99dd62654b77b3ca88987caad6fbd829c725a6f6
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1216379
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1216379 - Throw less cryptic error message when using a non-iterable in a for...of loop. r=jorendorff
js/src/frontend/BytecodeEmitter.cpp
js/src/jit-test/tests/basic/iterable-error-messages.js
js/src/jit/BaselineCompiler.cpp
js/src/jit/BaselineCompiler.h
js/src/jit/BaselineIC.cpp
js/src/jit/IonBuilder.cpp
js/src/jsopcode.cpp
js/src/jsopcode.h
js/src/tests/js1_8_5/extensions/decompile-for-of.js
js/src/vm/Interpreter.cpp
js/src/vm/Opcodes.h
js/src/vm/Xdr.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -5370,19 +5370,19 @@ BytecodeEmitter::emitIterator()
     if (!emit1(JSOP_DUP))                                         // OBJ OBJ
         return false;
     if (!emit2(JSOP_SYMBOL, uint8_t(JS::SymbolCode::iterator)))   // OBJ OBJ @@ITERATOR
         return false;
     if (!emitElemOpBase(JSOP_CALLELEM))                           // OBJ ITERFN
         return false;
     if (!emit1(JSOP_SWAP))                                        // ITERFN OBJ
         return false;
-    if (!emitCall(JSOP_CALL, 0))                                  // ITER
-        return false;
-    checkTypeSet(JSOP_CALL);
+    if (!emitCall(JSOP_CALLITER, 0))                              // ITER
+        return false;
+    checkTypeSet(JSOP_CALLITER);
     return true;
 }
 
 bool
 BytecodeEmitter::emitForInOrOfVariables(ParseNode* pn)
 {
     MOZ_ASSERT(pn->isKind(PNK_VAR) || pn->isKind(PNK_LET));
 
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/iterable-error-messages.js
@@ -0,0 +1,39 @@
+function assertThrowsMsg(f, msg) {
+    try {
+        f();
+        assertEq(0, 1);
+    } catch(e) {
+        assertEq(e instanceof TypeError, true);
+        assertEq(e.message, msg);
+    }
+}
+
+// For-of
+function testForOf(val) {
+    for (var x of val) {}
+}
+for (v of [{}, Math, new Proxy({}, {})]) {
+    assertThrowsMsg(() => testForOf(v), "val is not iterable");
+}
+assertThrowsMsg(() => testForOf(null), "val is null");
+assertThrowsMsg(() => { for (var x of () => 1) {}}, "() => 1 is not iterable");
+
+// Destructuring
+function testDestr(val) {
+    var [a, b] = val;
+}
+for (v of [{}, Math, new Proxy({}, {})]) {
+    assertThrowsMsg(() => testDestr(v), "val is not iterable");
+}
+assertThrowsMsg(() => testDestr(null), "val is null");
+assertThrowsMsg(() => { [a, b] = () => 1; }, "() => 1 is not iterable");
+
+// Spread
+function testSpread(val) {
+    [...val];
+}
+for (v of [{}, Math, new Proxy({}, {})]) {
+    assertThrowsMsg(() => testSpread(v), "val is not iterable");
+}
+assertThrowsMsg(() => testSpread(null), "val is null");
+assertThrowsMsg(() => { [...() => 1]; }, "() => 1 is not iterable");
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -3094,16 +3094,22 @@ BaselineCompiler::emitSpreadCall()
 
 bool
 BaselineCompiler::emit_JSOP_CALL()
 {
     return emitCall();
 }
 
 bool
+BaselineCompiler::emit_JSOP_CALLITER()
+{
+    return emitCall();
+}
+
+bool
 BaselineCompiler::emit_JSOP_NEW()
 {
     return emitCall();
 }
 
 bool
 BaselineCompiler::emit_JSOP_SUPERCALL()
 {
--- a/js/src/jit/BaselineCompiler.h
+++ b/js/src/jit/BaselineCompiler.h
@@ -151,16 +151,17 @@ namespace jit {
     _(JSOP_SETARG)             \
     _(JSOP_CHECKLEXICAL)       \
     _(JSOP_INITLEXICAL)        \
     _(JSOP_INITGLEXICAL)       \
     _(JSOP_CHECKALIASEDLEXICAL) \
     _(JSOP_INITALIASEDLEXICAL) \
     _(JSOP_UNINITIALIZED)      \
     _(JSOP_CALL)               \
+    _(JSOP_CALLITER)           \
     _(JSOP_FUNCALL)            \
     _(JSOP_FUNAPPLY)           \
     _(JSOP_NEW)                \
     _(JSOP_EVAL)               \
     _(JSOP_STRICTEVAL)         \
     _(JSOP_SPREADCALL)         \
     _(JSOP_SPREADNEW)          \
     _(JSOP_SPREADEVAL)         \
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -6137,20 +6137,26 @@ DoCallFallback(JSContext* cx, BaselineFr
     } else if ((op == JSOP_EVAL || op == JSOP_STRICTEVAL) &&
                frame->scopeChain()->global().valueIsEval(callee))
     {
         if (!DirectEval(cx, CallArgsFromVp(argc, vp)))
             return false;
         res.set(vp[0]);
     } else {
         MOZ_ASSERT(op == JSOP_CALL ||
+                   op == JSOP_CALLITER ||
                    op == JSOP_FUNCALL ||
                    op == JSOP_FUNAPPLY ||
                    op == JSOP_EVAL ||
                    op == JSOP_STRICTEVAL);
+        if (op == JSOP_CALLITER && callee.isPrimitive()) {
+            MOZ_ASSERT(argc == 0, "thisv must be on top of the stack");
+            ReportValueError(cx, JSMSG_NOT_ITERABLE, -1, thisv, nullptr);
+            return false;
+        }
         if (!Invoke(cx, thisv, callee, argc, args, res))
             return false;
     }
 
     TypeScript::Monitor(cx, script, pc, res);
 
     // Check if debug mode toggling made the stub invalid.
     if (stub.invalid())
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -1875,16 +1875,17 @@ IonBuilder::inspectOpcode(JSOp op)
 
       case JSOP_FUNCALL:
         return jsop_funcall(GET_ARGC(pc));
 
       case JSOP_FUNAPPLY:
         return jsop_funapply(GET_ARGC(pc));
 
       case JSOP_CALL:
+      case JSOP_CALLITER:
       case JSOP_NEW:
       case JSOP_SUPERCALL:
         return jsop_call(GET_ARGC(pc), (JSOp)*pc == JSOP_NEW || (JSOp)*pc == JSOP_SUPERCALL);
 
       case JSOP_EVAL:
       case JSOP_STRICTEVAL:
         return jsop_eval(GET_ARGC(pc));
 
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -125,17 +125,17 @@ js::StackUses(JSScript* script, jsbyteco
     switch (op) {
       case JSOP_POPN:
         return GET_UINT16(pc);
       case JSOP_NEW:
       case JSOP_SUPERCALL:
         return 2 + GET_ARGC(pc) + 1;
       default:
         /* stack: fun, this, [argc arguments] */
-        MOZ_ASSERT(op == JSOP_CALL || op == JSOP_EVAL ||
+        MOZ_ASSERT(op == JSOP_CALL || op == JSOP_EVAL || op == JSOP_CALLITER ||
                    op == JSOP_STRICTEVAL || op == JSOP_FUNCALL || op == JSOP_FUNAPPLY);
         return 2 + GET_ARGC(pc);
     }
 }
 
 unsigned
 js::StackDefs(JSScript* script, jsbytecode* pc)
 {
@@ -1162,16 +1162,17 @@ ExpressionDecompiler::decompilePC(jsbyte
         return write(js_undefined_str);
       case JSOP_GLOBALTHIS:
         // |this| could convert to a very long object initialiser, so cite it by
         // its keyword name.
         return write(js_this_str);
       case JSOP_NEWTARGET:
         return write("new.target");
       case JSOP_CALL:
+      case JSOP_CALLITER:
       case JSOP_FUNCALL:
         return decompilePCForStackOperand(pc, -int32_t(GET_ARGC(pc) + 2)) &&
                write("(...)");
       case JSOP_SPREADCALL:
         return decompilePCForStackOperand(pc, -int32_t(3)) &&
                write("(...)");
       case JSOP_NEWARRAY:
         return write("[]");
--- a/js/src/jsopcode.h
+++ b/js/src/jsopcode.h
@@ -71,17 +71,17 @@ enum {
     JOF_DETECTING       = 1 << 14,  /* object detection for warning-quelling */
     /* 1 << 15 is unused */
     JOF_LEFTASSOC       = 1 << 16,  /* left-associative operator */
     /* 1 << 17 is unused */
     /* 1 << 18 is unused */
     JOF_CHECKSLOPPY     = 1 << 19,  /* Op can only be generated in sloppy mode */
     JOF_CHECKSTRICT     = 1 << 20,  /* Op can only be generated in strict mode */
     JOF_INVOKE          = 1 << 21,  /* JSOP_CALL, JSOP_FUNCALL, JSOP_FUNAPPLY,
-                                       JSOP_NEW, JSOP_EVAL */
+                                       JSOP_NEW, JSOP_EVAL, JSOP_CALLITER */
     /* 1 << 22 is unused */
     /* 1 << 23 is unused */
     /* 1 << 24 is unused */
     JOF_GNAME           = 1 << 25,  /* predicted global name */
     JOF_TYPESET         = 1 << 26,  /* has an entry in a script's type sets */
     JOF_ARITH           = 1 << 27   /* unary or binary arithmetic opcode */
 };
 
--- a/js/src/tests/js1_8_5/extensions/decompile-for-of.js
+++ b/js/src/tests/js1_8_5/extensions/decompile-for-of.js
@@ -1,27 +1,27 @@
 // The decompiler can handle the implicit call to @@iterator in a for-of loop.
 
 var x;
-function check(code) {
+function check(code, msg) {
     var s = "no exception thrown";
     try {
         eval(code);
     } catch (exc) {
         s = exc.message;
     }
 
-    assertEq(s, `x[Symbol.iterator] is not a function`);
+    assertEq(s, msg);
 }
 
 x = {};
-check("for (var v of x) throw fit;");
-check("[...x]");
-check("Math.hypot(...x)");
+check("for (var v of x) throw fit;", "x is not iterable");
+check("[...x]", "x is not iterable");
+check("Math.hypot(...x)", "x is not iterable");
 
 x[Symbol.iterator] = "potato";
-check("for (var v of x) throw fit;");
+check("for (var v of x) throw fit;", "x is not iterable");
 
 x[Symbol.iterator] = {};
-check("for (var v of x) throw fit;");
+check("for (var v of x) throw fit;", "x[Symbol.iterator] is not a function");
 
 if (typeof reportCompare === "function")
     reportCompare(0, 0, "ok");
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -286,17 +286,17 @@ MakeDefaultConstructor(JSContext* cx, JS
 }
 
 bool
 js::ReportIsNotFunction(JSContext* cx, HandleValue v, int numToSkip, MaybeConstruct construct)
 {
     unsigned error = construct ? JSMSG_NOT_CONSTRUCTOR : JSMSG_NOT_FUNCTION;
     int spIndex = numToSkip >= 0 ? -(numToSkip + 1) : JSDVG_SEARCH_STACK;
 
-    ReportValueError3(cx, error, spIndex, v, nullptr, nullptr, nullptr);
+    ReportValueError(cx, error, spIndex, v, nullptr);
     return false;
 }
 
 JSObject*
 js::ValueToCallable(JSContext* cx, HandleValue v, int numToSkip, MaybeConstruct construct)
 {
     if (v.isObject() && v.toObject().isCallable()) {
         return &v.toObject();
@@ -1735,17 +1735,16 @@ CASE(EnableInterruptsPseudoOpcode)
     DISPATCH_TO(op);
 }
 
 /* Various 1-byte no-ops. */
 CASE(JSOP_NOP)
 CASE(JSOP_UNUSED14)
 CASE(JSOP_UNUSED65)
 CASE(JSOP_BACKPATCH)
-CASE(JSOP_UNUSED145)
 CASE(JSOP_UNUSED163)
 CASE(JSOP_UNUSED177)
 CASE(JSOP_UNUSED178)
 CASE(JSOP_UNUSED179)
 CASE(JSOP_UNUSED180)
 CASE(JSOP_UNUSED181)
 CASE(JSOP_UNUSED182)
 CASE(JSOP_UNUSED183)
@@ -2732,16 +2731,17 @@ CASE(JSOP_FUNAPPLY)
     CallArgs args = CallArgsFromSp(GET_ARGC(REGS.pc), REGS.sp);
     if (!GuardFunApplyArgumentsOptimization(cx, REGS.fp(), args))
         goto error;
     /* FALL THROUGH */
 }
 
 CASE(JSOP_NEW)
 CASE(JSOP_CALL)
+CASE(JSOP_CALLITER)
 CASE(JSOP_SUPERCALL)
 CASE(JSOP_FUNCALL)
 {
     if (REGS.fp()->hasPushedSPSFrame())
         cx->runtime()->spsProfiler.updatePC(script, REGS.pc);
 
     bool construct = (*REGS.pc == JSOP_NEW || *REGS.pc == JSOP_SUPERCALL);
     unsigned argStackSlots = GET_ARGC(REGS.pc) + construct;
@@ -2755,16 +2755,21 @@ CASE(JSOP_FUNCALL)
     /* Don't bother trying to fast-path calls to scripted non-constructors. */
     if (!isFunction || !maybeFun->isInterpreted() || !maybeFun->isConstructor() ||
         (!construct && maybeFun->isClassConstructor()))
     {
         if (construct) {
             if (!ConstructFromStack(cx, args))
                 goto error;
         } else {
+            if (*REGS.pc == JSOP_CALLITER && args.calleev().isPrimitive()) {
+                MOZ_ASSERT(args.length() == 0, "thisv must be on top of the stack");
+                ReportValueError(cx, JSMSG_NOT_ITERABLE, -1, args.thisv(), nullptr);
+                goto error;
+            }
             if (!Invoke(cx, args))
                 goto error;
         }
         Value* newsp = args.spAfterCall();
         TypeScript::Monitor(cx, script, REGS.pc, newsp[-1]);
         REGS.sp = newsp;
         ADVANCE_AND_DISPATCH(JSOP_CALL_LENGTH);
     }
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -1469,17 +1469,26 @@ 1234567890123456789012345678901234567890
     /*
      * Stores the top stack value in the specified intrinsic.
      *   Category: Variables and Scopes
      *   Type: Intrinsics
      *   Operands: uint32_t nameIndex
      *   Stack: val => val
      */ \
     macro(JSOP_SETINTRINSIC,  144, "setintrinsic",  NULL, 5,  1,  1, JOF_ATOM|JOF_NAME|JOF_SET|JOF_DETECTING) \
-    macro(JSOP_UNUSED145,     145, "unused145",     NULL, 1,  0,  0, JOF_BYTE) \
+    /*
+     * Like JSOP_CALL, but used as part of for-of and destructuring bytecode
+     * to provide better error messages.
+     *   Category: Statements
+     *   Type: Function
+     *   Operands: uint16_t argc (must be 0)
+     *   Stack: callee, this => rval
+     *   nuses: 2
+     */ \
+    macro(JSOP_CALLITER,      145, "calliter",      NULL, 3, -1,  1, JOF_UINT16|JOF_INVOKE|JOF_TYPESET) \
     /*
      * Initialize a non-configurable, non-writable, non-enumerable data-property on an object.
      *
      * Pops the top two values on the stack as 'val' and 'obj', defines
      * 'nameIndex' property of 'obj' as 'val', pushes 'obj' onto the stack.
      *   Category: Literals
      *   Type: Object
      *   Operands: uint32_t nameIndex
--- a/js/src/vm/Xdr.h
+++ b/js/src/vm/Xdr.h
@@ -24,17 +24,17 @@ namespace js {
  * versions.  If deserialization fails, the data should be invalidated if
  * possible.
  *
  * When you change this, run make_opcode_doc.py and copy the new output into
  * this wiki page:
  *
  *  https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/Bytecode
  */
-static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 325;
+static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 326;
 static const uint32_t XDR_BYTECODE_VERSION =
     uint32_t(0xb973c0de - XDR_BYTECODE_VERSION_SUBTRAHEND);
 
 static_assert(JSErr_Limit == 422,
               "GREETINGS, POTENTIAL SUBTRAHEND INCREMENTER! If you added or "
               "removed MSG_DEFs from js.msg, you should increment "
               "XDR_BYTECODE_VERSION_SUBTRAHEND and update this assertion's "
               "expected JSErr_Limit value.");