[MERGE #2327 @suwc] Issue#829: class extends null should bind this

Suwei Chen · Suwei Chen · commit 341a8c96ac5b · 2017-01-06T15:22:30.000-08:00
Merge pull request #2327 from suwc:build/suwc/buddy A class that extends null should have 'this' binding in its constructor same way as a base class. Add new function attribute 'BaseConstructorKind' to capture 'extends null' cases during class definition evaluation. Add new opcode for branching on 'extends null' scenario for 'this' binding. Fixes #829
diff --git a/lib/Backend/FlowGraph.cpp b/lib/Backend/FlowGraph.cpp
@@ -323,6 +323,7 @@ FlowGraph::RunPeeps()
         case Js::OpCode::BrFncCachedScopeNeq:
         case Js::OpCode::BrOnObject_A:
         case Js::OpCode::BrOnClassConstructor:
+        case Js::OpCode::BrOnBaseConstructorKind:
             if (tryUnsignedCmpPeep)
             {
                 this->UnsignedCmpPeep(instr);
diff --git a/lib/Backend/JnHelperMethodList.h b/lib/Backend/JnHelperMethodList.h
@@ -44,6 +44,7 @@ HELPERCALL(ScrObj_OP_IsInst, Js::JavascriptOperators::OP_IsInst, AttrCanThrow)
 HELPERCALL(Op_IsIn, Js::JavascriptOperators::IsIn, AttrCanThrow)
 HELPERCALL(Op_IsObject, Js::JavascriptOperators::IsObject, AttrCanThrow)
 HELPERCALL(Op_IsClassConstructor, Js::JavascriptOperators::IsClassConstructor, AttrCanThrow)
+HELPERCALL(Op_IsBaseConstructorKind, Js::JavascriptOperators::IsBaseConstructorKind, AttrCanThrow)
 HELPERCALL(Op_LoadHeapArguments, Js::JavascriptOperators::LoadHeapArguments, 0)
 HELPERCALL(Op_LoadHeapArgsCached, Js::JavascriptOperators::LoadHeapArgsCached, 0)
 HELPERCALL(OP_InitCachedScope, Js::JavascriptOperators::OP_InitCachedScope, 0)
diff --git a/lib/Backend/Lower.cpp b/lib/Backend/Lower.cpp
@@ -2154,6 +2154,10 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
             }
             break;
 
+        case Js::OpCode::BrOnBaseConstructorKind:
+            this->LowerBrOnClassConstructor(instr, IR::HelperOp_IsBaseConstructorKind);
+            break;
+
         case Js::OpCode::BrOnClassConstructor:
             this->LowerBrOnClassConstructor(instr, IR::HelperOp_IsClassConstructor);
             break;
diff --git a/lib/Backend/LowerMDShared.cpp b/lib/Backend/LowerMDShared.cpp
@@ -954,6 +954,7 @@ LowererMD::LowerCondBranch(IR::Instr * instr)
     case Js::OpCode::BrNotNull_A:
     case Js::OpCode::BrOnObject_A:
     case Js::OpCode::BrOnClassConstructor:
+    case Js::OpCode::BrOnBaseConstructorKind:
         Assert(!opndSrc1->IsFloat64());
         AssertMsg(instr->GetSrc2() == nullptr, "Expected 1 src on boolean branch");
         instrPrev = IR::Instr::New(Js::OpCode::TEST, this->m_func);
diff --git a/lib/Backend/arm/LowerMD.cpp b/lib/Backend/arm/LowerMD.cpp
@@ -2710,6 +2710,7 @@ LowererMD::LowerCondBranch(IR::Instr * instr)
         case Js::OpCode::BrNotNull_A:
         case Js::OpCode::BrOnObject_A:
         case Js::OpCode::BrOnClassConstructor:
+        case Js::OpCode::BrOnBaseConstructorKind:
             Assert(!opndSrc1->IsFloat64());
             AssertMsg(opndSrc1->IsRegOpnd(),"NYI for other operands");
             AssertMsg(instr->GetSrc2() == nullptr, "Expected 1 src on boolean branch");
diff --git a/lib/Runtime/Base/FunctionInfo.h b/lib/Runtime/Base/FunctionInfo.h
@@ -39,7 +39,8 @@ namespace Js
             Module                         = 0x20000, // The function is the function body wrapper for a module
             EnclosedByGlobalFunc           = 0x40000,
             CanDefer                       = 0x80000,
-            AllowDirectSuper               = 0x100000
+            AllowDirectSuper               = 0x100000,
+            BaseConstructorKind            = 0x200000
         };
         FunctionInfo(JavascriptMethod entryPoint, Attributes attributes = None, LocalFunctionId functionId = Js::Constants::NoFunctionId, FunctionProxy* functionBodyImpl = nullptr);
 
