Improve error reporting for proxy handlers and __iterator__ (568966, r=brendan).
authorAndreas Gal <gal@mozilla.com>
Sat, 29 May 2010 19:04:01 -0700
changeset 43211 79c2b94a31e55cad2f1672091dc1706feab79b9b
parent 43210 92a66e7519b743d754459b7ff7c351ab3f7d6315
child 43212 327f3abee3c86f04b5c2c3f146941078466c79e4
push id13641
push userrsayre@mozilla.com
push dateSun, 06 Jun 2010 19:08:23 +0000
treeherdermozilla-central@5b3604a3cfbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbrendan
bugs568966
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Improve error reporting for proxy handlers and __iterator__ (568966, r=brendan).
js/src/js.msg
js/src/jsfun.cpp
js/src/jsfun.h
js/src/jsinterp.h
js/src/jsiter.cpp
js/src/jsobj.cpp
js/src/jsproxy.cpp
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -95,17 +95,17 @@ MSG_DEF(JSMSG_BAD_SORT_ARG,            1
 MSG_DEF(JSMSG_BAD_ATOMIC_NUMBER,       13, 1, JSEXN_INTERNALERR, "internal error: no index for atom {0}")
 MSG_DEF(JSMSG_TOO_MANY_LITERALS,       14, 0, JSEXN_INTERNALERR, "too many literals")
 MSG_DEF(JSMSG_CANT_WATCH,              15, 1, JSEXN_TYPEERR, "can't watch non-native objects of class {0}")
 MSG_DEF(JSMSG_STACK_UNDERFLOW,         16, 2, JSEXN_INTERNALERR, "internal error compiling {0}: stack underflow at pc {1}")
 MSG_DEF(JSMSG_NEED_DIET,               17, 1, JSEXN_INTERNALERR, "{0} too large")
 MSG_DEF(JSMSG_TOO_MANY_LOCAL_ROOTS,    18, 0, JSEXN_ERR, "out of local root space")
 MSG_DEF(JSMSG_READ_ONLY,               19, 1, JSEXN_ERR, "{0} is read-only")
 MSG_DEF(JSMSG_BAD_FORMAL,              20, 0, JSEXN_SYNTAXERR, "malformed formal parameter")
-MSG_DEF(JSMSG_BAD_ITERATOR,            21, 3, JSEXN_TYPEERR, "{0} has invalid {1} value {2}")
+MSG_DEF(JSMSG_UNUSED21,                21, 0, JSEXN_NONE, "")
 MSG_DEF(JSMSG_NOT_FUNCTION,            22, 1, JSEXN_TYPEERR, "{0} is not a function")
 MSG_DEF(JSMSG_NOT_CONSTRUCTOR,         23, 1, JSEXN_TYPEERR, "{0} is not a constructor")
 MSG_DEF(JSMSG_SCRIPT_STACK_QUOTA,      24, 0, JSEXN_INTERNALERR, "script stack space quota is exhausted")
 MSG_DEF(JSMSG_TOO_DEEP,                25, 1, JSEXN_INTERNALERR, "{0} nested too deeply")
 MSG_DEF(JSMSG_OVER_RECURSED,           26, 0, JSEXN_INTERNALERR, "too much recursion")
 MSG_DEF(JSMSG_IN_NOT_OBJECT,           27, 1, JSEXN_TYPEERR, "invalid 'in' operand {0}")
 MSG_DEF(JSMSG_BAD_NEW_RESULT,          28, 1, JSEXN_TYPEERR, "invalid new expression result {0}")
 MSG_DEF(JSMSG_BAD_SHARP_DEF,           29, 1, JSEXN_ERR, "invalid sharp variable definition #{0}=")
@@ -286,17 +286,17 @@ MSG_DEF(JSMSG_BAD_SURROGATE_CHAR,     20
 MSG_DEF(JSMSG_UTF8_CHAR_TOO_LARGE,    204, 1, JSEXN_TYPEERR, "UTF-8 character {0} too large")
 MSG_DEF(JSMSG_MALFORMED_UTF8_CHAR,    205, 1, JSEXN_TYPEERR, "malformed UTF-8 character sequence at offset {0}")
 MSG_DEF(JSMSG_USER_DEFINED_ERROR,     206, 0, JSEXN_ERR, "JS_ReportError was called")
 MSG_DEF(JSMSG_WRONG_CONSTRUCTOR,      207, 1, JSEXN_TYPEERR, "wrong constructor called for {0}")
 MSG_DEF(JSMSG_BAD_GENERATOR_RETURN,   208, 1, JSEXN_TYPEERR, "generator function {0} returns a value")
 MSG_DEF(JSMSG_BAD_ANON_GENERATOR_RETURN, 209, 0, JSEXN_TYPEERR, "anonymous generator function returns a value")
 MSG_DEF(JSMSG_NAME_AFTER_FOR_PAREN,   210, 0, JSEXN_SYNTAXERR, "missing name after for (")
 MSG_DEF(JSMSG_IN_AFTER_FOR_NAME,      211, 0, JSEXN_SYNTAXERR, "missing in after for")
-MSG_DEF(JSMSG_BAD_ITERATOR_RETURN,    212, 1, JSEXN_TYPEERR, "{0}.__iterator__ returned a primitive value")
+MSG_DEF(JSMSG_BAD_TRAP_RETURN_VALUE,  212, 2, JSEXN_TYPEERR,"trap {1} for {0} returned a primitive value")
 MSG_DEF(JSMSG_KEYWORD_NOT_NS,         213, 0, JSEXN_SYNTAXERR, "keyword is used as namespace")
 MSG_DEF(JSMSG_BAD_GENERATOR_YIELD,    214, 1, JSEXN_TYPEERR, "yield from closing generator {0}")
 MSG_DEF(JSMSG_BAD_GENERATOR_SYNTAX,   215, 1, JSEXN_SYNTAXERR, "{0} expression must be parenthesized")
 MSG_DEF(JSMSG_ARRAY_COMP_LEFTSIDE,    216, 0, JSEXN_SYNTAXERR, "invalid array comprehension left-hand side")
 MSG_DEF(JSMSG_NON_XML_FILTER,         217, 1, JSEXN_TYPEERR, "XML filter is applied to non-XML value {0}")
 MSG_DEF(JSMSG_EMPTY_ARRAY_REDUCE,     218, 0, JSEXN_TYPEERR, "reduce of empty array with no initial value")
 MSG_DEF(JSMSG_NON_LIST_XML_METHOD,    219, 2, JSEXN_TYPEERR, "cannot call {0} method on an XML list with {1} elements")
 MSG_DEF(JSMSG_BAD_DELETE_OPERAND,     220, 0, JSEXN_SYNTAXERR, "invalid delete operand")
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -2616,49 +2616,28 @@ js_ValueToCallableObject(JSContext *cx, 
         return callable;
     }
     return js_ValueToFunctionObject(cx, vp, flags);
 }
 
 void
 js_ReportIsNotFunction(JSContext *cx, jsval *vp, uintN flags)
 {
-    uintN error;
     const char *name = NULL, *source = NULL;
     AutoValueRooter tvr(cx);
-    if (flags & JSV2F_ITERATOR) {
-        error = JSMSG_BAD_ITERATOR;
-        name = js_iterator_str;
-        JSString *src = js_ValueToSource(cx, *vp);
-        if (!src)
-            return;
-        tvr.setString(src);
-        JSString *qsrc = js_QuoteString(cx, src, 0);
-        if (!qsrc)
-            return;
-        tvr.setString(qsrc);
-        source = js_GetStringBytes(cx, qsrc);
-        if (!source)
-            return;
-    } else if (flags & JSV2F_CONSTRUCT) {
-        error = JSMSG_NOT_CONSTRUCTOR;
-    } else {
-        error = JSMSG_NOT_FUNCTION;
-    }
-
+    uintN error = (flags & JSV2F_CONSTRUCT) ? JSMSG_NOT_CONSTRUCTOR : JSMSG_NOT_FUNCTION;
     LeaveTrace(cx);
     FrameRegsIter i(cx);
     while (!i.done() && !i.pc())
         ++i;
 
     ptrdiff_t spindex =
-        !i.done() && StackBase(i.fp()) <= vp && vp < i.sp()
-            ? vp - i.sp()
-            : flags & JSV2F_SEARCH_STACK ? JSDVG_SEARCH_STACK
-                                         : JSDVG_IGNORE_STACK;
+        (!i.done() && StackBase(i.fp()) <= vp && vp < i.sp())
+        ? vp - i.sp()
+        : ((flags & JSV2F_SEARCH_STACK) ? JSDVG_SEARCH_STACK : JSDVG_IGNORE_STACK);
 
     js_ReportValueError3(cx, error, spindex, *vp, NULL, name, source);
 }
 
 /*
  * When a function has between 2 and MAX_ARRAY_LOCALS arguments and variables,
  * their name are stored as the JSLocalNames.array.
  */
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -335,17 +335,16 @@ js_DefineFunction(JSContext *cx, JSObjec
                   uintN nargs, uintN flags);
 
 /*
  * Flags for js_ValueToFunction and js_ReportIsNotFunction.  We depend on the
  * fact that JSINVOKE_CONSTRUCT (aka JSFRAME_CONSTRUCTING) is 1, and test that
  * with #if/#error in jsfun.c.
  */
 #define JSV2F_CONSTRUCT         JSINVOKE_CONSTRUCT
-#define JSV2F_ITERATOR          JSINVOKE_ITERATOR
 #define JSV2F_SEARCH_STACK      0x10000
 
 extern JSFunction *
 js_ValueToFunction(JSContext *cx, jsval *vp, uintN flags);
 
 extern JSObject *
 js_ValueToFunctionObject(JSContext *cx, jsval *vp, uintN flags);
 
--- a/js/src/jsinterp.h
+++ b/js/src/jsinterp.h
@@ -62,19 +62,18 @@ enum JSFrameFlags {
     JSFRAME_COMPUTED_THIS      =  0x02, /* frame.thisv was computed already and
                                            JSVAL_IS_OBJECT(thisv) */
     JSFRAME_ASSIGNING          =  0x04, /* a complex (not simplex JOF_ASSIGNING) op
                                            is currently assigning to a property */
     JSFRAME_DEBUGGER           =  0x08, /* frame for JS_EvaluateInStackFrame */
     JSFRAME_EVAL               =  0x10, /* frame for obj_eval */
     JSFRAME_FLOATING_GENERATOR =  0x20, /* frame copy stored in a generator obj */
     JSFRAME_YIELDING           =  0x40, /* js_Interpret dispatched JSOP_YIELD */
-    JSFRAME_ITERATOR           =  0x80, /* trying to get an iterator for for-in */
-    JSFRAME_GENERATOR          = 0x200, /* frame belongs to generator-iterator */
-    JSFRAME_OVERRIDE_ARGS      = 0x400, /* overridden arguments local variable */
+    JSFRAME_GENERATOR          =  0x80, /* frame belongs to generator-iterator */
+    JSFRAME_OVERRIDE_ARGS      = 0x100, /* overridden arguments local variable */
 
     JSFRAME_SPECIAL            = JSFRAME_DEBUGGER | JSFRAME_EVAL
 };
 
 /*
  * JS stack frame, may be allocated on the C stack by native callers.  Always
  * allocated on cx->stackPool for calls from the interpreter to an interpreted
  * function.
@@ -293,22 +292,21 @@ js_Invoke(JSContext *cx, const js::Invok
  *   js_ValueToFunction
  *   js_ValueToFunctionObject
  *   js_ValueToCallableObject
  *   js_ReportIsNotFunction
  *
  * See jsfun.h for the latter four and flag renaming macros.
  */
 #define JSINVOKE_CONSTRUCT      JSFRAME_CONSTRUCTING
-#define JSINVOKE_ITERATOR       JSFRAME_ITERATOR
 
 /*
  * Mask to isolate construct and iterator flags for use with jsfun.h functions.
  */
-#define JSINVOKE_FUNFLAGS       (JSINVOKE_CONSTRUCT | JSINVOKE_ITERATOR)
+#define JSINVOKE_FUNFLAGS       JSINVOKE_CONSTRUCT
 
 /*
  * "Internal" calls may come from C or C++ code using a JSContext on which no
  * JS is running (!cx->fp), so they may need to push a dummy JSStackFrame.
  */
 #define js_InternalCall(cx,obj,fval,argc,argv,rval)                           \
     js_InternalInvoke(cx, obj, fval, 0, argc, argv, rval)
 
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -358,20 +358,26 @@ GetCustomIterator(JSContext *cx, JSObjec
 
     /* If there is no custom __iterator__ method, we are done here. */
     if (*vp == JSVAL_VOID)
         return true;
 
     /* Otherwise call it and return that object. */
     LeaveTrace(cx);
     jsval arg = BOOLEAN_TO_JSVAL((flags & JSITER_FOREACH) == 0);
-    if (!js_InternalInvoke(cx, obj, *vp, JSINVOKE_ITERATOR, 1, &arg, vp))
+    if (!js_InternalCall(cx, obj, *vp, 1, &arg, vp))
         return false;
     if (JSVAL_IS_PRIMITIVE(*vp)) {
-        js_ReportValueError(cx, JSMSG_BAD_ITERATOR_RETURN, JSDVG_SEARCH_STACK, *vp, NULL);
+        /*
+         * We are always coming from js_ValueToIterator, and we are no longer on
+         * trace, so the object we are iterating over is on top of the stack (-1).
+         */
+        js_ReportValueError2(cx, JSMSG_BAD_TRAP_RETURN_VALUE,
+                             -1, OBJECT_TO_JSVAL(obj), NULL,
+                             js_AtomToPrintableString(cx, atom));
         return false;
     }
     return true;
 }
 
 template <typename T>
 static inline bool
 Compare(T *a, T *b, size_t c)
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -5627,17 +5627,17 @@ js_Call(JSContext *cx, JSObject *obj, ui
         }
         if (JSVAL_IS_OBJECT(fval) && JSVAL_TO_OBJECT(fval) != callee) {
             argv[-2] = fval;
             ok = js_Call(cx, obj, argc, argv, rval);
             argv[-2] = OBJECT_TO_JSVAL(callee);
             return ok;
         }
 #endif
-        js_ReportIsNotFunction(cx, &argv[-2], js_GetTopStackFrame(cx)->flags & JSFRAME_ITERATOR);
+        js_ReportIsNotFunction(cx, &argv[-2], js_GetTopStackFrame(cx)->flags);
         return JS_FALSE;
     }
     return clasp->call(cx, obj, argc, argv, rval);
 }
 
 JSBool
 js_Construct(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
              jsval *rval)
