Skip to content

Commit 6338ac2

Browse files
aneeshdkaneeshdk
committedJul 26, 2018
[1.10>master] [MERGE #5506 @aneeshdk] OS:12625415:Don't add let attribute for built-in arguments symbol when function has non-simple parameters
Merge pull request #5506 from aneeshdk:ArgsSymPropertyLetIssue In cached scope when we create the scope object we pass let attribute for all formals. This was marking the built-in arguments symbol also with let. This change removes it.
2 parents 1db67fb + 01e1f70 commit 6338ac2

File tree

5 files changed

+73
-7
lines changed

5 files changed

+73
-7
lines changed
 

‎lib/Runtime/Language/JavascriptOperators.cpp

+45-4
Original file line numberDiff line numberDiff line change
@@ -5706,11 +5706,18 @@ using namespace Js;
57065706
uint formalsSlotLimit = (firstFuncSlot != Constants::NoProperty) ? (uint)firstFuncSlot :
57075707
(firstVarSlot != Constants::NoProperty) ? (uint)firstVarSlot :
57085708
propIds->count;
5709-
type = PathTypeHandlerBase::CreateNewScopeObject(scriptContext, type, propIds, PropertyLet, formalsSlotLimit);
5709+
if (func->GetFunctionBody()->HasReferenceableBuiltInArguments())
5710+
{
5711+
type = PathTypeHandlerBase::CreateNewScopeObject<true>(scriptContext, type, propIds, PropertyLet, formalsSlotLimit);
5712+
}
5713+
else
5714+
{
5715+
type = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, type, propIds, PropertyLet, formalsSlotLimit);
5716+
}
57105717
}
57115718
else
57125719
{
5713-
type = PathTypeHandlerBase::CreateNewScopeObject(scriptContext, type, propIds);
5720+
type = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, type, propIds);
57145721
}
57155722
*literalType = type;
57165723
}
@@ -7167,7 +7174,26 @@ using namespace Js;
71677174
// CONSIDER : When we delay type sharing until the second instance is created, pass an argument indicating we want the types
71687175
// and handlers created here to be marked as shared up-front. This is to ensure we don't get any fixed fields and that the handler
71697176
// is ready for storing values directly to slots.
7170-
DynamicType* newType = PathTypeHandlerBase::CreateNewScopeObject(scriptContext, frameObject->GetDynamicType(), propIds, nonSimpleParamList ? PropertyLetDefaults : PropertyNone);
7177+
DynamicType* newType = nullptr;
7178+
if (nonSimpleParamList)
7179+
{
7180+
bool skipLetAttrForArguments = ((JavascriptGeneratorFunction::Is(funcCallee) || JavascriptAsyncFunction::Is(funcCallee)) ?
7181+
JavascriptGeneratorFunction::FromVar(funcCallee)->GetGeneratorVirtualScriptFunction()->GetFunctionBody()->HasReferenceableBuiltInArguments()
7182+
: funcCallee->GetFunctionBody()->HasReferenceableBuiltInArguments());
7183+
7184+
if (skipLetAttrForArguments)
7185+
{
7186+
newType = PathTypeHandlerBase::CreateNewScopeObject<true>(scriptContext, frameObject->GetDynamicType(), propIds, PropertyLetDefaults);
7187+
}
7188+
else
7189+
{
7190+
newType = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, frameObject->GetDynamicType(), propIds, PropertyLetDefaults);
7191+
}
7192+
}
7193+
else
7194+
{
7195+
newType = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, frameObject->GetDynamicType(), propIds);
7196+
}
71717197
int oldSlotCapacity = frameObject->GetDynamicType()->GetTypeHandler()->GetSlotCapacity();
71727198
int newSlotCapacity = newType->GetTypeHandler()->GetSlotCapacity();
71737199
__analysis_assume((uint32)newSlotCapacity >= formalsCount);
@@ -7267,7 +7293,22 @@ using namespace Js;
72677293
// CONSIDER : When we delay type sharing until the second instance is created, pass an argument indicating we want the types
72687294
// and handlers created here to be marked as shared up-front. This is to ensure we don't get any fixed fields and that the handler
72697295
// is ready for storing values directly to slots.
7270-
DynamicType* newType = PathTypeHandlerBase::CreateNewScopeObject(scriptContext, frameObject->GetDynamicType(), calleeBody->GetFormalsPropIdArray(), nonSimpleParamList ? PropertyLetDefaults : PropertyNone);
7296+
DynamicType* newType = nullptr;
7297+
if (nonSimpleParamList)
7298+
{
7299+
if (calleeBody->HasReferenceableBuiltInArguments())
7300+
{
7301+
newType = PathTypeHandlerBase::CreateNewScopeObject<true>(scriptContext, frameObject->GetDynamicType(), calleeBody->GetFormalsPropIdArray(), PropertyLetDefaults);
7302+
}
7303+
else
7304+
{
7305+
newType = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, frameObject->GetDynamicType(), calleeBody->GetFormalsPropIdArray(), PropertyLetDefaults);
7306+
}
7307+
}
7308+
else
7309+
{
7310+
newType = PathTypeHandlerBase::CreateNewScopeObject<false>(scriptContext, frameObject->GetDynamicType(), calleeBody->GetFormalsPropIdArray());
7311+
}
72717312

