Bug 1141865 - Part 7: Make new.target work in generator functions. (r=jorendorff, r=jandem)
☠☠ backed out by 05411d0a95ad ☠ ☠
authorEric Faust <efaustbmo@mozilla.com>
Wed, 03 Jun 2015 02:01:15 -0700
changeset 246977 213b09228bdfc38905dcd6dbd5ecea040d3bb675
parent 246976 0f963fbdc9182335ece49a6aa16b846529e623ad
child 246978 ad8d961bafef3aea8a29db045c273cb801a9a786
push id28848
push userryanvm@gmail.com
push dateWed, 03 Jun 2015 20:00:13 +0000
treeherdermozilla-central@0920f2325a6d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, jandem
bugs1141865
milestone41.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 1141865 - Part 7: Make new.target work in generator functions. (r=jorendorff, r=jandem)
js/src/frontend/SharedContext.h
js/src/jit/BaselineCompiler.cpp
js/src/tests/ecma_6/Class/newTargetGenerators.js
js/src/tests/js1_8_5/reflect-parse/newTarget.js
js/src/vm/GeneratorObject.cpp
js/src/vm/GeneratorObject.h
js/src/vm/Interpreter.cpp
js/src/vm/Stack-inl.h
js/src/vm/Stack.cpp
js/src/vm/Stack.h
--- a/js/src/frontend/SharedContext.h
+++ b/js/src/frontend/SharedContext.h
@@ -238,18 +238,18 @@ class SharedContext
 
   protected:
     static bool FunctionAllowsSyntax(JSFunction* func, AllowedSyntax allowed)
     {
         MOZ_ASSERT(!func->isArrow());
 
         switch (allowed) {
           case AllowedSyntax::NewTarget:
-            // For now, disallow new.target inside generators
-            return !func->isGenerator();
+            // Any function supports new.target
+            return true;
           case AllowedSyntax::SuperProperty:
             return func->allowSuperProperty();
           default:;
         }
         MOZ_CRASH("Unknown AllowedSyntax query");
     }
 };
 
