Backed out changeset 0fc1b3aba102 (bug 1178653) for js bustage CLOSED TREE
authorWes Kocher <wkocher@mozilla.com>
Mon, 17 Aug 2015 20:16:45 -0700
changeset 258163 26bcc69528819206fdf787f92e6a7c49b66062c2
parent 258162 84e4016eb3adf22318fb5cc2b905dbd8e6c72354
child 258164 4b1fccda7a74443b3093efe8d224dad547f0f36b
push id17041
push userryanvm@gmail.com
push dateTue, 18 Aug 2015 15:00:03 +0000
treeherderb2g-inbound@d5cf4e7900df [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1178653
milestone43.0a1
backs out0fc1b3aba102358fe1ac3af8faf6c81faf1ea70d
Backed out changeset 0fc1b3aba102 (bug 1178653) for js bustage CLOSED TREE
js/src/builtin/Reflect.cpp
js/src/jit/BaselineIC.cpp
js/src/jit/VMFunctions.cpp
js/src/jsapi.cpp
js/src/jsarray.cpp
js/src/jsfun.cpp
js/src/proxy/DirectProxyHandler.cpp
js/src/proxy/ScriptedDirectProxyHandler.cpp
js/src/proxy/ScriptedIndirectProxyHandler.cpp
js/src/tests/ecma_6/extensions/new-cross-compartment.js
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
js/src/vm/Stack.h
--- a/js/src/builtin/Reflect.cpp
+++ b/js/src/builtin/Reflect.cpp
@@ -19,36 +19,35 @@ using namespace js;
 
 /*** Reflect methods *****************************************************************************/
 
 /*
  * ES draft rev 32 (2015 Feb 2) 7.3.17 CreateListFromArrayLike.
  * The elementTypes argument is not supported. The result list is
  * pushed to *args.
  */
-template <class InvokeArgs>
 static bool
-InitArgsFromArrayLike(JSContext* cx, HandleValue v, InvokeArgs* args)
+InitArgsFromArrayLike(JSContext* cx, HandleValue v, InvokeArgs* args, bool construct)
 {
     // Step 3.
     RootedObject obj(cx, NonNullObject(cx, v));
     if (!obj)
         return false;
 
     // Steps 4-5.
     uint32_t len;
     if (!GetLengthProperty(cx, obj, &len))
         return false;
 
     // Allocate space for the arguments.
     if (len > ARGS_LENGTH_MAX) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_TOO_MANY_FUN_APPLY_ARGS);
         return false;
     }
-    if (!args->init(len))
+    if (!args->init(len, construct))
         return false;
 
     // Steps 6-8.
     for (uint32_t index = 0; index < len; index++) {
         if (!GetElement(cx, obj, obj, index, (*args)[index]))
             return false;
     }
 
@@ -67,17 +66,17 @@ Reflect_apply(JSContext* cx, unsigned ar
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NOT_FUNCTION,
                              "Reflect.apply argument");
         return false;
     }
 
     // Steps 2-3.
     FastInvokeGuard fig(cx, args.get(0));
     InvokeArgs& invokeArgs = fig.args();
-    if (!InitArgsFromArrayLike(cx, args.get(2), &invokeArgs))
+    if (!InitArgsFromArrayLike(cx, args.get(2), &invokeArgs, false))
         return false;
     invokeArgs.setCallee(args.get(0));
     invokeArgs.setThis(args.get(1));
 
     // Steps 4-5. This is specified to be a tail call, but isn't.
     if (!fig.invoke(cx))
         return false;
     args.rval().set(invokeArgs.rval());
@@ -104,22 +103,28 @@ Reflect_construct(JSContext* cx, unsigne
         if (!IsConstructor(newTarget)) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NOT_CONSTRUCTOR,
                                  "Reflect.construct argument 3");
             return false;
         }
     }
 
     // Step 4-5.
-    ConstructArgs constructArgs(cx);
-    if (!InitArgsFromArrayLike(cx, args.get(1), &constructArgs))
+    InvokeArgs invokeArgs(cx);
+    if (!InitArgsFromArrayLike(cx, args.get(1), &invokeArgs, true))
         return false;
+    invokeArgs.setCallee(args.get(0));
+    invokeArgs.setThis(MagicValue(JS_THIS_POISON));
+    invokeArgs.newTarget().set(newTarget);
 
     // Step 6.
