Bug 1230710 - Reenable direct eval and arrow functions in derived class constructors. (r=jorendorff, r=shu)

js/src/builtin/Eval.cpp

@@ -238,12 +238,6 @@ EvalKernel(JSContext* cx, const CallArgs& args, EvalType evalType, AbstractFrame
         return false;
     }
 
-    if (evalType == DIRECT_EVAL && caller.script()->isDerivedClassConstructor()) {
-        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_DISABLED_DERIVED_CLASS,
-                             "direct eval");
-        return false;
-    }
-
     // ES5 15.1.2.1 step 1.
     if (args.length() < 1) {
         args.rval().setUndefined();

js/src/frontend/BytecodeEmitter.cpp

@@ -3475,10 +3475,34 @@ BytecodeEmitter::emitSetThis(ParseNode* pn)
 
     JSOp setOp = name->getOp();
 
+    // Handle the eval case. Only accept the strict variant, as eval in a
+    // derived class constructor must be strict.
+    if (setOp == JSOP_STRICTSETNAME) {
+        if (!emitAtomOp(name, JSOP_GETNAME))
+            return false;
+        if (!emit1(JSOP_CHECKTHISREINIT))
+            return false;
+        if (!emit1(JSOP_POP))
+            return false;
+
+        if (!emitAtomOp(name, JSOP_BINDNAME))
+            return false;
+        if (!emit1(JSOP_SWAP))
+            return false;
+
+        return emitAtomOp(name, setOp);
+    }
+
     JSOp getOp;
     switch (setOp) {
-      case JSOP_SETLOCAL:      getOp = JSOP_GETLOCAL;      break;
+      case JSOP_SETLOCAL:
-      case JSOP_SETALIASEDVAR: getOp = JSOP_GETALIASEDVAR; break;
+        getOp = JSOP_GETLOCAL;
+        setOp = JSOP_INITLEXICAL;
+        break;
+      case JSOP_SETALIASEDVAR:
+        getOp = JSOP_GETALIASEDVAR;
+        setOp = JSOP_INITALIASEDLEXICAL;
+        break;
       default: MOZ_CRASH("Unexpected op");
     }
 

js/src/frontend/Parser.cpp

@@ -125,8 +125,12 @@ SharedContext::computeAllowSyntax(JSObject* staticScope)
             // Any function supports new.target.
             allowNewTarget_ = true;
             allowSuperProperty_ = it.fun().allowSuperProperty();
-            if (it.maybeFunctionBox())
+            if (it.maybeFunctionBox()) {
                 superScopeAlreadyNeedsHomeObject_ = it.maybeFunctionBox()->needsHomeObject();
+                allowSuperCall_ = it.maybeFunctionBox()->isDerivedClassConstructor();
+            } else {
+                allowSuperCall_ = it.fun().isDerivedClassConstructor();
+            }
             break;
         }
     }