@@ -368,21 +368,16 @@ class FunctionBox : public ObjectBox, pu
         return bindings.hasAnyAliasedBindings() ||
                hasExtensibleScope() ||
                needsDeclEnvObject() ||
                needsHomeObject()    ||
                isGenerator();
     }
 
     bool allowSyntax(AllowedSyntax allowed) const {
-        // For now (!) we don't allow new.target in generators, and can't
-        // check that for functions we haven't finished parsing, as they
-        // don't have initialized scripts. Check from our stashed bits instead.
-        if (allowed == AllowedSyntax::NewTarget)
-            return !isGenerator();
         return FunctionAllowsSyntax(function(), allowed);
     }
 };
 
 inline FunctionBox*
 SharedContext::asFunctionBox()
 {
     MOZ_ASSERT(isFunctionBox());
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -3673,16 +3673,29 @@ BaselineCompiler::emit_JSOP_RESUME()
     Register scratch1 = regs.takeAny();
     masm.loadPtr(Address(callee, JSFunction::offsetOfNativeOrScript()), scratch1);
 
     // Load the BaselineScript or call a stub if we don't have one.
     Label interpret;
     masm.loadPtr(Address(scratch1, JSScript::offsetOfBaselineScript()), scratch1);
     masm.branchPtr(Assembler::BelowOrEqual, scratch1, ImmPtr(BASELINE_DISABLED_SCRIPT), &interpret);
 
+    Register constructing = regs.takeAny();
+    ValueOperand newTarget = regs.takeAnyValue();
+    masm.loadValue(Address(genObj, GeneratorObject::offsetOfNewTargetSlot()), newTarget);
+    masm.move32(Imm32(0), constructing);
+    {
+        Label notConstructing;
+        masm.branchTestObject(Assembler::NotEqual, newTarget, &notConstructing);
+        masm.pushValue(newTarget);
+        masm.move32(Imm32(CalleeToken_FunctionConstructing), constructing);
+        masm.bind(&notConstructing);
+    }
+    regs.add(newTarget);
+
     // Push |undefined| for all formals.
     Register scratch2 = regs.takeAny();
     Label loop, loopDone;
     masm.load16ZeroExtend(Address(callee, JSFunction::offsetOfNargs()), scratch2);
     masm.bind(&loop);
     masm.branchTest32(Assembler::Zero, scratch2, scratch2, &loopDone);
     {
         masm.pushValue(UndefinedValue());
@@ -3697,20 +3710,24 @@ BaselineCompiler::emit_JSOP_RESUME()
     // Update BaselineFrame frameSize field and create the frame descriptor.
     masm.computeEffectiveAddress(Address(BaselineFrameReg, BaselineFrame::FramePointerOffset),
                                  scratch2);
     masm.subPtr(BaselineStackReg, scratch2);
     masm.store32(scratch2, Address(BaselineFrameReg, BaselineFrame::reverseOffsetOfFrameSize()));
     masm.makeFrameDescriptor(scratch2, JitFrame_BaselineJS);
 
     masm.Push(Imm32(0)); // actual argc
-    masm.PushCalleeToken(callee, false);
+
+    // Duplicate PushCalleeToken with a variable instead.
+    masm.orPtr(constructing, callee);
+    masm.Push(callee);
     masm.Push(scratch2); // frame descriptor
 
     regs.add(callee);
+    regs.add(constructing);
 
     // Load the return value.
     ValueOperand retVal = regs.takeAnyValue();
     masm.loadValue(frame.addressOfStackValue(frame.peek(-1)), retVal);
 
     // Push a fake return address on the stack. We will resume here when the
     // generator returns.
     Label genStart, returnTarget;
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Class/newTargetGenerators.js
@@ -0,0 +1,24 @@
+function *generatorNewTarget(expected) {
+    assertEq(new.target, expected);
+    assertEq(eval('new.target'), expected);
+    assertEq((() => new.target)(), expected);
+    yield (() => new.target);
+}
+
+const ITERATIONS = 25;
+
+for (let i = 0; i < ITERATIONS; i++)
+    assertEq(generatorNewTarget(undefined).next().value(), undefined);
+
+for (let i = 0; i < ITERATIONS; i++)
+    assertEq(new generatorNewTarget(generatorNewTarget).next().value(),
+             generatorNewTarget);
+
+// also check to make sure it's useful in yield inside generators.
+// Plus, this code is so ugly, how could it not be a test? ;)
+// Thanks to anba for supplying this ludicrous expression.
+assertDeepEq([...new function*(i) { yield i; if(i > 0) yield* new new.target(i-1) }(10)],
+             [ 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 ]);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");
--- a/js/src/tests/js1_8_5/reflect-parse/newTarget.js
+++ b/js/src/tests/js1_8_5/reflect-parse/newTarget.js
@@ -10,18 +10,19 @@ function testNewTarget() {
 
     // invalid in top-level scripts
     assertError("new.target", SyntaxError);
 
     // valid in arrow functions inside functions
     assertInFunctionExpr("()=>new.target", arrowExpr([], newTarget()));
     assertError("(() => new.target))", SyntaxError);
 
-    // invalid (for now!) in generators
-    assertError("function *foo() { new.target; }", SyntaxError);
+    // valid in generators, too!
+    assertStmt("function *foo() { new.target; }", genFunDecl(ident("foo"), [],
+               blockStmt([exprStmt(newTarget())])));
 
     // new.target is a member expression. You should be able to call, invoke, or
     // access properties of it.
     assertInFunctionExpr("new.target.foo", dotExpr(newTarget(), ident("foo")));
     assertInFunctionExpr("new.target[\"foo\"]", memExpr(newTarget(), literal("foo")));
 
     assertInFunctionExpr("new.target()", callExpr(newTarget(), []));
     assertInFunctionExpr("new new.target()", newExpr(newTarget(), []));
--- a/js/src/vm/GeneratorObject.cpp
+++ b/js/src/vm/GeneratorObject.cpp
@@ -44,16 +44,17 @@ GeneratorObject::create(JSContext* cx, A
         obj = NewNativeObjectWithGivenProto(cx, &LegacyGeneratorObject::class_, proto);
     }
     if (!obj)
         return nullptr;
 
     GeneratorObject* genObj = &obj->as<GeneratorObject>();
     genObj->setCallee(*frame.callee());
     genObj->setThisValue(frame.thisValue());
+    genObj->setNewTarget(frame.newTarget());
     genObj->setScopeChain(*frame.scopeChain());
     if (frame.script()->needsArgsObj())
         genObj->setArgsObj(frame.argsObj());
     genObj->clearExpressionStack();
 
     return obj;
 }
 
@@ -157,18 +158,19 @@ bool
 GeneratorObject::resume(JSContext* cx, InterpreterActivation& activation,
                         HandleObject obj, HandleValue arg, GeneratorObject::ResumeKind resumeKind)
 {
     Rooted<GeneratorObject*> genObj(cx, &obj->as<GeneratorObject>());
     MOZ_ASSERT(genObj->isSuspended());
 
     RootedFunction callee(cx, &genObj->callee());
     RootedValue thisv(cx, genObj->thisValue());
+    RootedValue newTarget(cx, genObj->newTarget());
     RootedObject scopeChain(cx, &genObj->scopeChain());
-    if (!activation.resumeGeneratorFrame(callee, thisv, scopeChain))
+    if (!activation.resumeGeneratorFrame(callee, thisv, newTarget, scopeChain))
         return false;
 
     if (genObj->hasArgsObj())
         activation.regs().fp()->initArgsObj(genObj->argsObj());
 
     if (genObj->hasExpressionStack()) {
         uint32_t len = genObj->expressionStack().length();
         MOZ_ASSERT(activation.regs().spForStackDepth(len));
--- a/js/src/vm/GeneratorObject.h
+++ b/js/src/vm/GeneratorObject.h
@@ -26,16 +26,17 @@ class GeneratorObject : public NativeObj
 
     enum {
         CALLEE_SLOT = 0,
         THIS_SLOT,
         SCOPE_CHAIN_SLOT,
         ARGS_OBJ_SLOT,
         EXPRESSION_STACK_SLOT,
         YIELD_INDEX_SLOT,
+        NEWTARGET_SLOT,
         RESERVED_SLOTS
     };
 
     enum ResumeKind { NEXT, THROW, CLOSE };
 
   private:
     static bool suspend(JSContext* cx, HandleObject obj, AbstractFramePtr frame, jsbytecode* pc,
                         Value* vp, unsigned nvalues);
@@ -112,16 +113,27 @@ class GeneratorObject : public NativeObj
     }
     void setExpressionStack(ArrayObject& expressionStack) {
         setFixedSlot(EXPRESSION_STACK_SLOT, ObjectValue(expressionStack));
     }
     void clearExpressionStack() {
         setFixedSlot(EXPRESSION_STACK_SLOT, NullValue());
     }
 
+    bool isConstructing() const {
+        return getFixedSlot(NEWTARGET_SLOT).isObject();
+    }
+    const Value& newTarget() const {
+        return getFixedSlot(NEWTARGET_SLOT);
+    }
+    void setNewTarget(Value newTarget) {
+        setFixedSlot(NEWTARGET_SLOT, newTarget);
+    }
+
+
     // The yield index slot is abused for a few purposes.  It's undefined if
     // it hasn't been set yet (before the initial yield), and null if the
     // generator is closed. If the generator is running, the yield index is
     // YIELD_INDEX_RUNNING. If the generator is in that bizarre "closing"
     // state, the yield index is YIELD_INDEX_CLOSING.
     //
     // If the generator is suspended, it's the yield index (stored as
     // JSOP_INITIALYIELD/JSOP_YIELD operand) of the yield instruction that
@@ -167,16 +179,17 @@ class GeneratorObject : public NativeObj
     }
     void setClosed() {
         setFixedSlot(CALLEE_SLOT, NullValue());
         setFixedSlot(THIS_SLOT, NullValue());
         setFixedSlot(SCOPE_CHAIN_SLOT, NullValue());
         setFixedSlot(ARGS_OBJ_SLOT, NullValue());
         setFixedSlot(EXPRESSION_STACK_SLOT, NullValue());
         setFixedSlot(YIELD_INDEX_SLOT, NullValue());
+        setFixedSlot(NEWTARGET_SLOT, NullValue());
     }
 
     static size_t offsetOfCalleeSlot() {
         return getFixedSlotOffset(CALLEE_SLOT);
     }
     static size_t offsetOfThisSlot() {
         return getFixedSlotOffset(THIS_SLOT);
     }
@@ -187,16 +200,19 @@ class GeneratorObject : public NativeObj
         return getFixedSlotOffset(ARGS_OBJ_SLOT);
     }
     static size_t offsetOfYieldIndexSlot() {
         return getFixedSlotOffset(YIELD_INDEX_SLOT);
     }
     static size_t offsetOfExpressionStackSlot() {
         return getFixedSlotOffset(EXPRESSION_STACK_SLOT);
     }
+    static size_t offsetOfNewTargetSlot() {
+        return getFixedSlotOffset(NEWTARGET_SLOT);
+    }
 };
 
 class LegacyGeneratorObject : public GeneratorObject
 {
   public:
     static const Class class_;
 
     static bool close(JSContext* cx, HandleObject obj);
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -3807,16 +3807,20 @@ CASE(JSOP_YIELD)
 CASE(JSOP_RESUME)
 {
     {
         ReservedRooted<JSObject*> gen(&rootObject0, &REGS.sp[-2].toObject());
         ReservedRooted<Value> val(&rootValue0, REGS.sp[-1]);
         // popInlineFrame expects there to be an additional value on the stack
         // to pop off, so leave "gen" on the stack.
 
+        // Again, lie to popInlineFrame. Add a "new.target" to pop later.
+        if (gen->as<GeneratorObject>().isConstructing())
+            PUSH_UNDEFINED();
+
         GeneratorObject::ResumeKind resumeKind = GeneratorObject::getResumeKind(REGS.pc);
         bool ok = GeneratorObject::resume(cx, activation, gen, val, resumeKind);
         SET_SCRIPT(REGS.fp()->script());
         if (!ok)
             goto error;
     }
     ADVANCE_AND_DISPATCH(0);
 }
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -343,44 +343,49 @@ InterpreterStack::pushInlineFrame(JSCont
 
     regs.prepareToRun(*fp, script);
     return true;
 }
 
 MOZ_ALWAYS_INLINE bool
 InterpreterStack::resumeGeneratorCallFrame(JSContext* cx, InterpreterRegs& regs,
                                            HandleFunction callee, HandleValue thisv,
-                                           HandleObject scopeChain)
+                                           HandleValue newTarget, HandleObject scopeChain)
 {
     MOZ_ASSERT(callee->isGenerator());
     RootedScript script(cx, callee->getOrCreateScript(cx));
     InterpreterFrame* prev = regs.fp();
     jsbytecode* prevpc = regs.pc;
     Value* prevsp = regs.sp;
     MOZ_ASSERT(prev);
 
     script->ensureNonLazyCanonicalFunction(cx);
 
     LifoAlloc::Mark mark = allocator_.mark();
 
-    // Include callee, |this|.
+    bool constructing = newTarget.isObject();
+
+    // Include callee, |this|, and maybe |new.target|
     unsigned nformal = callee->nargs();
-    unsigned nvals = 2 + nformal + script->nslots();
+    unsigned nvals = 2 + constructing + nformal + script->nslots();
 
     uint8_t* buffer = allocateFrame(cx, sizeof(InterpreterFrame) + nvals * sizeof(Value));
     if (!buffer)
         return false;
 
     Value* argv = reinterpret_cast<Value*>(buffer) + 2;
     argv[-2] = ObjectValue(*callee);
     argv[-1] = thisv;
     SetValueRangeToUndefined(argv, nformal);
+    if (constructing)
+        argv[nformal] = newTarget;
 
-    InterpreterFrame* fp = reinterpret_cast<InterpreterFrame*>(argv + nformal);
-    InterpreterFrame::Flags flags = ToFrameFlags(INITIAL_NONE);
+    InterpreterFrame* fp = reinterpret_cast<InterpreterFrame*>(argv + nformal + constructing);
+    InterpreterFrame::Flags flags = constructing ? ToFrameFlags(INITIAL_CONSTRUCT)
+                                                 : ToFrameFlags(INITIAL_NONE);
     fp->mark_ = mark;
     fp->initCallFrame(cx, prev, prevpc, prevsp, *callee, script, argv, 0, flags);
     fp->resumeGeneratorFrame(scopeChain);
 
     regs.prepareToRun(*fp, script);
     return true;
 }
 
