Skip to content

Commit 0b13df8

Browse files
jackhortonjackhorton
committed
[MERGE chakra-core#5610 @rhuanjl] Make Generator and Async functions have configurable length
Merge pull request chakra-core#5610 from rhuanjl:generatorLength Follow up to chakra-core#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 chakra-core#5405) it also prevents some internal anonymous functions from unnecessarily being given a length property - unintentional side effect of chakra-core#5405. fixes: chakra-core#5419 CC: @pleath @jackhorton
2 parents 84b553f + 8f31c25 commit 0b13df8

5 files changed

Lines changed: 75 additions & 125 deletions

File tree

‎lib/Runtime/Library/JavascriptGeneratorFunction.cpp‎

Lines changed: 0 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -278,11 +278,6 @@ using namespace Js;
278278

279279
PropertyQueryFlags JavascriptGeneratorFunction::HasPropertyQuery(PropertyId propertyId, _Inout_opt_ PropertyValueInfo* info)
280280
{
281-
if (propertyId == PropertyIds::length)
282-
{
283-
return PropertyQueryFlags::Property_Found;
284-
}
285-
286281
if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments)
287282
{
288283
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -294,12 +289,6 @@ using namespace Js;
294289

295290
PropertyQueryFlags JavascriptGeneratorFunction::GetPropertyQuery(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
296291
{
297-
BOOL result;
298-
if (GetPropertyBuiltIns(originalInstance, propertyId, value, info, requestContext, &result))
299-
{
300-
return JavascriptConversion::BooleanToPropertyQueryFlags(result);
301-
}
302-
303292
if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments)
304293
{
305294
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -316,12 +305,6 @@ using namespace Js;
316305

317306
if (propertyRecord != nullptr)
318307
{
319-
BOOL result;
320-
if (GetPropertyBuiltIns(originalInstance, propertyRecord->GetPropertyId(), value, info, requestContext, &result))
321-
{
322-
return JavascriptConversion::BooleanToPropertyQueryFlags(result);
323-
}
324-
325308
if (propertyRecord->GetPropertyId() == PropertyIds::caller || propertyRecord->GetPropertyId() == PropertyIds::arguments)
326309
{
327310
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -332,40 +315,13 @@ using namespace Js;
332315
return JavascriptFunction::GetPropertyQuery(originalInstance, propertyNameString, value, info, requestContext);
333316
}
334317

335-
bool JavascriptGeneratorFunction::GetPropertyBuiltIns(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext, BOOL* result)
336-
{
337-
if (propertyId == PropertyIds::length)
338-
{
339-
// Cannot just call the base GetProperty for `length` because we need
340-
// to get the length from our private ScriptFunction instead of ourself.
341-
int len = 0;
342-
Var varLength;
343-
if (scriptFunction->GetProperty(this, PropertyIds::length, &varLength, NULL, requestContext))
344-
{
345-
len = JavascriptConversion::ToInt32(varLength, requestContext);
346-
}
347-
348-
*value = JavascriptNumber::ToVar(len, requestContext);
349-
*result = true;
350-
return true;
351-
}
352-
353-
return false;
354-
}
355-
356318
PropertyQueryFlags JavascriptGeneratorFunction::GetPropertyReferenceQuery(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext)
357319
{
358320
return JavascriptGeneratorFunction::GetPropertyQuery(originalInstance, propertyId, value, info, requestContext);
359321
}
360322

361323
BOOL JavascriptGeneratorFunction::SetProperty(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info)
362324
{
363-
BOOL result;
364-
if (SetPropertyBuiltIns(propertyId, value, flags, info, &result))
365-
{
366-
return result;
367-
}
368-
369325
if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments)
370326
{
371327
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -382,12 +338,6 @@ using namespace Js;
382338

383339
if (propertyRecord != nullptr)
384340
{
385-
BOOL result;
386-
if (SetPropertyBuiltIns(propertyRecord->GetPropertyId(), value, flags, info, &result))
387-
{
388-
return result;
389-
}
390-
391341
if (propertyRecord->GetPropertyId() == PropertyIds::caller || propertyRecord->GetPropertyId() == PropertyIds::arguments)
392342
{
393343
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -398,36 +348,8 @@ using namespace Js;
398348
return JavascriptFunction::SetProperty(propertyNameString, value, flags, info);
399349
}
400350

401-
bool JavascriptGeneratorFunction::SetPropertyBuiltIns(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info, BOOL* result)
402-
{
403-
if (propertyId == PropertyIds::length)
404-
{
405-
JavascriptError::ThrowCantAssignIfStrictMode(flags, this->GetScriptContext());
406-
407-
*result = false;
408-
return true;
409-
}
410-
411-
return false;
412-
}
413-
414-
BOOL JavascriptGeneratorFunction::SetAccessors(PropertyId propertyId, Var getter, Var setter, PropertyOperationFlags flags)
415-
{
416-
if (propertyId == PropertyIds::length)
417-
{
418-
return this->scriptFunction->SetAccessors(propertyId, getter, setter, flags);
419-
}
420-
421-
return JavascriptFunction::SetAccessors(propertyId, getter, setter, flags);
422-
}
423-
424351
_Check_return_ _Success_(return) BOOL JavascriptGeneratorFunction::GetAccessors(PropertyId propertyId, _Outptr_result_maybenull_ Var* getter, _Outptr_result_maybenull_ Var* setter, ScriptContext* requestContext)
425352
{
426-
if (propertyId == PropertyIds::length)
427-
{
428-
return this->scriptFunction->GetAccessors(propertyId, getter, setter, requestContext);
429-
}
430-
431353
if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments)
432354
{
433355
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -445,11 +367,6 @@ using namespace Js;
445367
return DynamicObject::GetSetter(propertyId, setterValue, info, requestContext);
446368
}
447369

448-
if (propertyId == PropertyIds::length)
449-
{
450-
return this->scriptFunction->GetSetter(propertyId, setterValue, info, requestContext);
451-
}
452-
453370
return JavascriptFunction::GetSetter(propertyId, setterValue, info, requestContext);
454371
}
455372

@@ -460,11 +377,6 @@ using namespace Js;
460377

461378
if (propertyRecord != nullptr)
462379
{
463-
if (propertyRecord->GetPropertyId() == PropertyIds::length)
464-
{
465-
return this->scriptFunction->GetSetter(propertyNameString, setterValue, info, requestContext);
466-
}
467-
468380
if ((propertyRecord->GetPropertyId() == PropertyIds::caller || propertyRecord->GetPropertyId() == PropertyIds::arguments))
469381
{
470382
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -482,11 +394,6 @@ using namespace Js;
482394

483395
BOOL JavascriptGeneratorFunction::DeleteProperty(PropertyId propertyId, PropertyOperationFlags flags)
484396
{
485-
if (propertyId == PropertyIds::length)
486-
{
487-
return false;
488-
}
489-
490397
if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments)
491398
{
492399
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -498,11 +405,6 @@ using namespace Js;
498405

499406
BOOL JavascriptGeneratorFunction::DeleteProperty(JavascriptString *propertyNameString, PropertyOperationFlags flags)
500407
{
501-
if (BuiltInPropertyRecords::length.Equals(propertyNameString))
502-
{
503-
return false;
504-
}
505-
506408
if (BuiltInPropertyRecords::caller.Equals(propertyNameString) || BuiltInPropertyRecords::arguments.Equals(propertyNameString))
507409
{
508410
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -514,11 +416,6 @@ using namespace Js;
514416

515417
BOOL JavascriptGeneratorFunction::IsWritable(PropertyId propertyId)
516418
{
517-
if (propertyId == PropertyIds::length)
518-
{
519-
return false;
520-
}
521-
522419
if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments)
523420
{
524421
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.
@@ -530,11 +427,6 @@ using namespace Js;
530427

531428
BOOL JavascriptGeneratorFunction::IsEnumerable(PropertyId propertyId)
532429
{
533-
if (propertyId == PropertyIds::length)
534-
{
535-
return false;
536-
}
537-
538430
if (propertyId == PropertyIds::caller || propertyId == PropertyIds::arguments)
539431
{
540432
// JavascriptFunction has special case for caller and arguments; call DynamicObject:: virtual directly to skip that.

‎lib/Runtime/Library/JavascriptGeneratorFunction.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@ namespace Js
1414
static FunctionInfo functionInfo;
1515
Field(GeneratorVirtualScriptFunction*) scriptFunction;
1616

17-
bool GetPropertyBuiltIns(Var originalInstance, PropertyId propertyId, Var* value, PropertyValueInfo* info, ScriptContext* requestContext, BOOL* result);
18-
bool SetPropertyBuiltIns(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info, BOOL* result);
19-
2017
protected:
2118
DEFINE_VTABLE_CTOR(JavascriptGeneratorFunction, ScriptFunctionBase);
2219
DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(JavascriptGeneratorFunction);
@@ -65,7 +62,6 @@ namespace Js
6562
virtual BOOL SetProperty(PropertyId propertyId, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
6663
virtual BOOL SetProperty(JavascriptString* propertyNameString, Var value, PropertyOperationFlags flags, PropertyValueInfo* info) override;
6764

68-
virtual BOOL SetAccessors(PropertyId propertyId, Var getter, Var setter, PropertyOperationFlags flags = PropertyOperation_None) override;
6965
_Check_return_ _Success_(return) virtual BOOL GetAccessors(PropertyId propertyId, _Outptr_result_maybenull_ Var* getter, _Outptr_result_maybenull_ Var* setter, ScriptContext* requestContext) override;
7066
virtual DescriptorFlags GetSetter(PropertyId propertyId, Var *setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;
7167
virtual DescriptorFlags GetSetter(JavascriptString* propertyNameString, Var *setterValue, PropertyValueInfo* info, ScriptContext* requestContext) override;

‎lib/Runtime/Library/JavascriptLibrary.cpp‎

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -703,9 +703,20 @@ namespace Js
703703
bool isAnonymousFunction = function->IsAnonymousFunction();
704704

705705
JavascriptLibrary* javascriptLibrary = function->GetType()->GetLibrary();
706-
typeHandler->ConvertFunction(function, isAnonymousFunction ? javascriptLibrary->anonymousFunctionWithPrototypeTypeHandler : javascriptLibrary->functionWithPrototypeTypeHandler);
706+
typeHandler->ConvertFunction(function, isAnonymousFunction ? javascriptLibrary->anonymousFunctionWithPrototypeTypeHandler : javascriptLibrary->functionWithPrototypeAndLengthTypeHandler);
707707
function->SetPropertyWithAttributes(PropertyIds::prototype, javascriptLibrary->CreateGeneratorConstructorPrototypeObject(), PropertyWritable, nullptr);
708708

709+
Var varLength;
710+
GeneratorVirtualScriptFunction* scriptFunction = function->GetGeneratorVirtualScriptFunction();
711+
if (!scriptFunction->GetProperty(scriptFunction, PropertyIds::length, &varLength, nullptr, scriptFunction->GetScriptContext()))
712+
{
713+
// TODO - remove this if or convert it to a FailFast if this assert never triggers
714+
// Nothing in the ChakraCore CI will reach this code
715+
AssertMsg(false, "Initializing Generator function without a length property - why isn't there a length?.");
716+
varLength = TaggedInt::ToVarUnchecked(0);
717+
}
718+
function->SetPropertyWithAttributes(PropertyIds::length, varLength, PropertyConfigurable, nullptr, PropertyOperation_None, SideEffects_None);
719+
709720
if (!isAnonymousFunction)
710721
{
711722
JavascriptString * functionName = nullptr;
@@ -720,15 +731,31 @@ namespace Js
720731
bool JavascriptLibrary::InitializeAsyncFunction(DynamicObject *function, DeferredTypeHandlerBase * typeHandler, DeferredInitializeMode mode)
721732
{
722733
// Async function instances do not have a prototype property as they are not constructable
723-
typeHandler->Convert(function, mode, 1);
734+
JavascriptAsyncFunction* asyncFunction = JavascriptAsyncFunction::FromVar(function);
724735

725-
if (!JavascriptAsyncFunction::FromVar(function)->IsAnonymousFunction())
736+
if (!asyncFunction->IsAnonymousFunction())
726737
{
738+
typeHandler->Convert(function, mode, 2);
727739
JavascriptString * functionName = nullptr;
728740
DebugOnly(bool status = ) ((Js::JavascriptFunction*)function)->GetFunctionName(&functionName);
729741
Assert(status);
730742
function->SetPropertyWithAttributes(PropertyIds::name, functionName, PropertyConfigurable, nullptr);
731743
}
744+
else
745+
{
746+
typeHandler->Convert(function, mode, 1);
747+
}
748+
749+
Var varLength;
750+
GeneratorVirtualScriptFunction* scriptFunction = asyncFunction->GetGeneratorVirtualScriptFunction();
751+
if (!scriptFunction->GetProperty(scriptFunction, PropertyIds::length, &varLength, nullptr, scriptFunction->GetScriptContext()))
752+
{
753+
// TODO - remove this if or convert it to a FailFast if this assert never triggers
754+
// Nothing in the ChakraCore CI will reach this code
755+
AssertMsg(false, "Initializing Async function without a length property - why isn't there a length?.");
756+
varLength = TaggedInt::ToVarUnchecked(0);
757+
}
758+
function->SetPropertyWithAttributes(PropertyIds::length, varLength, PropertyConfigurable, nullptr, PropertyOperation_None, SideEffects_None);
732759

733760
return true;
734761
}
@@ -880,14 +907,14 @@ namespace Js
880907
template<bool isNameAvailable, bool isPrototypeAvailable>
881908
DynamicTypeHandler * JavascriptLibrary::GetDeferredGeneratorFunctionTypeHandlerBase()
882909
{
883-
return DeferredTypeHandler<InitializeGeneratorFunction, InitializeFunctionDeferredTypeHandlerFilter<isNameAvailable, isPrototypeAvailable>>::GetDefaultInstance();
910+
return DeferredTypeHandler<InitializeGeneratorFunction, InitializeFunctionDeferredTypeHandlerFilter<isNameAvailable, isPrototypeAvailable, /*isLengthAvailable*/ true>>::GetDefaultInstance();
884911
}
885912

886913
template<bool isNameAvailable>
887914
DynamicTypeHandler * JavascriptLibrary::GetDeferredAsyncFunctionTypeHandlerBase()
888915
{
889916
// Async functions do not have the prototype property
890-
return DeferredTypeHandler<InitializeAsyncFunction, InitializeFunctionDeferredTypeHandlerFilter<isNameAvailable, /* isPrototypeAvailable */ false>>::GetDefaultInstance();
917+
return DeferredTypeHandler<InitializeAsyncFunction, InitializeFunctionDeferredTypeHandlerFilter<isNameAvailable, /* isPrototypeAvailable */ false, /*isLengthAvailable*/ true>>::GetDefaultInstance();
891918
}
892919

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

913-
DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionTypeHandler()
940+
DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionWithLengthTypeHandler()
914941
{
915942
return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase</*isNameAvailable*/ false, /* isPrototypeAvailable */ true, /* isLengthAvailable */ true>();
916943
}
@@ -930,17 +957,22 @@ namespace Js
930957
return DeferredTypeHandler<Js::JavascriptExternalFunction::DeferredLengthInitializer, InitializeFunctionDeferredTypeHandlerFilter</* isNameAvailable */ true, /* isPrototypeAvailable */ true, /* isLengthAvailable */ true>>::GetDefaultInstance();
931958
}
932959

933-
DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionTypeHandler()
960+
DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionWithLengthTypeHandler()
934961
{
935962
return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase</* isNameAvailable */ false, /* isPrototypeAvailable */ false, /* isLengthAvailable */ true>();
936963
}
937964

965+
DynamicTypeHandler * JavascriptLibrary::GetDeferredAnonymousFunctionTypeHandler()
966+
{
967+
return JavascriptLibrary::GetDeferredFunctionTypeHandlerBase</* isNameAvailable */ false, /* isPrototypeAvailable */ false, /* isLengthAvailable */ false>();
968+
}
969+
938970
DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionTypeHandler()
939971
{
940972
return GetDeferredFunctionTypeHandlerBase</*isNameAvailable*/ true, /*isPrototypeAvailable*/ false, /* isLengthAvailable */ false>();
941973
}
942974

943-
DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionTypeHandlerNoPrototype()
975+
DynamicTypeHandler * JavascriptLibrary::GetDeferredFunctionWithLengthTypeHandler()
944976
{
945977
return GetDeferredFunctionTypeHandlerBase</*isNameAvailable*/ true, /*isPrototypeAvailable*/ false, /* isLengthAvailable */ true>();
946978
}
@@ -952,13 +984,13 @@ namespace Js
952984
if (noPrototypeProperty)
953985
{
954986
scriptFunctionTypeHandler = isAnonymousFunction ?
955-
this->GetDeferredAnonymousFunctionTypeHandler() :
956-
this->GetDeferredFunctionTypeHandlerNoPrototype();
987+
this->GetDeferredAnonymousFunctionWithLengthTypeHandler() :
988+
this->GetDeferredFunctionWithLengthTypeHandler();
957989
}
958990
else
959991
{
960992
scriptFunctionTypeHandler = isAnonymousFunction ?
961-
JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionTypeHandler() :
993+
JavascriptLibrary::GetDeferredAnonymousPrototypeFunctionWithLengthTypeHandler() :
962994
JavascriptLibrary::GetDeferredPrototypeFunctionWithNameAndLengthTypeHandler();
963995
}
964996
return scriptFunctionTypeHandler;

‎lib/Runtime/Library/JavascriptLibrary.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -918,14 +918,15 @@ namespace Js
918918

919919
static DynamicTypeHandler * GetDeferredPrototypeFunctionTypeHandler(ScriptContext* scriptContext);
920920
static DynamicTypeHandler * GetDeferredPrototypeFunctionWithLengthTypeHandler(ScriptContext* scriptContext);
921-
static DynamicTypeHandler * GetDeferredAnonymousPrototypeFunctionTypeHandler();
921+
static DynamicTypeHandler * GetDeferredAnonymousPrototypeFunctionWithLengthTypeHandler();
922922
static DynamicTypeHandler * GetDeferredAnonymousPrototypeGeneratorFunctionTypeHandler();
923923
static DynamicTypeHandler * GetDeferredAnonymousPrototypeAsyncFunctionTypeHandler();
924924

925925
DynamicTypeHandler * GetDeferredFunctionTypeHandler();
926-
DynamicTypeHandler * GetDeferredFunctionTypeHandlerNoPrototype();
926+
DynamicTypeHandler * GetDeferredFunctionWithLengthTypeHandler();
927927
DynamicTypeHandler * GetDeferredPrototypeFunctionWithNameAndLengthTypeHandler();
928928
DynamicTypeHandler * ScriptFunctionTypeHandler(bool noPrototypeProperty, bool isAnonymousFunction);
929+
DynamicTypeHandler * GetDeferredAnonymousFunctionWithLengthTypeHandler();
929930
DynamicTypeHandler * GetDeferredAnonymousFunctionTypeHandler();
930931
template<bool isNameAvailable, bool isPrototypeAvailable = true, bool isLengthAvailable = false>
931932
static DynamicTypeHandler * GetDeferredFunctionTypeHandlerBase();

0 commit comments

Comments
 (0)