Bug 1259877 - Eliminate Invoke(JSContext*, const CallArgs&, MaybeConstruct = NO_CONSTRUCT) by 1) renaming it to a more-internal name, 2) adding an Invoke overload for existing InvokeArgs providers only, and 3) adding an InternalInvoke function to temporarily mark non-InvokeArgs places using the existing signature that will later be changed not to. r=efaust
authorJeff Walden <jwalden@mit.edu>
Mon, 21 Mar 2016 14:32:26 -0700
changeset 293099 b9d51ed412a7ccf1ad49e7fb1bb998720e23a7fa
parent 293098 f0d5faf75aa52770bffb0e39ac786b5c9fc101af
child 293100 b538123cb69fccccb70dd6afb50c7829349e2796
push id75096
push userjwalden@mit.edu
push dateThu, 14 Apr 2016 00:09:53 +0000
treeherdermozilla-inbound@1e6cb22a8971 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1259877
milestone48.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 1259877 - Eliminate Invoke(JSContext*, const CallArgs&, MaybeConstruct = NO_CONSTRUCT) by 1) renaming it to a more-internal name, 2) adding an Invoke overload for existing InvokeArgs providers only, and 3) adding an InternalInvoke function to temporarily mark non-InvokeArgs places using the existing signature that will later be changed not to. r=efaust
js/src/asmjs/AsmJS.cpp
js/src/jit/BaselineIC.cpp
js/src/jsfun.cpp
js/src/vm/Interpreter-inl.h
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
--- a/js/src/asmjs/AsmJS.cpp
+++ b/js/src/asmjs/AsmJS.cpp
@@ -7576,17 +7576,17 @@ HandleDynamicLinkFailure(JSContext* cx, 
                                               ? SourceBufferHolder::GiveOwnership
                                               : SourceBufferHolder::NoOwnership;
     SourceBufferHolder srcBuf(chars, end - begin, ownership);
     if (!frontend::CompileFunctionBody(cx, &fun, options, formals, srcBuf))
         return false;
 
     // Call the function we just recompiled.
     args.setCallee(ObjectValue(*fun));
-    return Invoke(cx, args, args.isConstructing() ? CONSTRUCT : NO_CONSTRUCT);
+    return InternalCallOrConstruct(cx, args, args.isConstructing() ? CONSTRUCT : NO_CONSTRUCT);
 }
 
 static WasmModuleObject*
 AsmJSModuleToModuleObject(JSFunction* fun)
 {
     MOZ_ASSERT(IsAsmJSModule(fun));
     const Value& v = fun->getExtendedSlot(FunctionExtended::WASM_MODULE_SLOT);
     return &v.toObject().as<WasmModuleObject>();
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -6108,17 +6108,17 @@ DoCallFallback(JSContext* cx, BaselineFr
                    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, callArgs.thisv(), nullptr);
             return false;
         }
 
-        if (!Invoke(cx, callArgs))
+        if (!InternalInvoke(cx, callArgs))
             return false;
 
         res.set(callArgs.rval());
     }
 
     TypeScript::Monitor(cx, script, pc, res);
 
     // Check if debug mode toggling made the stub invalid.
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1158,17 +1158,17 @@ js::fun_call(JSContext* cx, unsigned arg
     args.setThis(args.get(0));
 
     if (args.length() > 0) {
         for (size_t i = 0; i < args.length() - 1; i++)
             args[i].set(args[i + 1]);
         args = CallArgsFromVp(args.length() - 1, vp);
     }
 
-    return Invoke(cx, args);
+    return InternalInvoke(cx, args);
 }
 
 // ES5 15.3.4.3
 bool
 js::fun_apply(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
--- a/js/src/vm/Interpreter-inl.h
+++ b/js/src/vm/Interpreter-inl.h
@@ -838,17 +838,17 @@ class FastInvokeGuard
 
             if (script_->canIonCompile()) {
                 // This script is not yet hot. Since calling into Ion is much
                 // faster here, bump the warm-up counter a bit to account for this.
                 script_->incWarmUpCounter(5);
             }
         }
 
-        return Invoke(cx, args_);
+        return InternalCallOrConstruct(cx, args_, NO_CONSTRUCT);
     }
 
   private:
     FastInvokeGuard(const FastInvokeGuard& other) = delete;
     const FastInvokeGuard& operator=(const FastInvokeGuard& other) = delete;
 };
 
 }  /* namespace js */
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -436,19 +436,23 @@ struct AutoGCIfRequested
     ~AutoGCIfRequested() { runtime->gc.gcIfRequested(); }
 };
 
 /*
  * Find a function reference and its 'this' value implicit first parameter
  * under argc arguments on cx's stack, and call the function.  Push missing
  * required arguments, allocate declared local variables, and pop everything
  * when done.  Then push the return value.
+ *
+ * Note: This function DOES NOT call GetThisValue to munge |args.thisv()| if
+ *       necessary.  The caller (usually the interpreter) must have performed
+ *       this step already!
  */
 bool