@@ -6642,18 +6642,16 @@ js_DumpStackFrame(JSContext *cx, JSStack
         if (fp->flags & JSFRAME_ASSIGNING)
             fprintf(stderr, " assigning");
         if (fp->flags & JSFRAME_DEBUGGER)
             fprintf(stderr, " debugger");
         if (fp->flags & JSFRAME_EVAL)
             fprintf(stderr, " eval");
         if (fp->flags & JSFRAME_YIELDING)
             fprintf(stderr, " yielding");
-        if (fp->flags & JSFRAME_ITERATOR)
-            fprintf(stderr, " iterator");
         if (fp->flags & JSFRAME_GENERATOR)
             fprintf(stderr, " generator");
         if (fp->flags & JSFRAME_OVERRIDE_ARGS)
             fprintf(stderr, " overridden_args");
         fputc('\n', stderr);
 
         if (fp->scopeChain)
             fprintf(stderr, "  scopeChain: (JSObject *) %p\n", (void *) fp->scopeChain);
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -496,35 +496,49 @@ class JSScriptedProxyHandler : public JS
 JSScriptedProxyHandler::JSScriptedProxyHandler()
 {
 }
 
 JSScriptedProxyHandler::~JSScriptedProxyHandler()
 {
 }
 
+static bool
+ReturnedValueMustNotBePrimitive(JSContext *cx, JSObject *proxy, JSAtom *atom, jsval v)
+{
+    if (JSVAL_IS_PRIMITIVE(v)) {
+        js_ReportValueError2(cx, JSMSG_BAD_TRAP_RETURN_VALUE,
+                             JSDVG_SEARCH_STACK, OBJECT_TO_JSVAL(proxy), NULL,
+                             js_AtomToPrintableString(cx, atom));
+        return false;
+    }
+    return true;
+}
+
 bool
 JSScriptedProxyHandler::getPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id,
                                               JSPropertyDescriptor *desc)
 {
     JSObject *handler = JSVAL_TO_OBJECT(proxy->getProxyHandler());
     AutoValueRooter tvr(cx);
     return FundamentalTrap(cx, handler, ATOM(getPropertyDescriptor), tvr.addr()) &&
            TryHandlerTrap(cx, proxy, Trap1(cx, handler, tvr.value(), id, tvr.addr())) &&
+           ReturnedValueMustNotBePrimitive(cx, proxy, ATOM(getPropertyDescriptor), tvr.value()) &&
            ParsePropertyDescriptorObject(cx, proxy, id, tvr.value(), desc);
 }
 
 bool
 JSScriptedProxyHandler::getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id,
                                                  JSPropertyDescriptor *desc)
 {
     JSObject *handler = JSVAL_TO_OBJECT(proxy->getProxyHandler());
     AutoValueRooter tvr(cx);
     return FundamentalTrap(cx, handler, ATOM(getOwnPropertyDescriptor), tvr.addr()) &&
            TryHandlerTrap(cx, proxy, Trap1(cx, handler, tvr.value(), id, tvr.addr())) &&
+           ReturnedValueMustNotBePrimitive(cx, proxy, ATOM(getPropertyDescriptor), tvr.value()) &&
            ParsePropertyDescriptorObject(cx, proxy, id, tvr.value(), desc);
 }
 
 bool
 JSScriptedProxyHandler::defineProperty(JSContext *cx, JSObject *proxy, jsid id,
                                        JSPropertyDescriptor *desc)
 {
     JSObject *handler = JSVAL_TO_OBJECT(proxy->getProxyHandler());
@@ -642,36 +656,26 @@ JSScriptedProxyHandler::enumerateOwn(JSC
         return false;
     if (!js_IsCallable(tvr.value()))
         return JSProxyHandler::enumerateOwn(cx, proxy, idap);
     return TryHandlerTrap(cx, proxy, Trap(cx, handler, tvr.value(), 0, NULL, tvr.addr())) &&
            ArrayToJSIdArray(cx, tvr.value(), idap);
 }
 
 bool
-ReportErrorIfPrimitive(JSContext *cx, jsval *vp)
-{
-    if (JSVAL_IS_PRIMITIVE(*vp)) {
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_NOT_NONNULL_OBJECT);
-        return false;
-    }
-    return true;
-}
-
-bool
 JSScriptedProxyHandler::iterate(JSContext *cx, JSObject *proxy, uintN flags, jsval *vp)
 {
     JSObject *handler = JSVAL_TO_OBJECT(proxy->getProxyHandler());
     AutoValueRooter tvr(cx);
     if (!DerivedTrap(cx, handler, ATOM(iterate), tvr.addr()))
         return false;
     if (!js_IsCallable(tvr.value()))
         return JSProxyHandler::iterate(cx, proxy, flags, vp);
     return TryHandlerTrap(cx, proxy, Trap(cx, handler, tvr.value(), 0, NULL, vp)) &&
-           ReportErrorIfPrimitive(cx, vp);
+           ReturnedValueMustNotBePrimitive(cx, proxy, ATOM(iterate), *vp);
 }
 
 const void *
 JSScriptedProxyHandler::family()
 {
     return &singleton;
 }