-    return Construct(cx, args.get(0), constructArgs, newTarget, args.rval());
+    if (!InvokeConstructor(cx, invokeArgs))
+        return false;
+    args.rval().set(invokeArgs.rval());
+    return true;
 }
 
 /* ES6 26.1.3 Reflect.defineProperty(target, propertyKey, attributes) */
 static bool
 Reflect_defineProperty(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -9998,35 +9998,17 @@ DoCallFallback(JSContext* cx, BaselineFr
     bool handled = false;
     if (!TryAttachCallStub(cx, stub, script, pc, op, argc, vp, constructing, false,
                            createSingleton, &handled))
     {
         return false;
     }
 
     if (op == JSOP_NEW) {
-        // Callees from the stack could have any old non-constructor callee.
-        if (!IsConstructor(callee)) {
-            ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, callee, nullptr);
-            return false;
-        }
-
-        ConstructArgs cargs(cx);
-        if (!cargs.init(argc))
-            return false;
-
-        for (uint32_t i = 0; i < argc; i++)
-            cargs[i].set(args[i]);
-
-        RootedValue newTarget(cx, args[argc]);
-        MOZ_ASSERT(IsConstructor(newTarget),
-                   "either callee == newTarget, or the initial |new| checked "
-                   "that IsConstructor(newTarget)");
-
-        if (!Construct(cx, callee, cargs, newTarget, res))
+        if (!InvokeConstructor(cx, callee, argc, args, true, res))
             return false;
     } 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 {
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -58,42 +58,26 @@ VMFunction::addToFunctions()
 
 bool
 InvokeFunction(JSContext* cx, HandleObject obj, bool constructing, uint32_t argc, Value* argv,
                MutableHandleValue rval)
 {
     AutoArrayRooter argvRoot(cx, argc + 1 + constructing, argv);
 
     // Data in the argument vector is arranged for a JIT -> JIT call.
-    RootedValue thisv(cx, argv[0]);
+    Value thisv = argv[0];
     Value* argvWithoutThis = argv + 1;
 
-    RootedValue fval(cx, ObjectValue(*obj));
-    if (constructing) {
-        ConstructArgs cargs(cx);
-        if (!cargs.init(argc))
-            return false;
-
-        for (uint32_t i = 0; i < argc; i++)
-            cargs[i].set(argvWithoutThis[i]);
-
-        RootedValue newTarget(cx, argvWithoutThis[argc]);
+    // For constructing functions, |this| is constructed at caller side and we can just call Invoke.
+    // When creating this failed / is impossible at caller site, i.e. MagicValue(JS_IS_CONSTRUCTING),
+    // we use InvokeConstructor that creates it at the callee side.
+    if (thisv.isMagic(JS_IS_CONSTRUCTING))
+        return InvokeConstructor(cx, ObjectValue(*obj), argc, argvWithoutThis, true, rval);
 
-        // If |this| hasn't been created, we can use normal construction code.
-        if (thisv.isMagic(JS_IS_CONSTRUCTING))
-            return Construct(cx, fval, cargs, newTarget, rval);
-
-        // Otherwise the default |this| has already been created.  We could
-        // almost perform a *call* at this point, but we'd break |new.target|
-        // in the function.  So in this one weird case we call a one-off
-        // construction path that *won't* set |this| to JS_IS_CONSTRUCTING.
-        return InternalConstructWithProvidedThis(cx, fval, thisv, cargs, newTarget, rval);
-    }
-
-    return Invoke(cx, thisv, fval, argc, argvWithoutThis, rval);
+    return Invoke(cx, thisv, ObjectValue(*obj), argc, argvWithoutThis, rval);
 }
 
 bool
 InvokeFunctionShuffleNewTarget(JSContext* cx, HandleObject obj, uint32_t numActualArgs,
                                uint32_t numFormalArgs, Value* argv, MutableHandleValue rval)
 {
     MOZ_ASSERT(numFormalArgs > numActualArgs);
     argv[1 + numActualArgs] = argv[1 + numFormalArgs];
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4610,77 +4610,87 @@ JS_PUBLIC_API(bool)
 JS::Construct(JSContext* cx, HandleValue fval, const JS::HandleValueArray& args,
               MutableHandleValue rval)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, fval, args);
     AutoLastFrameCheck lfc(cx);
 
-    if (!IsConstructor(fval)) {
-        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, fval, nullptr);
-        return false;
-    }
-
-    ConstructArgs cargs(cx);
-    if (!FillArgumentsFromArraylike(cx, cargs, args))
-        return false;
-
-    return js::Construct(cx, fval, cargs, fval, rval);
+    return InvokeConstructor(cx, fval, args.length(), args.begin(), false, rval);
 }
 
 JS_PUBLIC_API(bool)
 JS::Construct(JSContext* cx, HandleValue fval, HandleObject newTarget, const JS::HandleValueArray& args,
               MutableHandleValue rval)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, fval, newTarget, args);
     AutoLastFrameCheck lfc(cx);
 