-js::Invoke(JSContext* cx, const CallArgs& args, MaybeConstruct construct)
+js::InternalCallOrConstruct(JSContext* cx, const CallArgs& args, MaybeConstruct construct)
 {
     MOZ_ASSERT(args.length() <= ARGS_LENGTH_MAX);
     MOZ_ASSERT(!cx->zone()->types.activeAnalysis);
 
     /* Perform GC if necessary on exit from the function. */
     AutoGCIfRequested gcIfRequested(cx->runtime());
 
     unsigned skipForCallee = args.length() + 1 + (construct == CONSTRUCT);
@@ -520,17 +524,17 @@ js::Invoke(JSContext* cx, const Value& t
             !fval.toObject().as<JSFunction>().jitInfo() ||
             fval.toObject().as<JSFunction>().jitInfo()->needsOuterizedThisObject())
         {
             JSObject* thisObj = &args.thisv().toObject();
             args.mutableThisv().set(GetThisValue(thisObj));
         }
     }
 
-    if (!Invoke(cx, args))
+    if (!InternalCallOrConstruct(cx, args, NO_CONSTRUCT))
         return false;
 
     rval.set(args.rval());
     return true;
 }
 
 static bool
 InternalConstruct(JSContext* cx, const AnyConstructArgs& args)
@@ -547,17 +551,17 @@ InternalConstruct(JSContext* cx, const A
 
     JSObject& callee = args.callee();
     if (callee.is<JSFunction>()) {
         RootedFunction fun(cx, &callee.as<JSFunction>());
 
         if (fun->isNative())
             return CallJSNativeConstructor(cx, fun->native(), args);
 
-        if (!Invoke(cx, args, CONSTRUCT))
+        if (!InternalCallOrConstruct(cx, args, CONSTRUCT))
             return false;
 
         MOZ_ASSERT(args.CallArgs::rval().isObject());
         return true;
     }
 
     JSNative construct = callee.constructHook();
     MOZ_ASSERT(construct != nullptr, "IsConstructor without a construct hook?");
@@ -2719,17 +2723,17 @@ CASE(JSOP_STRICTEVAL)
     static_assert(JSOP_EVAL_LENGTH == JSOP_STRICTEVAL_LENGTH,
                   "eval and stricteval must be the same size");
 
     CallArgs args = CallArgsFromSp(GET_ARGC(REGS.pc), REGS.sp);
     if (REGS.fp()->scopeChain()->global().valueIsEval(args.calleev())) {
         if (!DirectEval(cx, args.get(0), args.rval()))
             goto error;
     } else {
-        if (!Invoke(cx, args))
+        if (!InternalInvoke(cx, args))
             goto error;
     }
 
     REGS.sp = args.spAfterCall();
     TypeScript::Monitor(cx, script, REGS.pc, REGS.sp[-1]);
 }
 END_CASE(JSOP_EVAL)
 
@@ -2801,17 +2805,17 @@ CASE(JSOP_FUNCALL)
             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))
+            if (!InternalInvoke(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/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -49,26 +49,49 @@ ReportIsNotFunction(JSContext* cx, Handl
                     MaybeConstruct construct = NO_CONSTRUCT);
 
 /* See ReportIsNotFunction comment for the meaning of numToSkip. */
 extern JSObject*
 ValueToCallable(JSContext* cx, HandleValue v, int numToSkip = -1,
                 MaybeConstruct construct = NO_CONSTRUCT);
 
 /*
- * Invoke assumes that the given args have been pushed on the top of the
- * VM stack.
+ * Call or construct arguments that are stored in rooted memory.
+ *
+ * NOTE: Any necessary |GetThisValue| computation must have been performed on
+ *       |args.thisv()|, likely by the interpreter when pushing |this| onto the
+ *       stack.  If you're not sure whether |GetThisValue| processing has been
+ *       performed, use |Invoke|.
  */
 extern bool
-Invoke(JSContext* cx, const CallArgs& args, MaybeConstruct construct = NO_CONSTRUCT);
+InternalCallOrConstruct(JSContext* cx, const CallArgs& args,
+                        MaybeConstruct construct);
+
+/* A call operation that'll be rewritten later in this patch stack. */
+inline bool
+Invoke(JSContext* cx, const AnyInvokeArgs& args)
+{
+    return InternalCallOrConstruct(cx, args, NO_CONSTRUCT);
+}
 
 /*
- * This Invoke overload places the least requirements on the caller: it may be
- * called at any time and it takes care of copying the given callee, this, and
- * arguments onto the stack.
+ * Similar to InternalCallOrConstruct, but for use in places that really
+ * shouldn't use such an internal method directly (and won't, later in this
+ * patch stack).
+ */
+inline bool
+InternalInvoke(JSContext* cx, const CallArgs& args)
+{
+    return InternalCallOrConstruct(cx, args, NO_CONSTRUCT);
+}
+
+/*
+ * Attempt to call a value with the provided |this| and arguments.  This
+ * function places no requirements on the caller: it may be called at any time,
+ * and it takes care of copying |fval|, |thisv|, and arguments onto the stack.
  */
 extern bool
 Invoke(JSContext* cx, const Value& thisv, const Value& fval, unsigned argc, const Value* argv,
        MutableHandleValue rval);
 
 /*
  * These helpers take care of the infinite-recursion check necessary for
  * getter/setter calls.