@@ -133,6 +134,8 @@ namespace Js
         bool GetEnclosedByGlobalFunc() const { return (attributes & Attributes::EnclosedByGlobalFunc) != 0; }
         void SetAllowDirectSuper() { attributes = (Attributes)(attributes | Attributes::AllowDirectSuper); }
         bool GetAllowDirectSuper() const { return (attributes & Attributes::AllowDirectSuper) != 0; }
+        void SetBaseConstructorKind() { attributes = (Attributes)(attributes | Attributes::BaseConstructorKind); }
+        bool GetBaseConstructorKind() const { return (attributes & Attributes::BaseConstructorKind) != 0; }
 
     protected:
         JavascriptMethod originalEntryPoint;
diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp
@@ -2113,16 +2113,31 @@ void ByteCodeGenerator::LoadThisObject(FuncInfo *funcInfo, bool thisLoadedFromPa
 {
     if (this->scriptContext->GetConfig()->IsES6ClassAndExtendsEnabled() && funcInfo->IsClassConstructor())
     {
-        // Derived class constructors initialize 'this' to be Undecl
+        // Derived class constructors initialize 'this' to be Undecl except "extends null" cases
         //   - we'll check this value during a super call and during 'this' access
-        // Base class constructors initialize 'this' to a new object using new.target
+        //
+        // Base class constructors or "extends null" cases initialize 'this' to a new object using new.target
         if (funcInfo->IsBaseClassConstructor())
         {
             EmitBaseClassConstructorThisObject(funcInfo);
         }
         else
         {
-            this->m_writer.Reg1(Js::OpCode::InitUndecl, funcInfo->thisPointerRegister);
+            Js::ByteCodeLabel thisLabel = this->Writer()->DefineLabel();
+            Js::ByteCodeLabel skipLabel = this->Writer()->DefineLabel();
+
+            Js::RegSlot tmpReg = funcInfo->AcquireTmpRegister();
+            this->Writer()->Reg1(Js::OpCode::LdFuncObj, tmpReg);
+            this->Writer()->BrReg1(Js::OpCode::BrOnBaseConstructorKind, thisLabel, tmpReg);  // branch when [[ConstructorKind]]=="base"
+            funcInfo->ReleaseTmpRegister(tmpReg);
+
+            this->m_writer.Reg1(Js::OpCode::InitUndecl, funcInfo->thisPointerRegister);  // not "extends null" case
+            this->Writer()->Br(Js::OpCode::Br, skipLabel);
+
+            this->Writer()->MarkLabel(thisLabel);
+            EmitBaseClassConstructorThisObject(funcInfo);  // "extends null" case
+
+            this->Writer()->MarkLabel(skipLabel);
         }
     }
     else if (!funcInfo->IsGlobalFunction() || (this->flags & fscrEval))
@@ -2411,12 +2426,6 @@ void ByteCodeGenerator::EmitClassConstructorEndCode(FuncInfo *funcInfo)
         EmitScopeSlotLoadThis(funcInfo, funcInfo->thisPointerRegister);
         this->Writer()->Reg2(Js::OpCode::Ld_A, ByteCodeGenerator::ReturnRegister, funcInfo->thisPointerRegister);
     }
-    else
-    {
-        // This is the case where we don't have any references to this or super in the constructor or any nested lambda functions.
-        // We know 'this' must be undecl so let's just emit the ReferenceError as part of the fallthrough.
-        EmitUseBeforeDeclarationRuntimeError(this, ByteCodeGenerator::ReturnRegister);
-    }
 }
 
 void ByteCodeGenerator::EmitBaseClassConstructorThisObject(FuncInfo *funcInfo)
diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp
@@ -2973,10 +2973,10 @@ FuncInfo* PostVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerat
             top->AssignUndefinedConstRegister();
 
             top->AssignNewTargetRegister();
+            top->AssignThisRegister();
 
             if (top->GetCallsEval() || top->GetChildCallsEval())
             {
-                top->AssignThisRegister();
                 top->SetIsThisLexicallyCaptured();
                 top->SetIsNewTargetLexicallyCaptured();
                 top->SetIsSuperLexicallyCaptured();
diff --git a/lib/Runtime/ByteCode/OpCodes.h b/lib/Runtime/ByteCode/OpCodes.h
@@ -318,6 +318,7 @@ MACRO_EXTEND_WMS(       InitClassMemberGet,         ElementC,       OpSideEffect
 MACRO_EXTEND_WMS(       InitClassMemberSetComputedName,ElementI,    OpSideEffect|OpOpndHasImplicitCall|OpPostOpDbgBailOut)                  // Class member in set syntax with computed property name
 MACRO_EXTEND_WMS(       InitClassMemberGetComputedName,ElementI,    OpSideEffect|OpOpndHasImplicitCall|OpPostOpDbgBailOut)                  // Class member in get syntax with computed property name
 MACRO_EXTEND_WMS(       BrOnClassConstructor,       BrReg1,         None)               // Branch if argument is a class constructor
+MACRO_EXTEND_WMS(       BrOnBaseConstructorKind,    BrReg1,         None)               // Branch if argument's [[ConstructorKind]] is 'base'
 
 MACRO_BACKEND_ONLY(     ArgIn_A,                    Empty,          None)       // Copy from "in slot" to "local slot", unchecked
 MACRO_WMS(              ArgIn0,                     Reg1,           OpByteCodeOnly)     // Copy from "in slot" to "local slot", unchecked
diff --git a/lib/Runtime/Language/InterpreterHandler.inl b/lib/Runtime/Language/InterpreterHandler.inl
@@ -219,6 +219,7 @@ EXDEF3_WMS(CUSTOM,                  InitClassMemberGetComputedName, OP_InitClass
 EXDEF3_WMS(CUSTOM,                  InitClassMemberGet,         OP_InitClassMemberGet, ElementC)
 EXDEF3_WMS(CUSTOM,                  InitClassMemberSetComputedName, OP_InitClassMemberSetComputedName, ElementI)
 EXDEF2_WMS(BRB,                     BrOnClassConstructor,       OP_BrOnClassConstructor)
+EXDEF2_WMS(BRB,                     BrOnBaseConstructorKind,    OP_BrOnBaseConstructorKind)
   DEF3_WMS(GET_ELEM_LOCALSLOTNonVar,LdLocalSlot,                OP_LdSlot, ElementSlotI1)
 EXDEF3_WMS(GET_ELEM_PARAMSLOTNonVar,LdParamSlot,                OP_LdSlot, ElementSlotI1)
   DEF3_WMS(GET_ELEM_INNERSLOTNonVar,LdInnerSlot,                OP_LdInnerSlot, ElementSlotI2)
diff --git a/lib/Runtime/Language/InterpreterStackFrame.cpp b/lib/Runtime/Language/InterpreterStackFrame.cpp
@@ -8216,6 +8216,11 @@ const byte * InterpreterStackFrame::OP_ProfiledLoopBodyStart(const byte * ip)
         return JavascriptOperators::IsClassConstructor(aValue);
     }
 
+    BOOL InterpreterStackFrame::OP_BrOnBaseConstructorKind(Var aValue)
+    {
+        return JavascriptOperators::IsBaseConstructorKind(aValue);
+    }
+
     template<class T>
     void InterpreterStackFrame::OP_LdLen(const unaligned T * const playout)
     {
diff --git a/lib/Runtime/Language/InterpreterStackFrame.h b/lib/Runtime/Language/InterpreterStackFrame.h
@@ -419,6 +419,7 @@ namespace Js
         BOOL OP_BrOnNoProperty(Var argInstance, uint propertyIdIndex, ScriptContext* scriptContext);
         BOOL OP_BrOnNoEnvProperty(Var envInstance, int32 slotIndex, uint propertyIdIndex, ScriptContext* scriptContext);
         BOOL OP_BrOnClassConstructor(Var aValue);
+        BOOL OP_BrOnBaseConstructorKind(Var aValue);
 
         RecyclableObject * OP_CallGetFunc(Var target);
 
diff --git a/lib/Runtime/Language/JavascriptOperators.cpp b/lib/Runtime/Language/JavascriptOperators.cpp
@@ -6651,6 +6651,11 @@ namespace Js
         return JavascriptFunction::Is(instance) && (JavascriptFunction::FromVar(instance)->GetFunctionInfo()->IsClassConstructor() || !JavascriptFunction::FromVar(instance)->IsScriptFunction());
     }
 
+    BOOL JavascriptOperators::IsBaseConstructorKind(Var instance)
+    {
+        return JavascriptFunction::Is(instance) && (JavascriptFunction::FromVar(instance)->GetFunctionInfo()->GetBaseConstructorKind());
+    }
+
     void JavascriptOperators::OP_InitGetter(Var object, PropertyId propertyId, Var getter)
     {
         AssertMsg(!TaggedNumber::Is(object), "GetMember on a non-object?");
@@ -7042,6 +7047,12 @@ namespace Js
 
                     ctorProtoObj->EnsureProperty(Js::PropertyIds::constructor);
                     ctorProtoObj->SetEnumerable(Js::PropertyIds::constructor, FALSE);
+
+                    if (ScriptFunctionBase::Is(constructor))
+                    {
+                        ScriptFunctionBase::FromVar(constructor)->GetFunctionInfo()->SetBaseConstructorKind();
+                    }
+
                     break;
                 }
 
diff --git a/lib/Runtime/Language/JavascriptOperators.h b/lib/Runtime/Language/JavascriptOperators.h
@@ -220,6 +220,7 @@ namespace Js
         static BOOL IsUndefinedObject(Var instance, JavascriptLibrary* library);
         static BOOL IsAnyNumberValue(Var instance);
         static BOOL IsClassConstructor(Var instance);
+        static BOOL IsBaseConstructorKind(Var instance);
 
         static bool CanShortcutOnUnknownPropertyName(RecyclableObject * instance);
         static bool CanShortcutInstanceOnUnknownPropertyName(RecyclableObject *instance);
diff --git a/lib/Runtime/Library/InJavascript/Intl.js.bc.32b.h b/lib/Runtime/Library/InJavascript/Intl.js.bc.32b.h
diff --git a/lib/Runtime/Library/InJavascript/Intl.js.bc.64b.h b/lib/Runtime/Library/InJavascript/Intl.js.bc.64b.h
diff --git a/lib/Runtime/Library/InJavascript/Intl.js.nojit.bc.32b.h b/lib/Runtime/Library/InJavascript/Intl.js.nojit.bc.32b.h
diff --git a/lib/Runtime/Library/InJavascript/Intl.js.nojit.bc.64b.h b/lib/Runtime/Library/InJavascript/Intl.js.nojit.bc.64b.h
diff --git a/test/es6/ES6Class_BaseClassConstruction.js b/test/es6/ES6Class_BaseClassConstruction.js

Original file line number	Diff line number	Diff line change
`@@ -323,6 +323,7 @@ FlowGraph::RunPeeps()`
`323`	`323`	`case Js::OpCode::BrFncCachedScopeNeq:`
`324`	`324`	`case Js::OpCode::BrOnObject_A:`
`325`	`325`	`case Js::OpCode::BrOnClassConstructor:`
	`326`	`+ case Js::OpCode::BrOnBaseConstructorKind:`
`326`	`327`	`if (tryUnsignedCmpPeep)`
`327`	`328`	`{`
`328`	`329`	`this->UnsignedCmpPeep(instr);`
Original file line number	Diff line number	Diff line change
`@@ -2113,16 +2113,31 @@ void ByteCodeGenerator::LoadThisObject(FuncInfo *funcInfo, bool thisLoadedFromPa`
`2113`	`2113`	`{`
`2114`	`2114`	`if (this->scriptContext->GetConfig()->IsES6ClassAndExtendsEnabled() && funcInfo->IsClassConstructor())`
`2115`	`2115`	`{`
`2116`		`- // Derived class constructors initialize 'this' to be Undecl`
	`2116`	`+ // Derived class constructors initialize 'this' to be Undecl except "extends null" cases`
`2117`	`2117`	`// - we'll check this value during a super call and during 'this' access`
`2118`		`- // Base class constructors initialize 'this' to a new object using new.target`
	`2118`	`+ //`
	`2119`	`+ // Base class constructors or "extends null" cases initialize 'this' to a new object using new.target`
`2119`	`2120`	`if (funcInfo->IsBaseClassConstructor())`
`2120`	`2121`	`{`
`2121`	`2122`	`EmitBaseClassConstructorThisObject(funcInfo);`
`2122`	`2123`	`}`
`2123`	`2124`	`else`
`2124`	`2125`	`{`
`2125`		`- this->m_writer.Reg1(Js::OpCode::InitUndecl, funcInfo->thisPointerRegister);`
	`2126`	`+ Js::ByteCodeLabel thisLabel = this->Writer()->DefineLabel();`
	`2127`	`+ Js::ByteCodeLabel skipLabel = this->Writer()->DefineLabel();`
	`2128`	`+`
	`2129`	`+ Js::RegSlot tmpReg = funcInfo->AcquireTmpRegister();`
	`2130`	`+ this->Writer()->Reg1(Js::OpCode::LdFuncObj, tmpReg);`
	`2131`	`+ this->Writer()->BrReg1(Js::OpCode::BrOnBaseConstructorKind, thisLabel, tmpReg); // branch when [[ConstructorKind]]=="base"`
	`2132`	`+ funcInfo->ReleaseTmpRegister(tmpReg);`
	`2133`	`+`
	`2134`	`+ this->m_writer.Reg1(Js::OpCode::InitUndecl, funcInfo->thisPointerRegister); // not "extends null" case`
	`2135`	`+ this->Writer()->Br(Js::OpCode::Br, skipLabel);`
	`2136`	`+`
	`2137`	`+ this->Writer()->MarkLabel(thisLabel);`
	`2138`	`+ EmitBaseClassConstructorThisObject(funcInfo); // "extends null" case`
	`2139`	`+`
	`2140`	`+ this->Writer()->MarkLabel(skipLabel);`
`2126`	`2141`	`}`
`2127`	`2142`	`}`
`2128`	`2143`	`else if (!funcInfo->IsGlobalFunction() \|\| (this->flags & fscrEval))`
`@@ -2411,12 +2426,6 @@ void ByteCodeGenerator::EmitClassConstructorEndCode(FuncInfo *funcInfo)`
`2411`	`2426`	`EmitScopeSlotLoadThis(funcInfo, funcInfo->thisPointerRegister);`
`2412`	`2427`	`this->Writer()->Reg2(Js::OpCode::Ld_A, ByteCodeGenerator::ReturnRegister, funcInfo->thisPointerRegister);`
`2413`	`2428`	`}`
`2414`		`- else`
`2415`		`- {`
`2416`		`- // This is the case where we don't have any references to this or super in the constructor or any nested lambda functions.`
`2417`		`- // We know 'this' must be undecl so let's just emit the ReferenceError as part of the fallthrough.`
`2418`		`- EmitUseBeforeDeclarationRuntimeError(this, ByteCodeGenerator::ReturnRegister);`
`2419`		`- }`
`2420`	`2429`	`}`
`2421`	`2430`
`2422`	`2431`	`void ByteCodeGenerator::EmitBaseClassConstructorThisObject(FuncInfo *funcInfo)`