-    if (!IsConstructor(fval)) {
-        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, fval, nullptr);
+    // Reflect.construct ensures that the supplied new.target value is a
+    // constructor. Frankly, this makes good sense, so we reproduce the check.
+    if (!newTarget->isConstructor()) {
+        RootedValue val(cx, ObjectValue(*newTarget));
+        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, val, nullptr);
         return false;
     }
 
-    RootedValue newTargetVal(cx, ObjectValue(*newTarget));
-    if (!IsConstructor(newTargetVal)) {
-        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, newTargetVal, nullptr);
+    // This is a littlesilly, but we need to convert from what's useful for our
+    // consumers to what we can actually handle internally.
+    AutoValueVector argv(cx);
+    unsigned argc = args.length();
+    if (!argv.reserve(argc + 1))
         return false;
+    for (unsigned i = 0; i < argc; i++) {
+        argv.infallibleAppend(args[i]);
     }
-
-    ConstructArgs cargs(cx);
-    if (!FillArgumentsFromArraylike(cx, cargs, args))
-        return false;
-
-    return js::Construct(cx, fval, cargs, newTargetVal, rval);
+    argv.infallibleAppend(ObjectValue(*newTarget));
+
+    return InvokeConstructor(cx, fval, argc, argv.begin(), true, rval);
 }
 
 static JSObject*
 JS_NewHelper(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, ctor, inputArgs);
 
-    RootedValue ctorVal(cx, ObjectValue(*ctor));
-    if (!IsConstructor(ctorVal)) {
-        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, ctorVal, nullptr);
+    // This is not a simple variation of JS_CallFunctionValue because JSOP_NEW
+    // is not a simple variation of JSOP_CALL. We have to determine what class
+    // of object to create, create it, and clamp the return value to an object,
+    // among other details. InvokeConstructor does the hard work.
+    InvokeArgs args(cx);
+    if (!args.init(inputArgs.length(), true))
+        return nullptr;
+
+    args.setCallee(ObjectValue(*ctor));
+    args.setThis(NullValue());
+    PodCopy(args.array(), inputArgs.begin(), inputArgs.length());
+    args.newTarget().setObject(*ctor);
+
+    if (!InvokeConstructor(cx, args))
+        return nullptr;
+
+    if (!args.rval().isObject()) {
+        /*
+         * Although constructors may return primitives (via proxies), this
+         * API is asking for an object, so we report an error.
+         */
+        JSAutoByteString bytes;
+        if (ValueToPrintable(cx, args.rval(), &bytes)) {
+            JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_BAD_NEW_RESULT,
+                                 bytes.ptr());
+        }
         return nullptr;
     }
 
