Bug 839420 - Refactor ReportIsNotFunction to not use spIndexOf and remove spIndexOf. r=bhackett
authorJan de Mooij <jdemooij@mozilla.com>
Sat, 09 Feb 2013 13:51:48 +0100
changeset 131310 e644bc44f39f65e08911a57cfe23517dea3f38f5
parent 131309 872e3eb6841edc1e9069546e71b4a79566968e42
child 131311 91a509f46a4907d2c7bb85289f166acde812d404
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs839420
milestone21.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 839420 - Refactor ReportIsNotFunction to not use spIndexOf and remove spIndexOf. r=bhackett
js/src/builtin/Object.cpp
js/src/builtin/ParallelArray.cpp
js/src/jit-test/tests/basic/bug839420.js
js/src/jsarray.cpp
js/src/jsinterp.cpp
js/src/jsinterp.h
js/src/jsinterpinlines.h
js/src/jsopcode.cpp
js/src/jsproxy.cpp
js/src/vm/Debugger.cpp
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -566,17 +566,17 @@ obj_watch(JSContext *cx, unsigned argc, 
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     if (args.length() <= 1) {
         js_ReportMissingArg(cx, args.calleev(), 1);
         return false;
     }
 
-    RootedObject callable(cx, ValueToCallable(cx, &args[1]));
+    RootedObject callable(cx, ValueToCallable(cx, args[1], args.length() - 2));
     if (!callable)
         return false;
 
     RootedId propid(cx);
     if (!ValueToId<CanGC>(cx, args[0], &propid))
         return false;
 
     RootedObject obj(cx, ToObject(cx, args.thisv()));
--- a/js/src/builtin/ParallelArray.cpp
+++ b/js/src/builtin/ParallelArray.cpp
@@ -1255,17 +1255,17 @@ ParallelArrayObject::construct(JSContext
     uint32_t length = iv.scalarLengthOfDimensions();
     double d = iv.dimensions[0];
     for (uint32_t i = 1; i < iv.dimensions.length(); i++)
         d *= iv.dimensions[i];
     if (d != static_cast<double>(length))
         return ReportBadLength(cx);
 
     // Extract second argument, the elemental function.
-    RootedObject elementalFun(cx, ValueToCallable(cx, &args[1]));
+    RootedObject elementalFun(cx, ValueToCallable(cx, args[1], args.length() - 2));
     if (!elementalFun)
         return false;
 
     // Create backing store.
     RootedObject buffer(cx, NewDenseArrayWithType(cx, length));
     if (!buffer)
         return false;
 
@@ -1296,17 +1296,17 @@ ParallelArrayObject::map(JSContext *cx, 
 
     RootedParallelArrayObject obj(cx, as(&args.thisv().toObject()));
 
     uint32_t outer = obj->outermostDimension();
     RootedObject buffer(cx, NewDenseArrayWithType(cx, outer));
     if (!buffer)
         return false;
 
-    RootedObject elementalFun(cx, ValueToCallable(cx, &args[0]));
+    RootedObject elementalFun(cx, ValueToCallable(cx, args[0], args.length() - 1));
     if (!elementalFun)
         return false;
 
 #ifdef DEBUG
     if (args.length() > 1) {
         DebugOptions options;
         if (!options.init(cx, args[1]) ||
             !options.check(cx, options.mode->map(cx, obj, elementalFun, buffer)))
@@ -1334,17 +1334,17 @@ ParallelArrayObject::reduce(JSContext *c
     uint32_t outer = obj->outermostDimension();
 
     // Throw if the array is empty.
     if (outer == 0) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_PAR_ARRAY_REDUCE_EMPTY);
         return false;
     }
 
-    RootedObject elementalFun(cx, ValueToCallable(cx, &args[0]));
+    RootedObject elementalFun(cx, ValueToCallable(cx, args[0], args.length() - 1));
     if (!elementalFun)
         return false;
 
 #ifdef DEBUG
     if (args.length() > 1) {
         DebugOptions options;
         if (!options.init(cx, args[1]))
             return false;
@@ -1372,17 +1372,17 @@ ParallelArrayObject::scan(JSContext *cx,
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_PAR_ARRAY_REDUCE_EMPTY);
         return false;
     }
 
     RootedObject buffer(cx, NewDenseArrayWithType(cx, outer));
     if (!buffer)
         return false;
 
-    RootedObject elementalFun(cx, ValueToCallable(cx, &args[0]));
+    RootedObject elementalFun(cx, ValueToCallable(cx, args[0], args.length() - 1));
     if (!elementalFun)
         return false;
 
     // Call reduce with a dummy out value to be discarded and a buffer to
     // store intermediates.
     RootedValue dummy(cx);
 
 #ifdef DEBUG
@@ -1423,17 +1423,17 @@ ParallelArrayObject::scatter(JSContext *
     if (args.length() >= 2)
         defaultValue = args[1];
     else
         defaultValue.setUndefined();
 
     // The conflict function is optional.
     RootedObject conflictFun(cx);
     if (args.length() >= 3 && !args[2].isUndefined()) {
-        conflictFun = ValueToCallable(cx, &args[2]);
+        conflictFun = ValueToCallable(cx, args[2], args.length() - 3);
         if (!conflictFun)
             return false;
     }
 
     // The length of the result array is optional and defaults to the length
     // of the source array.
     uint32_t resultLength;
     if (args.length() >= 4) {
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug839420.js
@@ -0,0 +1,17 @@
+function f() {
+    var x = undefined;
+    try {
+	[1, 2, 3].map(x);
+	assertEq(0, 1);
+    } catch(e) {
+	assertEq(e.toString().contains("x is not"), true);
+    }
+
+    try {
+	[1, 2, 3].filter(x, 1, 2);
+	assertEq(0, 1);
+    } catch(e) {
+	assertEq(e.toString().contains("x is not"), true);
+    }
+}
+f();
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -2179,17 +2179,17 @@ array_map(JSContext *cx, unsigned argc, 
     if (!GetLengthProperty(cx, obj, &len))
         return false;
 
     /* Step 4. */
     if (args.length() == 0) {
         js_ReportMissingArg(cx, args.calleev(), 0);
         return false;
     }
-    RootedObject callable(cx, ValueToCallable(cx, &args[0]));
+    RootedObject callable(cx, ValueToCallable(cx, args[0], args.length() - 1));
     if (!callable)
         return false;
 
     /* Step 5. */
     RootedValue thisv(cx, args.length() >= 2 ? args[1] : UndefinedValue());
 
     /* Step 6. */
     RootedObject arr(cx, NewDenseAllocatedArray(cx, len));
@@ -2258,17 +2258,17 @@ array_filter(JSContext *cx, unsigned arg
     if (!GetLengthProperty(cx, obj, &len))
         return false;
 
     /* Step 4. */
     if (args.length() == 0) {
         js_ReportMissingArg(cx, args.calleev(), 0);
         return false;
     }
-    RootedObject callable(cx, ValueToCallable(cx, &args[0]));
+    RootedObject callable(cx, ValueToCallable(cx, args[0], args.length() - 1));
     if (!callable)
         return false;
 
     /* Step 5. */
     RootedValue thisv(cx, args.length() >= 2 ? args[1] : UndefinedValue());
 
     /* Step 6. */
     RootedObject arr(cx, NewDenseAllocatedArray(cx, 0));
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -234,46 +234,36 @@ NoSuchMethod(JSContext *cx, unsigned arg
     JSBool ok = Invoke(cx, args);
     vp[0] = args.rval();
     return ok;
 }
 
 #endif /* JS_HAS_NO_SUCH_METHOD */
 
 bool
-js::ReportIsNotFunction(JSContext *cx, const Value &v, MaybeConstruct construct)
+js::ReportIsNotFunction(JSContext *cx, const Value &v, int numToSkip, MaybeConstruct construct)
 {
     unsigned error = construct ? JSMSG_NOT_CONSTRUCTOR : JSMSG_NOT_FUNCTION;
+    int spIndex = numToSkip >= 0 ? -(numToSkip + 1) : JSDVG_SEARCH_STACK;
 
     RootedValue val(cx, v);
-    js_ReportValueError3(cx, error, JSDVG_SEARCH_STACK, val, NullPtr(), NULL, NULL);
-    return false;
-}
-
-bool
-js::ReportIsNotFunction(JSContext *cx, const Value *vp, MaybeConstruct construct)
-{
-    ptrdiff_t spIndex = cx->stack.spIndexOf(vp);
-    unsigned error = construct ? JSMSG_NOT_CONSTRUCTOR : JSMSG_NOT_FUNCTION;
-
-    RootedValue val(cx, *vp);
     js_ReportValueError3(cx, error, spIndex, val, NullPtr(), NULL, NULL);
     return false;
 }
 
 JSObject *
-js::ValueToCallable(JSContext *cx, const Value *vp, MaybeConstruct construct)
+js::ValueToCallable(JSContext *cx, const Value &v, int numToSkip, MaybeConstruct construct)
 {
-    if (vp->isObject()) {
-        JSObject *callable = &vp->toObject();
+    if (v.isObject()) {
+        JSObject *callable = &v.toObject();
         if (callable->isCallable())
             return callable;
     }
 
-    ReportIsNotFunction(cx, vp, construct);
+    ReportIsNotFunction(cx, v, numToSkip, construct);
     return NULL;
 }
 
 bool
 js::RunScript(JSContext *cx, StackFrame *fp)
 {
     JS_ASSERT(fp == cx->fp());
     JSScript *script = fp->script();
@@ -348,30 +338,30 @@ js::InvokeKernel(JSContext *cx, CallArgs
 
     /* We should never enter a new script while cx->iterValue is live. */
     JS_ASSERT(cx->iterValue.isMagic(JS_NO_ITER_VALUE));
 
     /* MaybeConstruct is a subset of InitialFrameFlags */
     InitialFrameFlags initial = (InitialFrameFlags) construct;
 
     if (args.calleev().isPrimitive())
-        return ReportIsNotFunction(cx, args.calleev().address(), construct);
+        return ReportIsNotFunction(cx, args.calleev().get(), args.length() + 1, construct);
 
     JSObject &callee = args.callee();
     Class *clasp = callee.getClass();
 
     /* Invoke non-functions. */
     if (JS_UNLIKELY(clasp != &FunctionClass)) {
 #if JS_HAS_NO_SUCH_METHOD
         if (JS_UNLIKELY(clasp == &js_NoSuchMethodClass))
             return NoSuchMethod(cx, args.length(), args.base());
 #endif
         JS_ASSERT_IF(construct, !clasp->construct);
         if (!clasp->call)
-            return ReportIsNotFunction(cx, args.calleev().address(), construct);
+            return ReportIsNotFunction(cx, args.calleev().get(), args.length() + 1, construct);
         return CallJSNative(cx, clasp->call, args);
     }
 
     /* Invoke native functions. */
     JSFunction *fun = callee.toFunction();
     JS_ASSERT_IF(construct, !fun->isNativeConstructor());
     if (fun->isNative())
         return CallJSNative(cx, fun->native(), args);
@@ -431,42 +421,42 @@ js::Invoke(JSContext *cx, const Value &t
 bool
 js::InvokeConstructorKernel(JSContext *cx, CallArgs args)
 {
     JS_ASSERT(!FunctionClass.construct);
 
     args.setThis(MagicValue(JS_IS_CONSTRUCTING));
 
     if (!args.calleev().isObject())
-        return ReportIsNotFunction(cx, args.calleev().address(), CONSTRUCT);
+        return ReportIsNotFunction(cx, args.calleev().get(), args.length() + 1, CONSTRUCT);
 
     JSObject &callee = args.callee();
     if (callee.isFunction()) {
         JSFunction *fun = callee.toFunction();
 
         if (fun->isNativeConstructor()) {
             Probes::calloutBegin(cx, fun);
             bool ok = CallJSNativeConstructor(cx, fun->native(), args);
             Probes::calloutEnd(cx, fun);
             return ok;
         }
 
         if (!fun->isInterpretedConstructor())
-            return ReportIsNotFunction(cx, args.calleev().address(), CONSTRUCT);
+            return ReportIsNotFunction(cx, args.calleev().get(), args.length() + 1, CONSTRUCT);
 
         if (!InvokeKernel(cx, args, CONSTRUCT))
             return false;
 
         JS_ASSERT(args.rval().isObject());
         return true;
     }
 
     Class *clasp = callee.getClass();
     if (!clasp->construct)
-        return ReportIsNotFunction(cx, args.calleev().address(), CONSTRUCT);
+        return ReportIsNotFunction(cx, args.calleev().get(), args.length() + 1, CONSTRUCT);
 
     return CallJSNativeConstructor(cx, clasp->construct, args);
 }
 
 bool
 js::InvokeConstructor(JSContext *cx, const Value &fval, unsigned argc, Value *argv, Value *rval)
 {
     InvokeArgsGuard args;
--- a/js/src/jsinterp.h
+++ b/js/src/jsinterp.h
@@ -98,24 +98,28 @@ BoxNonStrictThis(JSContext *cx, MutableH
 inline bool
 ComputeThis(JSContext *cx, AbstractFramePtr frame);
 
 enum MaybeConstruct {
     NO_CONSTRUCT = INITIAL_NONE,
     CONSTRUCT = INITIAL_CONSTRUCT
 };
 
-extern bool
-ReportIsNotFunction(JSContext *cx, const Value &v, MaybeConstruct construct = NO_CONSTRUCT);
-
+/*
+ * numToSkip is the number of stack values the expression decompiler should skip
+ * before it reaches |v|. If it's -1, the decompiler will search the stack.
+ */
 extern bool
-ReportIsNotFunction(JSContext *cx, const Value *vp, MaybeConstruct construct = NO_CONSTRUCT);
+ReportIsNotFunction(JSContext *cx, const Value &v, int numToSkip = -1,
+                    MaybeConstruct construct = NO_CONSTRUCT);
 
+/* See ReportIsNotFunction comment for the meaning of numToSkip. */
 extern JSObject *
-ValueToCallable(JSContext *cx, const Value *vp, MaybeConstruct construct = NO_CONSTRUCT);
+ValueToCallable(JSContext *cx, const Value &vp, int numToSkip = -1,
+                MaybeConstruct construct = NO_CONSTRUCT);
 
 /*
  * InvokeKernel assumes that the given args have been pushed on the top of the
  * VM stack. Additionally, if 'args' is contained in a CallArgsList, that they
  * have already been marked 'active'.
  */
 extern bool
 InvokeKernel(JSContext *cx, CallArgs args, MaybeConstruct construct = NO_CONSTRUCT);
--- a/js/src/jsinterpinlines.h
+++ b/js/src/jsinterpinlines.h
@@ -1090,17 +1090,17 @@ UrshOperation(JSContext *cx, HandleScrip
 #undef RELATIONAL_OP
 
 inline JSFunction *
 ReportIfNotFunction(JSContext *cx, const Value &v, MaybeConstruct construct = NO_CONSTRUCT)
 {
     if (v.isObject() && v.toObject().isFunction())
         return v.toObject().toFunction();
 
-    ReportIsNotFunction(cx, v, construct);
+    ReportIsNotFunction(cx, v, -1, construct);
     return NULL;
 }
 
 /*
  * FastInvokeGuard is used to optimize calls to JS functions from natives written
  * in C++, for instance Array.map. If the callee is not Ion-compiled, this will
  * just call Invoke. If the callee has a valid IonScript, however, it will enter
  * Ion directly.
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -1498,16 +1498,19 @@ FindStartPC(JSContext *cx, ScriptFrameIt
         return true;
 
     *valuepc = NULL;
 
     PCStack pcstack;
     if (!pcstack.init(cx, iter.script(), current))
         return false;
 
+    if (spindex < 0 && spindex + pcstack.depth() < 0)
+        spindex = JSDVG_SEARCH_STACK;
+
     if (spindex == JSDVG_SEARCH_STACK) {
         size_t index = iter.numFrameSlots();
         JS_ASSERT(index >= size_t(pcstack.depth()));
 
         // We search from fp->sp to base to find the most recently calculated
         // value matching v under assumption that it is the value that caused
         // the exception.
         int stackHits = 0;
--- a/js/src/jsproxy.cpp
+++ b/js/src/jsproxy.cpp
@@ -3239,22 +3239,22 @@ proxy_createFunction(JSContext *cx, unsi
         return false;
     RootedObject proto(cx), parent(cx);
     parent = vp[0].toObject().getParent();
     proto = parent->global().getOrCreateFunctionPrototype(cx);
     if (!proto)
         return false;
     parent = proto->getParent();
 
-    RootedObject call(cx, ValueToCallable(cx, &vp[3]));
+    RootedObject call(cx, ValueToCallable(cx, vp[3], argc - 2));
     if (!call)
         return false;
     JSObject *construct = NULL;
     if (argc > 2) {
-        construct = ValueToCallable(cx, &vp[4]);
+        construct = ValueToCallable(cx, vp[4], argc - 3);
         if (!construct)
             return false;
     }
 
     JSObject *proxy = NewProxyObject(cx, &ScriptedIndirectProxyHandler::singleton,
                                      ObjectValue(*handler), proto, parent, call, construct);
     if (!proxy)
         return false;
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1717,17 +1717,17 @@ Debugger::getHookImpl(JSContext *cx, uns
 JSBool
 Debugger::setHookImpl(JSContext *cx, unsigned argc, Value *vp, Hook which)
 {
     JS_ASSERT(which >= 0 && which < HookCount);
     REQUIRE_ARGC("Debugger.setHook", 1);
     THIS_DEBUGGER(cx, argc, vp, "setHook", args, dbg);
     if (args[0].isObject()) {
         if (!args[0].toObject().isCallable())
-            return ReportIsNotFunction(cx, &args[0]);
+            return ReportIsNotFunction(cx, args[0], args.length() - 1);
     } else if (!args[0].isUndefined()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_NOT_CALLABLE_OR_UNDEFINED);
         return false;
     }
     dbg->object->setReservedSlot(JSSLOT_DEBUG_HOOK_START + which, args[0]);
     args.rval().setUndefined();
     return true;
 }
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -854,30 +854,16 @@ ContextStack::ContextStack(JSContext *cx
     cx_(cx)
 {}
 
 ContextStack::~ContextStack()
 {
     JS_ASSERT(!seg_);
 }
 
-ptrdiff_t
-ContextStack::spIndexOf(const Value *vp)
-{
-    if (!hasfp())
-        return JSDVG_SEARCH_STACK;
-
-    Value *base = fp()->base();
-    Value *sp = regs().sp;
-    if (vp < base || vp >= sp)
-        return JSDVG_SEARCH_STACK;
-
-    return vp - sp;
-}
-
 bool
 ContextStack::onTop() const
 {
     return seg_ && seg_ == space().seg_;
 }
 
 /*
  * This helper function brings the ContextStack to the top of the thread stack
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1636,24 +1636,16 @@ class ContextStack
     /*
      * Return whether there has been at least one frame pushed since the most
      * recent call to JS_SaveFrameChain. Note that natives do not have frames
      * hence this query has little semantic meaning past "you can call fp()".
      */
     inline bool hasfp() const { return seg_ && seg_->maybeRegs(); }
 
     /*
-     * Return the spindex value for 'vp' which can be used to call
-     * DecompileValueGenerator. (The spindex is either the negative offset of
-     * 'vp' from 'sp', if 'vp' points to a value in the innermost scripted
-     * stack frame, otherwise it is JSDVG_SEARCH_STACK.)
-     */
-    ptrdiff_t spIndexOf(const Value *vp);
-
-    /*
      * Return the most recent script activation's registers with the same
      * caveat as hasfp regarding JS_SaveFrameChain.
      */
     inline FrameRegs *maybeRegs() const { return seg_ ? seg_->maybeRegs() : NULL; }
     inline StackFrame *maybefp() const { return seg_ ? seg_->maybefp() : NULL; }
 
     /* Faster alternatives to maybe* functions. */
     inline FrameRegs &regs() const { JS_ASSERT(hasfp()); return seg_->regs(); }