@@ -960,20 +965,20 @@ InterpreterActivation::popInlineFrame(In
     MOZ_ASSERT(regs_.fp() == frame);
     MOZ_ASSERT(regs_.fp() != entryFrame_);
 
     cx_->asJSContext()->runtime()->interpreterStack().popInlineFrame(regs_);
 }
 
 inline bool
 InterpreterActivation::resumeGeneratorFrame(HandleFunction callee, HandleValue thisv,
-                                            HandleObject scopeChain)
+                                            HandleValue newTarget, HandleObject scopeChain)
 {
     InterpreterStack& stack = cx_->asJSContext()->runtime()->interpreterStack();
-    if (!stack.resumeGeneratorCallFrame(cx_->asJSContext(), regs_, callee, thisv, scopeChain))
+    if (!stack.resumeGeneratorCallFrame(cx_->asJSContext(), regs_, callee, thisv, newTarget, scopeChain))
         return false;
 
     MOZ_ASSERT(regs_.fp()->script()->compartment() == compartment_);
     return true;
 }
 
 inline JSContext*
 AsmJSActivation::cx()
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -280,17 +280,17 @@ InterpreterFrame::epilogue(JSContext* cx
                       scopeChain()->as<CallObject>().callee().nonLazyScript() == script);
     } else {
         AssertDynamicScopeMatchesStaticScope(cx, script, scopeChain());
     }
 
     if (MOZ_UNLIKELY(cx->compartment()->isDebuggee()))
         DebugScopes::onPopCall(this, cx);
 