-    ConstructArgs args(cx);
-    if (!FillArgumentsFromArraylike(cx, args, inputArgs))
-        return nullptr;
-
-    RootedValue rval(cx);
-    if (!js::Construct(cx, ctorVal, args, ctorVal, &rval))
-        return nullptr;
-
-    return &rval.toObject();
+    return &args.rval().toObject();
 }
 
 JS_PUBLIC_API(JSObject*)
 JS_New(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& inputArgs)
 {
     RootedObject obj(cx);
     {
         AutoLastFrameCheck lfc(cx);
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -2983,26 +2983,23 @@ array_of(JSContext* cx, unsigned argc, V
         // IsArrayConstructor(this) will usually be true in practice. This is
         // the most common path.
         return ArrayFromCallArgs(cx, args);
     }
 
     // Step 4.
     RootedObject obj(cx);
     {
-        ConstructArgs cargs(cx);
-        if (!cargs.init(1))
+        RootedValue v(cx);
+        Value argv[1] = {NumberValue(args.length())};
+        if (!InvokeConstructor(cx, args.thisv(), 1, argv, false, &v))
             return false;
-        cargs[0].setNumber(args.length());
-
-        RootedValue v(cx);
-        if (!Construct(cx, args.thisv(), cargs, args.thisv(), &v))
+        obj = ToObject(cx, v);
+        if (!obj)
             return false;
-
-        obj = &v.toObject();
     }
 
     // Step 8.
     for (unsigned k = 0; k < args.length(); k++) {
         if (!DefineElement(cx, obj, k, args[k]))
             return false;
     }
 
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1499,68 +1499,54 @@ JSFunction::maybeRelazify(JSRuntime* rt)
 bool
 js::CallOrConstructBoundFunction(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     RootedFunction fun(cx, &args.callee().as<JSFunction>());
     MOZ_ASSERT(fun->isBoundFunction());
 
     /* 15.3.4.5.1 step 1, 15.3.4.5.2 step 3. */
-    unsigned boundArgsLen = fun->getBoundFunctionArgumentCount();
+    unsigned argslen = fun->getBoundFunctionArgumentCount();
 
-    uint32_t argsLen = args.length();
-    if (argsLen + boundArgsLen > ARGS_LENGTH_MAX) {
+    if (args.length() + argslen > ARGS_LENGTH_MAX) {
         ReportAllocationOverflow(cx);
         return false;
     }
 
     /* 15.3.4.5.1 step 3, 15.3.4.5.2 step 1. */
     RootedObject target(cx, fun->getBoundFunctionTarget());
 
     /* 15.3.4.5.1 step 2. */
     const Value& boundThis = fun->getBoundFunctionThis();
 
-    if (args.isConstructing()) {
-        ConstructArgs cargs(cx);
-        if (!cargs.init(argsLen + boundArgsLen))
-            return false;
-
-        /* 15.3.4.5.1, 15.3.4.5.2 step 4. */
-        for (uint32_t i = 0; i < boundArgsLen; i++)
-            cargs[i].set(fun->getBoundFunctionArgument(i));
-        for (uint32_t i = 0; i < argsLen; i++)
-            cargs[boundArgsLen + i].set(args[i]);
-
-        RootedValue targetv(cx, ObjectValue(*target));
-
-        /* ES6 9.4.1.2 step 5 */
-        RootedValue newTarget(cx);
-        if (&args.newTarget().toObject() == fun)
-            newTarget.set(targetv);
-        else
-            newTarget.set(args.newTarget());
-
-        return Construct(cx, targetv, cargs, newTarget, args.rval());
-    }
-
     InvokeArgs invokeArgs(cx);
-    if (!invokeArgs.init(argsLen + boundArgsLen))
+    if (!invokeArgs.init(args.length() + argslen, args.isConstructing()))
         return false;
 
     /* 15.3.4.5.1, 15.3.4.5.2 step 4. */
-    for (uint32_t i = 0; i < boundArgsLen; i++)
+    for (unsigned i = 0; i < argslen; i++)
         invokeArgs[i].set(fun->getBoundFunctionArgument(i));
-    for (uint32_t i = 0; i < argsLen; i++)
-        invokeArgs[boundArgsLen + i].set(args[i]);
+    PodCopy(invokeArgs.array() + argslen, vp + 2, args.length());
 
     /* 15.3.4.5.1, 15.3.4.5.2 step 5. */
     invokeArgs.setCallee(ObjectValue(*target));
-    invokeArgs.setThis(boundThis);
+
+    bool constructing = args.isConstructing();
+    if (!constructing)
+        invokeArgs.setThis(boundThis);
 
-    if (!Invoke(cx, invokeArgs))
+    /* ES6 9.4.1.2 step 5 */
+    if (constructing) {
+        if (&args.newTarget().toObject() == fun)
+            invokeArgs.newTarget().setObject(*target);
+        else
+            invokeArgs.newTarget().set(args.newTarget());
+    }
+
+    if (constructing ? !InvokeConstructor(cx, invokeArgs) : !Invoke(cx, invokeArgs))
         return false;
 
     args.rval().set(invokeArgs.rval());
     return true;
 }
 
 static bool
 fun_isGenerator(JSContext* cx, unsigned argc, Value* vp)
--- a/js/src/proxy/DirectProxyHandler.cpp
+++ b/js/src/proxy/DirectProxyHandler.cpp
@@ -76,28 +76,18 @@ DirectProxyHandler::call(JSContext* cx, 
     RootedValue target(cx, proxy->as<ProxyObject>().private_());
     return Invoke(cx, args.thisv(), target, args.length(), args.array(), args.rval());
 }
 
 bool
 DirectProxyHandler::construct(JSContext* cx, HandleObject proxy, const CallArgs& args) const
 {
     assertEnteredPolicy(cx, proxy, JSID_VOID, CALL);
-
     RootedValue target(cx, proxy->as<ProxyObject>().private_());
-    if (!IsConstructor(target)) {
-        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, target, nullptr);
-        return false;
-    }
-
-    ConstructArgs cargs(cx);
-    if (!FillArgumentsFromArraylike(cx, cargs, args))
-        return false;
-
-    return Construct(cx, target, cargs, args.newTarget(), args.rval());
+    return InvokeConstructor(cx, target, args.length(), args.array(), true, args.rval());
 }
 
 bool
 DirectProxyHandler::nativeCall(JSContext* cx, IsAcceptableThis test, NativeImpl impl,
                                CallArgs args) const
 {
     args.setThis(ObjectValue(*args.thisv().toObject().as<ProxyObject>().target()));
     if (!test(args.thisv())) {
--- a/js/src/proxy/ScriptedDirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp
@@ -1047,22 +1047,18 @@ ScriptedDirectProxyHandler::construct(JS
 
     // step 4-5
     RootedValue trap(cx);
     if (!GetProperty(cx, handler, handler, cx->names().construct, &trap))
         return false;
 
     // step 6
     if (trap.isUndefined()) {
-        ConstructArgs cargs(cx);
-        if (!FillArgumentsFromArraylike(cx, cargs, args))
-            return false;
-
         RootedValue targetv(cx, ObjectValue(*target));
-        return Construct(cx, targetv, cargs, args.newTarget(), args.rval());
+        return InvokeConstructor(cx, targetv, args.length(), args.array(), true, args.rval());
     }
 
     // step 8-9
     Value constructArgv[] = {
         ObjectValue(*target),
         ObjectValue(*argsArray),
         args.newTarget()
     };
--- a/js/src/proxy/ScriptedIndirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedIndirectProxyHandler.cpp
@@ -458,34 +458,21 @@ CallableScriptedIndirectProxyHandler::ca
     MOZ_ASSERT(call.isObject() && call.toObject().isCallable());
     return Invoke(cx, args.thisv(), call, args.length(), args.array(), args.rval());
 }
 
 bool
 CallableScriptedIndirectProxyHandler::construct(JSContext* cx, HandleObject proxy, const CallArgs& args) const
 {
     assertEnteredPolicy(cx, proxy, JSID_VOID, CALL);
-
     RootedObject ccHolder(cx, &proxy->as<ProxyObject>().extra(0).toObject());
     MOZ_ASSERT(ccHolder->getClass() == &CallConstructHolder);
-
     RootedValue construct(cx, ccHolder->as<NativeObject>().getReservedSlot(1));
-
-    // We could enforce this at proxy creation time, but lipstick on a pig.
-    // Plus, let's delay in-the-field bustage as long as possible.
-    if (!IsConstructor(construct)) {
-        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_IGNORE_STACK, construct, nullptr);
-        return false;
-    }
-
-    ConstructArgs cargs(cx);
-    if (!FillArgumentsFromArraylike(cx, cargs, args))
-        return false;
-
-    return Construct(cx, construct, cargs, args.newTarget(), args.rval());
+    MOZ_ASSERT(construct.isObject() && construct.toObject().isCallable());
+    return InvokeConstructor(cx, construct, args.length(), args.array(), true, args.rval());
 }
 
 const CallableScriptedIndirectProxyHandler CallableScriptedIndirectProxyHandler::singleton;
 
 bool
 js::proxy_create(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
deleted file mode 100644
--- a/js/src/tests/ecma_6/extensions/new-cross-compartment.js
+++ /dev/null
@@ -1,42 +0,0 @@
-// Any copyright is dedicated to the Public Domain.
-// http://creativecommons.org/licenses/publicdomain/
-
-//-----------------------------------------------------------------------------
-var BUGNUMBER = 1178653;
-var summary =
-  "|new| on a cross-compartment wrapper to a non-constructor shouldn't assert";
-
-print(BUGNUMBER + ": " + summary);
-
-/**************
- * BEGIN TEST *
- **************/
-
-var g = newGlobal();
-
-var otherStr = new g.String("foo");
-assertEq(otherStr instanceof g.String, true);
-assertEq(otherStr.valueOf(), "foo");
-
-// THIS IS WRONG.  |new| itself should throw if !IsConstructor(constructor),
-// meaning this global's TypeError should be used.  The problem ultimately is
-// that wrappers conflate callable/constructible, so any old function from
-// another global appears to be both.  Somebody fix bug XXXXXX!
-try
-{
-  var constructor = g.parseInt;
-  new constructor();
-  throw new Error("no error thrown");
-}
-catch (e)
-{
-  assertEq(e instanceof g.TypeError, true,
-           "THIS REALLY SHOULD BE |e instanceof TypeError|");
-}
-
-/******************************************************************************/
-
-if (typeof reportCompare === "function")
-  reportCompare(true, true);
-
-print("Tests complete");
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -827,103 +827,70 @@ js::Invoke(JSContext* cx, const Value& t
 
     if (!Invoke(cx, args))
         return false;
 
     rval.set(args.rval());
     return true;
 }
 
-static bool
-InternalConstruct(JSContext* cx, const CallArgs& args)
-{
-    MOZ_ASSERT(args.array() + args.length() + 1 == args.end(),
-               "must pass constructing arguments to a construction attempt");
+bool
+js::InvokeConstructor(JSContext* cx, CallArgs args)
+{
     MOZ_ASSERT(!JSFunction::class_.construct);
 
-    // Callers are responsible for enforcing these preconditions.
-    MOZ_ASSERT(IsConstructor(args.calleev()),
-               "trying to construct a value that isn't a constructor");
-    MOZ_ASSERT(IsConstructor(args.newTarget()),
-               "provided new.target value must be a constructor");
+    args.setThis(MagicValue(JS_IS_CONSTRUCTING));
+
+    // +2 here and below to pass over |this| and |new.target|
+    if (!args.calleev().isObject())
+        return ReportIsNotFunction(cx, args.calleev(), args.length() + 2, CONSTRUCT);
+
+    MOZ_ASSERT(args.newTarget().isObject());
 
     JSObject& callee = args.callee();
     if (callee.is<JSFunction>()) {
         RootedFunction fun(cx, &callee.as<JSFunction>());
 
+        if (!fun->isConstructor())
+            return ReportIsNotFunction(cx, args.calleev(), args.length() + 2, CONSTRUCT);
+
         if (fun->isNative())
             return CallJSNativeConstructor(cx, fun->native(), args);
 
         if (!Invoke(cx, args, CONSTRUCT))
             return false;
 
         MOZ_ASSERT(args.rval().isObject());
         return true;
     }
 
     JSNative construct = callee.constructHook();
-    MOZ_ASSERT(construct != nullptr, "IsConstructor without a construct hook?");
+    if (!construct)
+        return ReportIsNotFunction(cx, args.calleev(), args.length() + 2, CONSTRUCT);
 
     return CallJSNativeConstructor(cx, construct, args);
 }
 
-// Check that |callee|, the callee in a |new| expression, is a constructor.
-static bool
-StackCheckIsConstructorCalleeNewTarget(JSContext* cx, HandleValue callee, HandleValue newTarget)
-{
-    // Calls from the stack could have any old non-constructor callee.
-    if (!IsConstructor(callee)) {
-        ReportValueError(cx, JSMSG_NOT_CONSTRUCTOR, JSDVG_SEARCH_STACK, callee, nullptr);
-        return false;
-    }
-
-    // The new.target for a stack construction attempt is just the callee: no
-    // need to check that it's a constructor.
-    MOZ_ASSERT(&callee.toObject() == &newTarget.toObject());
-
-    return true;
-}
-
-static bool
-ConstructFromStack(JSContext* cx, const CallArgs& args)
-{
-    if (!StackCheckIsConstructorCalleeNewTarget(cx, args.calleev(), args.newTarget()))
-        return false;
-
-    args.setThis(MagicValue(JS_IS_CONSTRUCTING));
-    return InternalConstruct(cx, args);
-}
-
 bool
-js::Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
-              MutableHandleValue rval)
-{
-    args.setCallee(fval);
-    args.setThis(MagicValue(JS_IS_CONSTRUCTING));
-    args.newTarget().set(newTarget);
-    if (!InternalConstruct(cx, args))
+js::InvokeConstructor(JSContext* cx, Value fval, unsigned argc, const Value* argv,
+                      bool newTargetInArgv, MutableHandleValue rval)
+{
+    InvokeArgs args(cx);
+    if (!args.init(argc, true))
         return false;
 
-    rval.set(args.rval());
-    return true;
-}
-
-bool
-js::InternalConstructWithProvidedThis(JSContext* cx, HandleValue fval, HandleValue thisv,
-                                      const ConstructArgs& args, HandleValue newTarget,
-                                      MutableHandleValue rval)
-{
     args.setCallee(fval);
-
-    MOZ_ASSERT(thisv.isObject());
-    args.setThis(thisv);
-
-    args.newTarget().set(newTarget);
-
-    if (!InternalConstruct(cx, args))
+    args.setThis(MagicValue(JS_THIS_POISON));
+    PodCopy(args.array(), argv, argc);
+    if (newTargetInArgv)
+        args.newTarget().set(argv[argc]);
+    else
+        args.newTarget().set(fval);
+
+    if (!InvokeConstructor(cx, args))
         return false;
 
     rval.set(args.rval());
     return true;
 }
 
 bool
 js::InvokeGetter(JSContext* cx, JSObject* obj, Value fval, MutableHandleValue rval)
@@ -3057,17 +3024,17 @@ CASE(JSOP_FUNCALL)
     JSFunction* maybeFun;
     bool isFunction = IsFunctionObject(args.calleev(), &maybeFun);
 
     /* 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))
+            if (!InvokeConstructor(cx, args))
                 goto error;
         } else {
             if (!Invoke(cx, args))
                 goto error;
         }
         Value* newsp = args.spAfterCall();
         TypeScript::Monitor(cx, script, REGS.pc, newsp[-1]);
         REGS.sp = newsp;
@@ -3971,17 +3938,17 @@ CASE(JSOP_CLASSHERITAGE)
 
     ReservedRooted<Value> objProto(&rootValue1);
     ReservedRooted<JSObject*> funcProto(&rootObject0);
     if (val.isNull()) {
         objProto = NullValue();
         if (!GetBuiltinPrototype(cx, JSProto_Function, &funcProto))
             goto error;
     } else {
-        if (!IsConstructor(val)) {
+        if (!val.isObject() || !val.toObject().isConstructor()) {
             ReportIsNotFunction(cx, val, 0, CONSTRUCT);
             goto error;
         }
 
         funcProto = &val.toObject();
 
         if (!GetProperty(cx, funcProto, funcProto, cx->names().prototype, &objProto))
             goto error;
@@ -4695,63 +4662,54 @@ js::SpreadCallOperation(JSContext* cx, H
     // The object must be an array with dense elements and no holes. Baseline's
     // optimized spread call stubs rely on this.
     MOZ_ASSERT(aobj->getDenseInitializedLength() == length);
     MOZ_ASSERT(!aobj->isIndexed());
     for (uint32_t i = 0; i < length; i++)
         MOZ_ASSERT(!aobj->getDenseElement(i).isMagic());
 #endif
 
-    if (op == JSOP_SPREADNEW) {
-        if (!StackCheckIsConstructorCalleeNewTarget(cx, callee, newTarget))
-            return false;
-
-        ConstructArgs cargs(cx);
-        if (!cargs.init(length))
-            return false;
-
-        if (!GetElements(cx, aobj, length, cargs.array()))
-            return false;
-
-        if (!Construct(cx, callee, cargs, newTarget, res))
+    InvokeArgs args(cx);
+
+    if (!args.init(length, constructing))
+        return false;
+
+    args.setCallee(callee);
+    args.setThis(thisv);
+
+    if (!GetElements(cx, aobj, length, args.array()))
+        return false;
+
+    if (constructing)
+        args.newTarget().set(newTarget);
+
+    switch (op) {
+      case JSOP_SPREADNEW:
+        if (!InvokeConstructor(cx, args))
             return false;
-    } else {
-        InvokeArgs args(cx);
-
-        if (!args.init(length))
+        break;
+      case JSOP_SPREADCALL:
+        if (!Invoke(cx, args))
             return false;
-
-        args.setCallee(callee);
-        args.setThis(thisv);
-
-        if (!GetElements(cx, aobj, length, args.array()))
-            return false;
-
-        switch (op) {
-          case JSOP_SPREADCALL:
+        break;
+      case JSOP_SPREADEVAL:
+      case JSOP_STRICTSPREADEVAL:
+        if (cx->global()->valueIsEval(args.calleev())) {
+            if (!DirectEval(cx, args))
+                return false;
+        } else {
             if (!Invoke(cx, args))
                 return false;
-            break;
-          case JSOP_SPREADEVAL:
-          case JSOP_STRICTSPREADEVAL:
-            if (cx->global()->valueIsEval(args.calleev())) {
-                if (!DirectEval(cx, args))
-                    return false;
-            } else {
-                if (!Invoke(cx, args))
-                    return false;
-            }
-            break;
-          default:
-            MOZ_CRASH("bad spread opcode");
         }
-
-        res.set(args.rval());
+        break;
+      default:
+        MOZ_CRASH("bad spread opcode");
     }
 
+    res.set(args.rval());
     TypeScript::Monitor(cx, script, pc, res);
     return true;
 }
 
 JSObject*
 js::NewObjectOperation(JSContext* cx, HandleScript script, jsbytecode* pc,
                        NewObjectKind newKind /* = GenericObject */)
 {
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -80,36 +80,27 @@ Invoke(JSContext* cx, const Value& thisv
  * getter/setter calls.
  */
 extern bool
 InvokeGetter(JSContext* cx, JSObject* obj, Value fval, MutableHandleValue rval);
 
 extern bool
 InvokeSetter(JSContext* cx, const Value& thisv, Value fval, HandleValue v);
 
-// ES6 7.3.13 Construct(F, argumentsList, newTarget).  All parameters are
-// required, hopefully forcing callers to be careful not to (say) blindly pass
-// callee as |newTarget| when a different value should have been passed.
-//
-// NOTE: As with the ES6 spec operation, it's the caller's responsibility to
-//       ensure |fval| and |newTarget| are both |IsConstructor|.
+/*
+ * InvokeConstructor implement a function call from a constructor context
+ * (e.g. 'new') handling the the creation of the new 'this' object.
+ */
 extern bool
-Construct(JSContext* cx, HandleValue fval, const ConstructArgs& args, HandleValue newTarget,
-          MutableHandleValue rval);
+InvokeConstructor(JSContext* cx, CallArgs args);
 
-// Call Construct(fval, args, newTarget), but use the given |thisv| as |this|
-// during construction of |fval|.
-//
-// This method exists only for very rare cases where a |this| was created
-// caller-side for construction of |fval|: basically only for JITs using
-// |CreateThis|.  If that's not you, use Construct()!
+/* See the fval overload of Invoke. */
 extern bool
-InternalConstructWithProvidedThis(JSContext* cx, HandleValue fval, HandleValue thisv,
-                                  const ConstructArgs& args, HandleValue newTarget,
-                                  MutableHandleValue rval);
+InvokeConstructor(JSContext* cx, Value fval, unsigned argc, const Value* argv,
+                  bool newTargetInArgv, MutableHandleValue rval);
 
 /*
  * Executes a script with the given scopeChain/this. The 'type' indicates
  * whether this is eval code or global code. To support debugging, the
  * evalFrame parameter can point to an arbitrary frame in the context's call
  * stack to simulate executing an eval in that frame.
  */
 extern bool
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1078,72 +1078,34 @@ class InterpreterStack
         return allocator_.sizeOfExcludingThis(mallocSizeOf);
     }
 };
 
 void MarkInterpreterActivations(JSRuntime* rt, JSTracer* trc);
 
 /*****************************************************************************/
 
-namespace detail {
-
-class GenericInvokeArgs : public JS::CallArgs
+class InvokeArgs : public JS::CallArgs
 {
-  protected:
     AutoValueVector v_;
 
-    explicit GenericInvokeArgs(JSContext* cx) : v_(cx) {}
+  public:
+    explicit InvokeArgs(JSContext* cx, bool construct = false) : v_(cx) {}
 
-    bool init(unsigned argc, bool construct) {
+    bool init(unsigned argc, bool construct = false) {
         MOZ_ASSERT(2 + argc + construct > argc);  // no overflow
         if (!v_.resize(2 + argc + construct))
             return false;
-
-        *static_cast<JS::CallArgs*>(this) = CallArgsFromVp(argc, v_.begin());
+        ImplicitCast<CallArgs>(*this) = CallArgsFromVp(argc, v_.begin());
+        // Set the internal flag, since we are not initializing from a made array
         constructing_ = construct;
         return true;
     }
 };
 
-} // namespace detail
-
-class InvokeArgs : public detail::GenericInvokeArgs
-{
-  public:
-    explicit InvokeArgs(JSContext* cx) : detail::GenericInvokeArgs(cx) {}
-
-    bool init(unsigned argc) {
-        return detail::GenericInvokeArgs::init(argc, false);
-    }
-};
-
-class ConstructArgs : public detail::GenericInvokeArgs
-{
-  public:
-    explicit ConstructArgs(JSContext* cx) : detail::GenericInvokeArgs(cx) {}
-
-    bool init(unsigned argc) {
-        return detail::GenericInvokeArgs::init(argc, true);
-    }
-};
-
-template <class Args, class Arraylike>
-inline bool
-FillArgumentsFromArraylike(JSContext* cx, Args& args, const Arraylike& arraylike)
-{
-    uint32_t len = arraylike.length();
-    if (!args.init(len))
-        return false;
-
-    for (uint32_t i = 0; i < len; i++)
-        args[i].set(arraylike[i]);
-
-    return true;
-}
-
 template <>
 struct DefaultHasher<AbstractFramePtr> {
     typedef AbstractFramePtr Lookup;
 
     static js::HashNumber hash(const Lookup& key) {
         return size_t(key.raw());
     }