From d0f361c5fe0fac863032804244c8b78030307c64 Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Mon, 20 Aug 2018 08:33:18 +0100 Subject: [PATCH 1/2] Fix asyncFuncion ans generatorFunction length configurability --- .../Library/JavascriptGeneratorFunction.cpp | 108 ------------------ .../Library/JavascriptGeneratorFunction.h | 4 - lib/Runtime/Library/JavascriptLibrary.cpp | 37 +++++- test/Function/funcAndboundFuncLength.js | 29 +++++ 4 files changed, 61 insertions(+), 117 deletions(-) diff --git a/lib/Runtime/Library/JavascriptGeneratorFunction.cpp b/lib/Runtime/Library/JavascriptGeneratorFunction.cpp index bc99a42d0bb..b7598d4079f 100644 --- a/lib/Runtime/Library/JavascriptGeneratorFunction.cpp +++ b/lib/Runtime/Library/JavascriptGeneratorFunction.cpp @@ -278,11 +278,6 @@ using namespace Js; PropertyQueryFlags JavascriptGeneratorFunction::HasPropertyQuery(PropertyId propertyId, _Inout_opt_ PropertyValueInfo* info) { - if (propertyId == PropertyIds::length) - { - return PropertyQueryFlags::Property_Found; - } - if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -294,12 +289,6 @@ using namespace Js; PropertyQueryFlags JavascriptGeneratorFunction::GetPropertyQuery(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) { - BOOL result; - if (GetPropertyBuiltIns(originalInstance, propertyId, value, info, requestContext, &result)) - { - return JavascriptConversion::BooleanToPropertyQueryFlags(result); - } - if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -316,12 +305,6 @@ using namespace Js; if (propertyRecord != nullptr) { - BOOL result; - if (GetPropertyBuiltIns(originalInstance, propertyRecord->GetPropertyId(), value, info, requestContext, &result)) - { - return JavascriptConversion::BooleanToPropertyQueryFlags(result); - } - if (propertyRecord->GetPropertyId() == PropertyIds::caller || propertyRecord->GetPropertyId() == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -332,27 +315,6 @@ using namespace Js; return JavascriptFunction::GetPropertyQuery(originalInstance, propertyNameString, value, info, requestContext); } - bool JavascriptGeneratorFunction::GetPropertyBuiltIns(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext, BOOL* result) - { - if (propertyId == PropertyIds::length) - { - // Cannot just call the base GetProperty for `length` because we need - // to get the length from our private ScriptFunction instead of ourself. - int len = 0; - Var varLength; - if (scriptFunction->GetProperty(this, PropertyIds::length, &varLength, NULL, requestContext)) - { - len = JavascriptConversion::ToInt32(varLength, requestContext); - } - - *value = JavascriptNumber::ToVar(len, requestContext); - *result = true; - return true; - } - - return false; - } - PropertyQueryFlags JavascriptGeneratorFunction::GetPropertyReferenceQuery(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext) { return JavascriptGeneratorFunction::GetPropertyQuery(originalInstance, propertyId, value, info, requestContext); @@ -360,12 +322,6 @@ using namespace Js; BOOL JavascriptGeneratorFunction::SetProperty(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) { - BOOL result; - if (SetPropertyBuiltIns(propertyId, value, flags, info, &result)) - { - return result; - } - if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -382,12 +338,6 @@ using namespace Js; if (propertyRecord != nullptr) { - BOOL result; - if (SetPropertyBuiltIns(propertyRecord->GetPropertyId(), value, flags, info, &result)) - { - return result; - } - if (propertyRecord->GetPropertyId() == PropertyIds::caller || propertyRecord->GetPropertyId() == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -398,36 +348,8 @@ using namespace Js; return JavascriptFunction::SetProperty(propertyNameString, value, flags, info); } - bool JavascriptGeneratorFunction::SetPropertyBuiltIns(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info, BOOL* result) - { - if (propertyId == PropertyIds::length) - { - JavascriptError::ThrowCantAssignIfStrictMode(flags, this->GetScriptContext()); - - *result = false; - return true; - } - - return false; - } - - BOOL JavascriptGeneratorFunction::SetAccessors(PropertyId propertyId, Var getter, Var setter, PropertyOperationFlags flags) - { - if (propertyId == PropertyIds::length) - { - return this->scriptFunction->SetAccessors(propertyId, getter, setter, flags); - } - - return JavascriptFunction::SetAccessors(propertyId, getter, setter, flags); - } - _Check_return_ _Success_(return) BOOL JavascriptGeneratorFunction::GetAccessors(PropertyId propertyId, _Outptr_result_maybenull_ Var* getter, _Outptr_result_maybenull_ Var* setter, ScriptContext* requestContext) { - if (propertyId == PropertyIds::length) - { - return this->scriptFunction->GetAccessors(propertyId, getter, setter, requestContext); - } - if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -445,11 +367,6 @@ using namespace Js; return DynamicObject::GetSetter(propertyId, setterValue, info, requestContext); } - if (propertyId == PropertyIds::length) - { - return this->scriptFunction->GetSetter(propertyId, setterValue, info, requestContext); - } - return JavascriptFunction::GetSetter(propertyId, setterValue, info, requestContext); } @@ -460,11 +377,6 @@ using namespace Js; if (propertyRecord != nullptr) { - if (propertyRecord->GetPropertyId() == PropertyIds::length) - { - return this->scriptFunction->GetSetter(propertyNameString, setterValue, info, requestContext); - } - if ((propertyRecord->GetPropertyId() == PropertyIds::caller || propertyRecord->GetPropertyId() == PropertyIds::arguments)) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -482,11 +394,6 @@ using namespace Js; BOOL JavascriptGeneratorFunction::DeleteProperty(PropertyId propertyId, PropertyOperationFlags flags) { - if (propertyId == PropertyIds::length) - { - return false; - } - if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -498,11 +405,6 @@ using namespace Js; BOOL JavascriptGeneratorFunction::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags) { - if (BuiltInPropertyRecords::length.Equals(propertyNameString)) - { - return false; - } - if (BuiltInPropertyRecords::caller.Equals(propertyNameString) || BuiltInPropertyRecords::arguments.Equals(propertyNameString)) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -514,11 +416,6 @@ using namespace Js; BOOL JavascriptGeneratorFunction::IsWritable(PropertyId propertyId) { - if (propertyId == PropertyIds::length) - { - return false; - } - if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. @@ -530,11 +427,6 @@ using namespace Js; BOOL JavascriptGeneratorFunction::IsEnumerable(PropertyId propertyId) { - if (propertyId == PropertyIds::length) - { - return false; - } - if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments) { // JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that. diff --git a/lib/Runtime/Library/JavascriptGeneratorFunction.h b/lib/Runtime/Library/JavascriptGeneratorFunction.h index f76b3b87f7b..bed37861e84 100644 --- a/lib/Runtime/Library/JavascriptGeneratorFunction.h +++ b/lib/Runtime/Library/JavascriptGeneratorFunction.h @@ -14,9 +14,6 @@ namespace Js static FunctionInfo functionInfo; Field(GeneratorVirtualScriptFunction*) scriptFunction; - bool GetPropertyBuiltIns(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext, BOOL* result); - bool SetPropertyBuiltIns(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info, BOOL* result); - protected: DEFINE_VTABLE_CTOR(JavascriptGeneratorFunction, ScriptFunctionBase); DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(JavascriptGeneratorFunction); @@ -65,7 +62,6 @@ namespace Js virtual BOOL SetProperty(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override; virtual BOOL SetProperty(JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override; - virtual BOOL SetAccessors(PropertyId propertyId, Var getter, Var setter, PropertyOperationFlags flags = PropertyOperation_None) override; _Check_return_ _Success_(return) virtual BOOL GetAccessors(PropertyId propertyId, _Outptr_result_maybenull_ Var* getter, _Outptr_result_maybenull_ Var* setter, ScriptContext* requestContext) override; virtual DescriptorFlags GetSetter(PropertyId propertyId, Var *setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override; virtual DescriptorFlags GetSetter(JavascriptString* propertyNameString, Var *setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override; diff --git a/lib/Runtime/Library/JavascriptLibrary.cpp b/lib/Runtime/Library/JavascriptLibrary.cpp index 3aa42992823..136d566bd4d 100644 --- a/lib/Runtime/Library/JavascriptLibrary.cpp +++ b/lib/Runtime/Library/JavascriptLibrary.cpp @@ -703,9 +703,20 @@ namespace Js bool isAnonymousFunction = function->IsAnonymousFunction(); JavascriptLibrary* javascriptLibrary = function->GetType()->GetLibrary(); - typeHandler->ConvertFunction(function, isAnonymousFunction ? javascriptLibrary->anonymousFunctionWithPrototypeTypeHandler : javascriptLibrary->functionWithPrototypeTypeHandler); + typeHandler->ConvertFunction(function, isAnonymousFunction ? javascriptLibrary->anonymousFunctionWithPrototypeTypeHandler : javascriptLibrary->functionWithPrototypeAndLengthTypeHandler); function->SetPropertyWithAttributes(PropertyIds::prototype, javascriptLibrary->CreateGeneratorConstructorPrototypeObject(), PropertyWritable, nullptr); + Var varLength; + GeneratorVirtualScriptFunction* scriptFunction = function->GetGeneratorVirtualScriptFunction(); + if (!scriptFunction->GetProperty(scriptFunction, PropertyIds::length, &varLength, nullptr, scriptFunction->GetScriptContext())) + { + // TODO - remove this if or convert it to a FailFast if this assert never triggers + // Nothing in the ChakraCore CI will reach this code + AssertMsg(false, "Initializing Generator function without a length property - why isn't there a length?."); + varLength = TaggedInt::ToVarUnchecked(0); + } + function->SetPropertyWithAttributes(PropertyIds::length, varLength, PropertyConfigurable, nullptr, PropertyOperation_None, SideEffects_None); + if (!isAnonymousFunction) { JavascriptString * functionName = nullptr; @@ -720,15 +731,31 @@ namespace Js bool JavascriptLibrary::InitializeAsyncFunction(DynamicObject *function, DeferredTypeHandlerBase * typeHandler, DeferredInitializeMode mode) { // Async function instances do not have a prototype property as they are not constructable - typeHandler->Convert(function, mode, 1); + JavascriptAsyncFunction* asyncFunction = JavascriptAsyncFunction::FromVar(function); - if (!JavascriptAsyncFunction::FromVar(function)->IsAnonymousFunction()) + if (!asyncFunction->IsAnonymousFunction()) { + typeHandler->Convert(function, mode, 2); JavascriptString * functionName = nullptr; DebugOnly(bool status = ) ((Js::JavascriptFunction*)function)->GetFunctionName(&functionName); Assert(status); function->SetPropertyWithAttributes(PropertyIds::name, functionName, PropertyConfigurable, nullptr); } + else + { + typeHandler->Convert(function, mode, 1); + } + + Var varLength; + GeneratorVirtualScriptFunction* scriptFunction = asyncFunction->GetGeneratorVirtualScriptFunction(); + if (!scriptFunction->GetProperty(scriptFunction, PropertyIds::length, &varLength, nullptr, scriptFunction->GetScriptContext())) + { + // TODO - remove this if or convert it to a FailFast if this assert never triggers + // Nothing in the ChakraCore CI will reach this code + AssertMsg(false, "Initializing Async function without a length property - why isn't there a length?."); + varLength = TaggedInt::ToVarUnchecked(0); + } + function->SetPropertyWithAttributes(PropertyIds::length, varLength, PropertyConfigurable, nullptr, PropertyOperation_None, SideEffects_None); return true; } @@ -880,14 +907,14 @@ namespace Js template DynamicTypeHandler * JavascriptLibrary::GetDeferredGeneratorFunctionTypeHandlerBase() { - return DeferredTypeHandler>::GetDefaultInstance(); + return DeferredTypeHandler>::GetDefaultInstance(); } template DynamicTypeHandler * JavascriptLibrary::GetDeferredAsyncFunctionTypeHandlerBase() { // Async functions do not have the prototype property - return DeferredTypeHandler>::GetDefaultInstance(); + return DeferredTypeHandler>::GetDefaultInstance(); } DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousPrototypeGeneratorFunctionTypeHandler() diff --git a/test/Function/funcAndboundFuncLength.js b/test/Function/funcAndboundFuncLength.js index f10d72ac17d..725245f5d08 100644 --- a/test/Function/funcAndboundFuncLength.js +++ b/test/Function/funcAndboundFuncLength.js @@ -70,12 +70,25 @@ const tests = [ function normalFunction (a, b) { } const anonymousFunction = function (a, b, c) { } const lambda = (a, b, c, d) => { } + const anonGen = function* (a, b) {} + function* genFunc () {} + async function asyncFunc (a) {} + const anonAsync = async function () { } + lengthDefaultState(normalFunction, 2, "function"); lengthDefaultState(anonymousFunction, 3, "Anonymous function"); lengthDefaultState(lambda, 4, "Lambda function"); + lengthDefaultState(anonGen, 2, "Anonymous generator"); + lengthDefaultState(genFunc, 0, "Generator function"); + lengthDefaultState(anonAsync, 0, "Anonymous async function"); + lengthDefaultState(asyncFunc, 1, "Async function"); deleteLength(normalFunction, "function"); deleteLength(anonymousFunction, "Anonymous function"); deleteLength(lambda, "Lambda function"); + deleteLength(anonGen, "Anonymous generator"); + deleteLength(genFunc, "Generator function"); + deleteLength(anonAsync, "Anonymous async function"); + deleteLength(asyncFunc, "Async function"); } }, { @@ -85,9 +98,17 @@ const tests = [ function normalFunction (a, b) { } const anonymousFunction = function (a, b, c) { } const lambda = (a, b, c, d) => { } + const anonGen = function* (a, b) {} + function* genFunc () {} + async function asyncFunc (a) {} + const anonAsync = async function () { } reDefineLength(normalFunction, "function"); reDefineLength(anonymousFunction, "Anonymous function"); reDefineLength(lambda, "Lambda function"); + reDefineLength(anonGen, "Lambda function"); + reDefineLength(genFunc, "Lambda function"); + reDefineLength(asyncFunc, "Lambda function"); + reDefineLength(anonAsync, "Lambda function"); } }, { @@ -97,16 +118,24 @@ const tests = [ function normalFunction (a, b) { } const anonymousFunction = function (a, b, c) { } const lambda = (a, b, c, d) => { } + function* genFunc (a, b, c, d, e) {} + async function asyncFunc (a, b) {} const boundNormal = normalFunction.bind({}, 1); const boundAnon = anonymousFunction.bind({}, 1, 1, 1, 1); const boundLambda = lambda.bind({}, 1, 1); + const boundGen = genFunc.bind({}, 1, 1, 1, 1); + const boundAsync = asyncFunc.bind({}, 1); lengthDefaultState(boundNormal, 1, "Bound Function"); lengthDefaultState(boundAnon, 0, "Anonymous Bound Function"); lengthDefaultState(boundLambda, 2, "Bound Lambda Function"); + lengthDefaultState(boundGen, 1, "Bound Generator Function"); + lengthDefaultState(boundAsync, 1, "Bound Async Function"); deleteLength(boundNormal, "Bound Function"); deleteLength(boundAnon, "Anonymous Bound Function"); deleteLength(boundLambda, "Bound Lambda Function"); + deleteLength(boundGen, 1, "Bound Generator Function"); + deleteLength(boundAsync, 1, "Bound Async Function"); } }, { From 8f31c2572c67557ef2b7fcff60ab926942710f5b Mon Sep 17 00:00:00 2001 From: rhuanjl Date: Mon, 20 Aug 2018 08:33:35 +0100 Subject: [PATCH 2/2] Revise TypeHandler names for consistency --- lib/Runtime/Library/JavascriptLibrary.cpp | 17 +++++++++++------ lib/Runtime/Library/JavascriptLibrary.h | 5 +++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/Runtime/Library/JavascriptLibrary.cpp b/lib/Runtime/Library/JavascriptLibrary.cpp index 136d566bd4d..1f894a235a8 100644 --- a/lib/Runtime/Library/JavascriptLibrary.cpp +++ b/lib/Runtime/Library/JavascriptLibrary.cpp @@ -937,7 +937,7 @@ namespace Js return JavascriptLibrary::GetDeferredAsyncFunctionTypeHandlerBase(); } - DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionTypeHandler() + DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionWithLengthTypeHandler() { return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase(); } @@ -957,17 +957,22 @@ namespace Js return DeferredTypeHandler>::GetDefaultInstance(); } - DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionTypeHandler() + DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionWithLengthTypeHandler() { return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase(); } + DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionTypeHandler() + { + return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase(); + } + DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionTypeHandler() { return GetDeferredFunctionTypeHandlerBase(); } - DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionTypeHandlerNoPrototype() + DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionWithLengthTypeHandler() { return GetDeferredFunctionTypeHandlerBase(); } @@ -979,13 +984,13 @@ namespace Js if (noPrototypeProperty) { scriptFunctionTypeHandler = isAnonymousFunction ? - this->GetDeferredAnonymousFunctionTypeHandler() : - this->GetDeferredFunctionTypeHandlerNoPrototype(); + this->GetDeferredAnonymousFunctionWithLengthTypeHandler() : + this->GetDeferredFunctionWithLengthTypeHandler(); } else { scriptFunctionTypeHandler = isAnonymousFunction ? - JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionTypeHandler() : + JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionWithLengthTypeHandler() : JavascriptLibrary::GetDeferredPrototypeFunctionWithNameAndLengthTypeHandler(); } return scriptFunctionTypeHandler; diff --git a/lib/Runtime/Library/JavascriptLibrary.h b/lib/Runtime/Library/JavascriptLibrary.h index 041d5e9cad6..d5ba90d690b 100644 --- a/lib/Runtime/Library/JavascriptLibrary.h +++ b/lib/Runtime/Library/JavascriptLibrary.h @@ -918,14 +918,15 @@ namespace Js static DynamicTypeHandler * GetDeferredPrototypeFunctionTypeHandler(ScriptContext* scriptContext); static DynamicTypeHandler * GetDeferredPrototypeFunctionWithLengthTypeHandler(ScriptContext* scriptContext); - static DynamicTypeHandler * GetDeferredAnonymousPrototypeFunctionTypeHandler(); + static DynamicTypeHandler * GetDeferredAnonymousPrototypeFunctionWithLengthTypeHandler(); static DynamicTypeHandler * GetDeferredAnonymousPrototypeGeneratorFunctionTypeHandler(); static DynamicTypeHandler * GetDeferredAnonymousPrototypeAsyncFunctionTypeHandler(); DynamicTypeHandler * GetDeferredFunctionTypeHandler(); - DynamicTypeHandler * GetDeferredFunctionTypeHandlerNoPrototype(); + DynamicTypeHandler * GetDeferredFunctionWithLengthTypeHandler(); DynamicTypeHandler * GetDeferredPrototypeFunctionWithNameAndLengthTypeHandler(); DynamicTypeHandler * ScriptFunctionTypeHandler(bool noPrototypeProperty, bool isAnonymousFunction); + DynamicTypeHandler * GetDeferredAnonymousFunctionWithLengthTypeHandler(); DynamicTypeHandler * GetDeferredAnonymousFunctionTypeHandler(); template static DynamicTypeHandler * GetDeferredFunctionTypeHandlerBase();