Bug 1141865 - Part 7: Make new.target work in generator functions. (r=jorendorff, r=jandem)

js/src/frontend/SharedContext.h

@@ -243,8 +243,8 @@ class SharedContext
 
         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:;
@@ -373,11 +373,6 @@ class FunctionBox : public ObjectBox, public SharedContext
     }
 
     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);
     }
 };

js/src/jit/BaselineCompiler.cpp

@@ -3678,6 +3678,19 @@ BaselineCompiler::emit_JSOP_RESUME()
     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;
@@ -3702,10 +3715,14 @@ BaselineCompiler::emit_JSOP_RESUME()
     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();

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");

js/src/tests/js1_8_5/reflect-parse/newTarget.js

@@ -15,8 +15,9 @@ function testNewTarget() {
     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.

js/src/vm/GeneratorObject.cpp

@@ -49,6 +49,7 @@ GeneratorObject::create(JSContext* cx, AbstractFramePtr frame)
     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());
@@ -162,8 +163,9 @@ GeneratorObject::resume(JSContext* cx, InterpreterActivation& activation,
 
     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())

js/src/vm/GeneratorObject.h

@@ -31,6 +31,7 @@ class GeneratorObject : public NativeObject
         ARGS_OBJ_SLOT,
         EXPRESSION_STACK_SLOT,
         YIELD_INDEX_SLOT,
+        NEWTARGET_SLOT,
         RESERVED_SLOTS
     };
 
@@ -117,6 +118,17 @@ class GeneratorObject : public NativeObject
         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
@@ -172,6 +184,7 @@ class GeneratorObject : public NativeObject
         setFixedSlot(ARGS_OBJ_SLOT, NullValue());
         setFixedSlot(EXPRESSION_STACK_SLOT, NullValue());
         setFixedSlot(YIELD_INDEX_SLOT, NullValue());
+        setFixedSlot(NEWTARGET_SLOT, NullValue());
     }
 
     static size_t offsetOfCalleeSlot() {
@@ -192,6 +205,9 @@ class GeneratorObject : public NativeObject
     static size_t offsetOfExpressionStackSlot() {
         return getFixedSlotOffset(EXPRESSION_STACK_SLOT);
     }
+    static size_t offsetOfNewTargetSlot() {
+        return getFixedSlotOffset(NEWTARGET_SLOT);
+    }
 };
 
 class LegacyGeneratorObject : public GeneratorObject

js/src/vm/Interpreter.cpp

@@ -3812,6 +3812,10 @@ CASE(JSOP_RESUME)
         // 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());

js/src/vm/Stack-inl.h

@@ -348,7 +348,7 @@ InterpreterStack::pushInlineFrame(JSContext* cx, InterpreterRegs& regs, const Ca
 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));
@@ -361,9 +361,11 @@ InterpreterStack::resumeGeneratorCallFrame(JSContext* cx, InterpreterRegs& regs,
 
     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)
@@ -373,9 +375,12 @@ InterpreterStack::resumeGeneratorCallFrame(JSContext* cx, InterpreterRegs& regs,
     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);
@@ -965,10 +970,10 @@ InterpreterActivation::popInlineFrame(InterpreterFrame* frame)
 
 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_);

js/src/vm/Stack.cpp

@@ -285,7 +285,7 @@ InterpreterFrame::epilogue(JSContext* cx)
     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()));
 }
 

js/src/vm/Stack.h

@@ -1041,7 +1041,7 @@ class InterpreterStack
 
     bool resumeGeneratorCallFrame(JSContext* cx, InterpreterRegs& regs,
                                   HandleFunction callee, HandleValue thisv,
-                                  HandleObject scopeChain);
+                                  HandleValue newTarget, HandleObject scopeChain);
 
     inline void purge(JSRuntime* rt);
 
@@ -1249,7 +1249,7 @@ class InterpreterActivation : public Activation
     inline void popInlineFrame(InterpreterFrame* frame);
 
     inline bool resumeGeneratorFrame(HandleFunction callee, HandleValue thisv,
-                                     HandleObject scopeChain);
+                                     HandleValue newTarget, HandleObject scopeChain);
 
     InterpreterFrame* current() const {
         return regs_.fp();