-    if (isConstructing() && thisValue().isObject() && returnValue().isPrimitive())
+    if (!fun()->isGenerator() && isConstructing() && thisValue().isObject() && returnValue().isPrimitive())
         setReturnValue(ObjectValue(constructorThis()));
 }
 
 bool
 InterpreterFrame::pushBlock(JSContext* cx, StaticBlockObject& block)
 {
     MOZ_ASSERT(block.needsClone());
 
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1036,17 +1036,17 @@ class InterpreterStack
     // new InterpreterActivation or recursively calling Interpret.
     bool pushInlineFrame(JSContext* cx, InterpreterRegs& regs, const CallArgs& args,
                          HandleScript script, InitialFrameFlags initial);
 
     void popInlineFrame(InterpreterRegs& regs);
 
     bool resumeGeneratorCallFrame(JSContext* cx, InterpreterRegs& regs,
                                   HandleFunction callee, HandleValue thisv,
-                                  HandleObject scopeChain);
+                                  HandleValue newTarget, HandleObject scopeChain);
 
     inline void purge(JSRuntime* rt);
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
         return allocator_.sizeOfExcludingThis(mallocSizeOf);
     }
 };
 
@@ -1244,17 +1244,17 @@ class InterpreterActivation : public Act
     inline InterpreterActivation(RunState& state, JSContext* cx, InterpreterFrame* entryFrame);
     inline ~InterpreterActivation();
 
     inline bool pushInlineFrame(const CallArgs& args, HandleScript script,
                                 InitialFrameFlags initial);
     inline void popInlineFrame(InterpreterFrame* frame);
 
     inline bool resumeGeneratorFrame(HandleFunction callee, HandleValue thisv,
-                                     HandleObject scopeChain);
+                                     HandleValue newTarget, HandleObject scopeChain);
 
     InterpreterFrame* current() const {
         return regs_.fp();
     }
     InterpreterRegs& regs() {
         return regs_;
     }
     InterpreterFrame* entryFrame() const {