Skip to content

Commit

Permalink
[MERGE #5610 @rhuanjl] Make Generator and Async functions have config…
Browse files Browse the repository at this point in the history 
…urable length

Merge pull request #5610 from rhuanjl:generatorLength

Follow up to #5405

Generator and async functions should also have configurable lengths - currently they don't.
Specific test262 tests:
https://github.com/tc39/test262/blob/master/test/built-ins/AsyncFunction/instance-length.js
https://github.com/tc39/test262/blob/master/test/built-ins/GeneratorFunction/instance-length.js

Additionally the second commit restores some consistency to function type handler names (I'd unintentionally made these inconsistent in #5405) it also prevents some internal anonymous functions from unnecessarily being given a length property - unintentional side effect of #5405.

fixes: #5419

CC: @pleath @jackhorton
  • Loading branch information
@jackhorton
jackhorton committed Aug 27, 2018
2 parents 84b553f + 8f31c25 commit 0b13df8
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 125 deletions.
108 changes: 0 additions & 108 deletions lib/Runtime/Library/JavascriptGeneratorFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -332,40 +315,13 @@ 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);
}

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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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);
}

Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down
4 changes: 0 additions & 4 deletions lib/Runtime/Library/JavascriptGeneratorFunction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
54 changes: 43 additions & 11 deletions lib/Runtime/Library/JavascriptLibrary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -880,14 +907,14 @@ namespace Js
template<bool isNameAvailable, bool isPrototypeAvailable>
DynamicTypeHandler * JavascriptLibrary::GetDeferredGeneratorFunctionTypeHandlerBase()
{
return DeferredTypeHandler<InitializeGeneratorFunction, InitializeFunctionDeferredTypeHandlerFilter<isNameAvailable, isPrototypeAvailable>>::GetDefaultInstance();
return DeferredTypeHandler<InitializeGeneratorFunction, InitializeFunctionDeferredTypeHandlerFilter<isNameAvailable, isPrototypeAvailable, /*isLengthAvailable*/ true>>::GetDefaultInstance();
}

template<bool isNameAvailable>
DynamicTypeHandler * JavascriptLibrary::GetDeferredAsyncFunctionTypeHandlerBase()
{
// Async functions do not have the prototype property
return DeferredTypeHandler<InitializeAsyncFunction, InitializeFunctionDeferredTypeHandlerFilter<isNameAvailable, /* isPrototypeAvailable */ false>>::GetDefaultInstance();
return DeferredTypeHandler<InitializeAsyncFunction, InitializeFunctionDeferredTypeHandlerFilter<isNameAvailable, /* isPrototypeAvailable */ false, /*isLengthAvailable*/ true>>::GetDefaultInstance();
}

DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousPrototypeGeneratorFunctionTypeHandler()
Expand All @@ -910,7 +937,7 @@ namespace Js
return JavascriptLibrary::GetDeferredAsyncFunctionTypeHandlerBase</*isNameAvailable*/ true>();
}

DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionTypeHandler()
DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionWithLengthTypeHandler()
{
return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase</*isNameAvailable*/ false, /* isPrototypeAvailable */ true, /* isLengthAvailable */ true>();
}
Expand All @@ -930,17 +957,22 @@ namespace Js
return DeferredTypeHandler<Js::JavascriptExternalFunction::DeferredLengthInitializer, InitializeFunctionDeferredTypeHandlerFilter</* isNameAvailable */ true, /* isPrototypeAvailable */ true, /* isLengthAvailable */ true>>::GetDefaultInstance();
}

DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionTypeHandler()
DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionWithLengthTypeHandler()
{
return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase</* isNameAvailable */ false, /* isPrototypeAvailable */ false, /* isLengthAvailable */ true>();
}

DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionTypeHandler()
{
return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase</* isNameAvailable */ false, /* isPrototypeAvailable */ false, /* isLengthAvailable */ false>();
}

DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionTypeHandler()
{
return GetDeferredFunctionTypeHandlerBase</*isNameAvailable*/ true, /*isPrototypeAvailable*/ false, /* isLengthAvailable */ false>();
}

DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionTypeHandlerNoPrototype()
DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionWithLengthTypeHandler()
{
return GetDeferredFunctionTypeHandlerBase</*isNameAvailable*/ true, /*isPrototypeAvailable*/ false, /* isLengthAvailable */ true>();
}
Expand All @@ -952,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;
Expand Down
5 changes: 3 additions & 2 deletions lib/Runtime/Library/JavascriptLibrary.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool isNameAvailable, bool isPrototypeAvailable = true, bool isLengthAvailable = false>
static DynamicTypeHandler * GetDeferredFunctionTypeHandlerBase();
Expand Down
Loading

0 comments on commit 0b13df8

Please sign in to comment.