72727313
int oldSlotCapacity = frameObject->GetDynamicType()->GetTypeHandler()->GetSlotCapacity();
72737314
int newSlotCapacity = newType->GetTypeHandler()->GetSlotCapacity();

‎lib/Runtime/Types/PathTypeHandler.cpp

+9-2
Original file line numberDiff line numberDiff line change
@@ -1880,8 +1880,8 @@ namespace Js
18801880
return type;
18811881
}
18821882

1883-
DynamicType *
1884-
PathTypeHandlerBase::CreateNewScopeObject(ScriptContext *scriptContext, DynamicType *type, const PropertyIdArray *propIds, PropertyAttributes extraAttributes, uint extraAttributesSlotCount)
1883+
template <bool skipLetAttrForArguments>
1884+
DynamicType * PathTypeHandlerBase::CreateNewScopeObject(ScriptContext *scriptContext, DynamicType *type, const PropertyIdArray *propIds, PropertyAttributes extraAttributes, uint extraAttributesSlotCount)
18851885
{
18861886
uint count = propIds->count;
18871887

@@ -1899,6 +1899,11 @@ namespace Js
18991899
if (i < extraAttributesSlotCount)
19001900
{
19011901
attributes |= extraAttributes;
1902+
if (skipLetAttrForArguments && propertyId == PropertyIds::arguments)
1903+
{
1904+
// Do not add let attribute for built-in arguments symbol
1905+
attributes &= ~PropertyLet;
1906+
}
19021907
}
19031908
typeHandler->Add(propertyRecord, attributes, scriptContext);
19041909
}
@@ -1912,6 +1917,8 @@ namespace Js
19121917

19131918
return type;
19141919
}
1920+
template DynamicType * PathTypeHandlerBase::CreateNewScopeObject<true>(ScriptContext *scriptContext, DynamicType *type, const PropertyIdArray *propIds, PropertyAttributes extraAttributes, uint extraAttributesSlotCount);
1921+
template DynamicType * PathTypeHandlerBase::CreateNewScopeObject<false>(ScriptContext *scriptContext, DynamicType *type, const PropertyIdArray *propIds, PropertyAttributes extraAttributes, uint extraAttributesSlotCount);
19151922

19161923
template <bool isObjectLiteral>
19171924
DynamicType* PathTypeHandlerBase::PromoteType(DynamicType* predecessorType, const PathTypeSuccessorKey key, bool shareType, ScriptContext* scriptContext, DynamicObject* instance, PropertyIndex* propertyIndex)

‎lib/Runtime/Types/PathTypeHandler.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ namespace Js
127127
static bool ObjectSlotAttributesContains(const PropertyAttributes attr) { return attr == (attr & ObjectSlotAttr_PropertyAttributesMask); }
128128
static bool UsePathTypeHandlerForObjectLiteral(const PropertyIdArray *const propIds, bool *const check__proto__Ref = nullptr);
129129
static DynamicType* CreateTypeForNewScObject(ScriptContext* scriptContext, DynamicType* type, const Js::PropertyIdArray *propIds, bool shareType);
130-
static DynamicType* CreateNewScopeObject(ScriptContext* scriptContext, DynamicType* type, const Js::PropertyIdArray *propIds, PropertyAttributes extraAttributes = PropertyNone, uint extraAttributesSlotCount = UINT_MAX);
130+
template <bool skipLetAttrForArguments> static DynamicType* CreateNewScopeObject(ScriptContext* scriptContext, DynamicType* type, const Js::PropertyIdArray *propIds, PropertyAttributes extraAttributes = PropertyNone, uint extraAttributesSlotCount = UINT_MAX);
131131

132132
static PathTypeHandlerBase * FromTypeHandler(DynamicTypeHandler * const typeHandler) { Assert(typeHandler->IsPathTypeHandler()); return static_cast<PathTypeHandlerBase*>(typeHandler); }
133133

‎test/Bugs/ArgumentsAttrIssue.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
function f1(...args) {
7+
eval("var arguments = 1;");
8+
}
9+
f1();
10+
f1();
11+
f1(); // Shouldn't throw.
12+
print("PASSED");

‎test/Bugs/rlexe.xml

+6
Original file line numberDiff line numberDiff line change
@@ -517,4 +517,10 @@
517517
<compile-flags>-forceundodefer</compile-flags>
518518
</default>
519519
</test>
520+
<test>
521+
<default>
522+
<files>ArgumentsAttrIssue.js</files>
523+
<compile-flags>-force:cachedScope</compile-flags>
524+
</default>
525+
</test>
520526
</regress-exe>

0 commit comments

Comments
 (0)
Please sign in to comment.