@@ -7470,11 +7474,6 @@ Parser<ParseHandler>::assignExpr(InHandling inHandling, YieldHandling yieldHandl
         if (!tokenStream.peekToken(&ignored, TokenStream::Operand))
             return null();
 
-        if (pc->sc->isFunctionBox() && pc->sc->asFunctionBox()->isDerivedClassConstructor()) {
-            report(ParseError, false, null(), JSMSG_DISABLED_DERIVED_CLASS, "arrow functions");
-            return null();
-        }
-
         Node arrowFunc = functionDef(inHandling, yieldHandling, nullptr, Arrow, NotGenerator);
         if (!arrowFunc)
             return null();
@@ -8890,7 +8889,7 @@ Parser<ParseHandler>::memberExpr(YieldHandling yieldHandling, TripledotHandling
                    tt == TOK_NO_SUBS_TEMPLATE)
         {
             if (handler.isSuperBase(lhs)) {
-                if (!pc->sc->isFunctionBox() || !pc->sc->asFunctionBox()->isDerivedClassConstructor()) {
+                if (!pc->sc->allowSuperCall()) {
                     report(ParseError, false, null(), JSMSG_BAD_SUPERCALL);
                     return null();
                 }

js/src/frontend/SharedContext.h

@@ -202,6 +202,7 @@ class SharedContext
 
     bool allowNewTarget_;
     bool allowSuperProperty_;
+    bool allowSuperCall_;
     bool inWith_;
     bool needsThisTDZChecks_;
     bool superScopeAlreadyNeedsHomeObject_;
@@ -217,6 +218,7 @@ class SharedContext
         thisBinding_(ThisBinding::Global),
         allowNewTarget_(false),
         allowSuperProperty_(false),
+        allowSuperCall_(false),
         inWith_(false),
         needsThisTDZChecks_(false),
         superScopeAlreadyNeedsHomeObject_(false)
@@ -244,6 +246,7 @@ class SharedContext
 
     bool allowNewTarget()              const { return allowNewTarget_; }
     bool allowSuperProperty()          const { return allowSuperProperty_; }
+    bool allowSuperCall()              const { return allowSuperCall_; }
     bool inWith()                      const { return inWith_; }
     bool needsThisTDZChecks()          const { return needsThisTDZChecks_; }
 

js/src/js.msg

@@ -105,7 +105,6 @@ MSG_DEF(JSMSG_INVALID_ARG_TYPE,        3, JSEXN_TYPEERR, "Invalid type: {0} can'
 MSG_DEF(JSMSG_TERMINATED,              1, JSEXN_ERR, "Script terminated by timeout at:\n{0}")
 MSG_DEF(JSMSG_PROTO_NOT_OBJORNULL,     1, JSEXN_TYPEERR, "{0}.prototype is not an object or null")
 MSG_DEF(JSMSG_CANT_CALL_CLASS_CONSTRUCTOR, 0, JSEXN_TYPEERR, "class constructors must be invoked with |new|")
-MSG_DEF(JSMSG_DISABLED_DERIVED_CLASS,  1, JSEXN_INTERNALERR, "{0} temporarily disallowed in derived class constructors")
 MSG_DEF(JSMSG_UNINITIALIZED_THIS,      1, JSEXN_REFERENCEERR, "|this| used uninitialized in {0} class constructor")
 MSG_DEF(JSMSG_BAD_DERIVED_RETURN,      1, JSEXN_TYPEERR, "derived class constructor returned invalid value {0}")
 

js/src/jsobj.cpp

@@ -2231,11 +2231,12 @@ js::LookupNameUnqualified(JSContext* cx, HandlePropertyName name, HandleObject s
 
     // See note above RuntimeLexicalErrorObject.
     if (pobj == scope) {
-        if (IsUninitializedLexicalSlot(scope, shape)) {
+        if (name != cx->names().dotThis && IsUninitializedLexicalSlot(scope, shape)) {
             scope = RuntimeLexicalErrorObject::create(cx, scope, JSMSG_UNINITIALIZED_LEXICAL);
             if (!scope)
                 return false;
         } else if (scope->is<ScopeObject>() && !scope->is<DeclEnvObject>() && !shape->writable()) {
+            MOZ_ASSERT(name != cx->names().dotThis);
             scope = RuntimeLexicalErrorObject::create(cx, scope, JSMSG_BAD_CONST_ASSIGN);
             if (!scope)
                 return false;

js/src/tests/ecma_6/Class/derivedConstructorArrowEvalBinding.js

@@ -0,0 +1,19 @@
+// Make sure it doesn't matter when we make the arrow function
+var test = `
+
+new class extends class { } {
+    constructor() {
+        let arrow = () => this;
+        assertThrowsInstanceOf(arrow, ReferenceError);
+        super();
+        assertEq(arrow(), this);
+    }
+}();
+
+`;
+
+if (classesEnabled())
+    eval(test);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/tests/ecma_6/Class/derivedConstructorArrowEvalClosed.js

@@ -0,0 +1,18 @@
+var test = `
+
+new class extends class { } {
+    constructor() {
+        let a1 = () => this;
+        let a2 = (() => super());
+        assertThrowsInstanceOf(a1, ReferenceError);
+        assertEq(a2(), a1());
+    }
+}();
+
+`;
+
+if (classesEnabled())
+    eval(test);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/tests/ecma_6/Class/derivedConstructorArrowEvalEscape.js

@@ -0,0 +1,20 @@
+var test = `
+
+let arrow;
+
+class foo extends class { } {
+    constructor() {
+        arrow = () => this;
+        super();
+    }
+}
+
+assertEq(new foo(), arrow());
+
+`;
+
+if (classesEnabled())
+    eval(test);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/tests/ecma_6/Class/derivedConstructorArrowEvalEscapeUninitialized.js

@@ -0,0 +1,45 @@
+var test = `
+
+let superArrow;
+let thisArrow;
+
+let thisStash;
+
+class base {
+    constructor() {
+        // We run this constructor twice as part of the double init check
+        if (!thisStash)
+            thisStash = {prop:45};
+        return thisStash;
+    }
+}
+
+class foo extends base {
+    constructor() {
+        superArrow = (()=>super());
+        thisArrow = ()=>this;
+    }
+}
+
+// Populate the arrow function saves. Since we never invoke super(), we throw
+assertThrowsInstanceOf(()=>new foo(), ReferenceError);
+
+// No |this| binding in the closure, yet
+assertThrowsInstanceOf(thisArrow, ReferenceError);
+
+// call super()
+superArrow();
+
+// Can't call it twice
+assertThrowsInstanceOf(superArrow, ReferenceError);
+
+// Oh look, |this| is populated, now.
+assertEq(thisArrow(), thisStash);
+
+`;
+
+if (classesEnabled())
+    eval(test);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/tests/ecma_6/Class/derivedConstructorArrowEvalGetThis.js

@@ -0,0 +1,17 @@
+var test = `
+
+new class extends class { } {
+    constructor() {
+        super();
+        assertEq(this, (()=>this)());
+        assertEq(this, eval("this"));
+    }
+}();
+
+`;
+
+if (classesEnabled())
+    eval(test);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/tests/ecma_6/Class/derivedConstructorArrowEvalNestedSuperCall.js

@@ -0,0 +1,41 @@
+var test = `
+
+new class extends class { } {
+    constructor() {
+        (()=>eval("super()"))();
+        assertEq(this, eval("this"));
+        assertEq(this, (()=>this)());
+    }
+}();
+
+new class extends class { } {
+    constructor() {
+        (()=>(()=>super())())();
+        assertEq(this, eval("this"));
+        assertEq(this, (()=>this)());
+    }
+}();
+
+new class extends class { } {
+    constructor() {
+        eval("(()=>super())()");
+        assertEq(this, eval("this"));
+        assertEq(this, (()=>this)());
+    }
+}();
+
+new class extends class { } {
+    constructor() {
+        eval("eval('super()')");
+        assertEq(this, eval("this"));
+        assertEq(this, (()=>this)());
+    }
+}();
+
+`;
+
+if (classesEnabled())
+    eval(test);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/tests/ecma_6/Class/derivedConstructorArrowEvalSuperCall.js

@@ -0,0 +1,25 @@
+var test = `
+
+new class extends class { } {
+    constructor() {
+        assertEq(eval("super(); this"), this);
+        assertEq(this, eval("this"));
+        assertEq(this, (()=>this)());
+    }
+}();
+
+new class extends class { } {
+    constructor() {
+        (()=>super())();
+        assertEq(this, eval("this"));
+        assertEq(this, (()=>this)());
+    }
+}();
+
+`;
+
+if (classesEnabled())
+    eval(test);
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/tests/ecma_6/Class/derivedConstructorDisabled.js

@@ -1,38 +0,0 @@
-// |reftest| skip-if(!xulRuntime.shell)
-
-var test = `
-
-class base {
-    constructor() {
-        eval('');
-        (()=>0)();
-    }
-}
-
-class derived extends base {
-    constructor() {
-        eval('');
-    }
-}
-
-// Make sure eval and arrows are still valid in non-derived constructors.
-new base();
-
-
-// Eval throws in derived class constructors, in both class expressions and
-// statements.
-assertThrowsInstanceOf((() => new derived()), InternalError);
-assertThrowsInstanceOf((() => new class extends base { constructor() { eval('') } }()), InternalError);
-
-var g = newGlobal();
-var dbg = Debugger(g);
-dbg.onDebuggerStatement = function(frame) { assertThrowsInstanceOf(()=>frame.eval(''), InternalError); };
-
-g.eval("new class foo extends null { constructor() { debugger; return {}; } }()");
-`;
-
-if (classesEnabled())
-    eval(test);
-
-if (typeof reportCompare === 'function')
-    reportCompare(0,0,"OK");

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

@@ -132,9 +132,6 @@ function testClasses() {
     assertClass("class NAME { }", []);
     assertClass("class NAME extends null { }", [], lit(null));
 
-    // For now, disallow arrow functions in derived class constructors
-    assertClassError("class NAME extends null { constructor() { (() => 0); }", InternalError);
-
     // Derived class constructor must have curly brackets
     assertClassError("class NAME extends null {  constructor() 1 }", SyntaxError);
 

js/src/vm/Debugger.cpp

@@ -6724,13 +6724,6 @@ DebuggerGenericEval(JSContext* cx, const char* fullMethodName, const Value& code
     MOZ_ASSERT_IF(iter, !scope);
     MOZ_ASSERT_IF(!iter, scope && IsGlobalLexicalScope(scope));
 
-    if (iter && iter->script()->isDerivedClassConstructor()) {
-        MOZ_ASSERT(iter->isFunctionFrame() && iter->calleeTemplate()->isClassConstructor());
-        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_DISABLED_DERIVED_CLASS,
-                             "debugger eval");
-        return false;
-    }
-
     /* Check the first argument, the eval code string. */
     if (!code.isString()) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NOT_EXPECTED_TYPE,

js/src/vm/Interpreter-inl.h

@@ -204,6 +204,10 @@ FetchName(JSContext* cx, HandleObject obj, HandleObject obj2, HandlePropertyName
         }
     }
 
+    // We do our own explicit checking for |this|
+    if (name == cx->names().dotThis)
+        return true;
+
     // NAME operations are the slow paths already, so unconditionally check
     // for uninitialized lets.
     return CheckUninitializedLexical(cx, name, vp);

js/src/vm/Interpreter.cpp

@@ -3230,10 +3230,7 @@ CASE(JSOP_SETLOCAL)
 {
     uint32_t i = GET_LOCALNO(REGS.pc);
 
-    // Derived class constructors store the TDZ Value in the .this slot
+    MOZ_ASSERT(!IsUninitializedLexical(REGS.fp()->unaliasedLocal(i)));
-    // before a super() call.
-    MOZ_ASSERT_IF(!script->isDerivedClassConstructor(),
-                  !IsUninitializedLexical(REGS.fp()->unaliasedLocal(i)));
 
     REGS.fp()->unaliasedLocal(i) = REGS.sp[-1];
 }
@@ -4843,9 +4840,6 @@ js::ThrowUninitializedThis(JSContext* cx, AbstractFramePtr frame)
 {
     RootedFunction fun(cx, frame.callee());
 
-    MOZ_ASSERT(fun->isClassConstructor());
-    MOZ_ASSERT(fun->nonLazyScript()->isDerivedClassConstructor());
-
     const char* name = "anonymous";
     JSAutoByteString str;
     if (fun->atom()) {

js/src/vm/ScopeObject.h

@@ -953,7 +953,7 @@ class ClonedBlockObject : public BlockObject
 // Attempting to access anything on this scope throws the appropriate
 // ReferenceError.
 //
-// ES6 'const' bindings induce a runtime assignment when assigned to outside
+// ES6 'const' bindings induce a runtime error when assigned to outside
 // of initialization, regardless of strictness.
 class RuntimeLexicalErrorObject : public ScopeObject
 {

js/src/vm/Xdr.h

@@ -29,11 +29,11 @@ namespace js {
  *
  *  https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/Bytecode
  */
-static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 328;
+static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 329;
 static const uint32_t XDR_BYTECODE_VERSION =
     uint32_t(0xb973c0de - XDR_BYTECODE_VERSION_SUBTRAHEND);
 
-static_assert(JSErr_Limit == 425,
+static_assert(JSErr_Limit == 424,
               "GREETINGS, POTENTIAL SUBTRAHEND INCREMENTER! If you added or "
               "removed MSG_DEFs from js.msg, you should increment "
               "XDR_BYTECODE_VERSION_SUBTRAHEND and update this assertion's "