Skip to content

Commit d0f361c

Browse files
rhuanjlrhuanjl
committed
Fix asyncFuncion ans generatorFunction length configurability
1 parent 45a6271 commit d0f361c

File tree

4 files changed

+61
-117
lines changed

4 files changed

+61
-117
lines changed

‎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: 32 additions & 5 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()

‎test/Function/funcAndboundFuncLength.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,25 @@ const tests = [
7070
function normalFunction (a, b) { }
7171
const anonymousFunction = function (a, b, c) { }
7272
const lambda = (a, b, c, d) => { }
73+
const anonGen = function* (a, b) {}
74+
function* genFunc () {}
75+
async function asyncFunc (a) {}
76+
const anonAsync = async function () { }
77+
7378
lengthDefaultState(normalFunction, 2, "function");
7479
lengthDefaultState(anonymousFunction, 3, "Anonymous function");
7580
lengthDefaultState(lambda, 4, "Lambda function");
81+
lengthDefaultState(anonGen, 2, "Anonymous generator");
82+
lengthDefaultState(genFunc, 0, "Generator function");
83+
lengthDefaultState(anonAsync, 0, "Anonymous async function");
84+
lengthDefaultState(asyncFunc, 1, "Async function");
7685
deleteLength(normalFunction, "function");
7786
deleteLength(anonymousFunction, "Anonymous function");
7887
deleteLength(lambda, "Lambda function");
88+
deleteLength(anonGen, "Anonymous generator");
89+
deleteLength(genFunc, "Generator function");
90+
deleteLength(anonAsync, "Anonymous async function");
91+
deleteLength(asyncFunc, "Async function");
7992
}
8093
},
8194
{
@@ -85,9 +98,17 @@ const tests = [
8598
function normalFunction (a, b) { }
8699
const anonymousFunction = function (a, b, c) { }
87100
const lambda = (a, b, c, d) => { }
101+
const anonGen = function* (a, b) {}
102+
function* genFunc () {}
103+
async function asyncFunc (a) {}
104+
const anonAsync = async function () { }
88105
reDefineLength(normalFunction, "function");
89106
reDefineLength(anonymousFunction, "Anonymous function");
90107
reDefineLength(lambda, "Lambda function");
108+
reDefineLength(anonGen, "Lambda function");
109+
reDefineLength(genFunc, "Lambda function");
110+
reDefineLength(asyncFunc, "Lambda function");
111+
reDefineLength(anonAsync, "Lambda function");
91112
}
92113
},
93114
{
@@ -97,16 +118,24 @@ const tests = [
97118
function normalFunction (a, b) { }
98119
const anonymousFunction = function (a, b, c) { }
99120
const lambda = (a, b, c, d) => { }
121+
function* genFunc (a, b, c, d, e) {}
122+
async function asyncFunc (a, b) {}
100123
const boundNormal = normalFunction.bind({}, 1);
101124
const boundAnon = anonymousFunction.bind({}, 1, 1, 1, 1);
102125
const boundLambda = lambda.bind({}, 1, 1);
126+
const boundGen = genFunc.bind({}, 1, 1, 1, 1);
127+
const boundAsync = asyncFunc.bind({}, 1);
103128

104129
lengthDefaultState(boundNormal, 1, "Bound Function");
105130
lengthDefaultState(boundAnon, 0, "Anonymous Bound Function");
106131
lengthDefaultState(boundLambda, 2, "Bound Lambda Function");
132+
lengthDefaultState(boundGen, 1, "Bound Generator Function");
133+
lengthDefaultState(boundAsync, 1, "Bound Async Function");
107134
deleteLength(boundNormal, "Bound Function");
108135
deleteLength(boundAnon, "Anonymous Bound Function");
109136
deleteLength(boundLambda, "Bound Lambda Function");
137+
deleteLength(boundGen, 1, "Bound Generator Function");
138+
deleteLength(boundAsync, 1, "Bound Async Function");
110139
}
111140
},
112141
{

0 commit comments

Comments
 (0)