Skip to content

Commit

Permalink
[MSFT 12313182] Detect assignment to const in nested functions that w…
Browse files Browse the repository at this point in the history
…ere originally deferred. Add symbol-is-const flag to Symbol and to ScopeInfo to allow us to detect this case. Don't rely on having access to a parse node for the declaration.
  • Loading branch information
pleath committed Sep 6, 2017
1 parent f8307d2 commit 8f40df4
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 6 deletions.
2 changes: 1 addition & 1 deletion lib/Runtime/ByteCode/ByteCodeEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5169,7 +5169,7 @@ void ByteCodeGenerator::EmitPropStore(Js::RegSlot rhsLocation, Symbol *sym, Iden
}
else if (sym->IsInSlot(funcInfo) || envIndex != -1)
{
if (!isConstDecl && sym->GetDecl() && sym->GetDecl()->nop == knopConstDecl)
if (!isConstDecl && sym->GetIsConst())
{
// This is a case where const reassignment can't be proven statically (e.g., eval, with) so
// we have to catch it at runtime.
Expand Down
1 change: 1 addition & 0 deletions lib/Runtime/ByteCode/ByteCodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3552,6 +3552,7 @@ void PreVisitBlock(ParseNode *pnodeBlock, ByteCodeGenerator *byteCodeGenerator)
#endif
sym->SetIsGlobal(isGlobalScope);
sym->SetIsBlockVar(true);
sym->SetIsConst(pnode->nop == knopConstDecl);
sym->SetNeedDeclaration(true);
pnode->sxVar.sym = sym;
};
Expand Down
2 changes: 2 additions & 0 deletions lib/Runtime/ByteCode/ScopeInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ namespace Js
this->SetSymbolType(scopeSlot, sym->GetSymbolType());
this->SetHasFuncAssignment(scopeSlot, sym->GetHasFuncAssignment());
this->SetIsBlockVariable(scopeSlot, sym->GetIsBlockVar());
this->SetIsConst(scopeSlot, sym->GetIsConst());
this->SetIsFuncExpr(scopeSlot, sym->GetIsFuncExpr());
this->SetIsModuleExportStorage(scopeSlot, sym->GetIsModuleExportStorage());
this->SetIsModuleImport(scopeSlot, sym->GetIsModuleImport());
Expand Down Expand Up @@ -235,6 +236,7 @@ namespace Js

sym->SetScopeSlot(static_cast<PropertyId>(i));
sym->SetIsBlockVar(GetIsBlockVariable(i));
sym->SetIsConst(GetIsConst(i));
sym->SetIsFuncExpr(GetIsFuncExpr(i));
sym->SetIsModuleExportStorage(GetIsModuleExportStorage(i));
sym->SetIsModuleImport(GetIsModuleImport(i));
Expand Down
24 changes: 19 additions & 5 deletions lib/Runtime/ByteCode/ScopeInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@ namespace Js {
Field(PropertyRecord const*) name;
};
Field(SymbolType) symbolType;
Field(bool) hasFuncAssignment;
Field(bool) isBlockVariable;
Field(bool) isFuncExpr;
Field(bool) isModuleExportStorage;
Field(bool) isModuleImport;
Field(bool) hasFuncAssignment : 1;
Field(bool) isBlockVariable : 1;
Field(bool) isConst : 1;
Field(bool) isFuncExpr : 1;
Field(bool) isModuleExportStorage : 1;
Field(bool) isModuleImport : 1;
};

private:
Expand Down Expand Up @@ -86,6 +87,13 @@ namespace Js {
symbols[i].isBlockVariable = is;
}

void SetIsConst(int i, bool is)
{
Assert(!areNamesCached);
Assert(i >= 0 && i < symbolCount);
symbols[i].isConst = is;
}

void SetIsFuncExpr(int i, bool is)
{
Assert(!areNamesCached);
Expand Down Expand Up @@ -151,6 +159,12 @@ namespace Js {
return symbols[i].isBlockVariable;
}

bool GetIsConst(int i)
{
Assert(i >= 0 && i < symbolCount);
return symbols[i].isConst;
}

bool GetIsFuncExpr(int i)
{
Assert(i >= 0 && i < symbolCount);
Expand Down
12 changes: 12 additions & 0 deletions lib/Runtime/ByteCode/Symbol.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Symbol
BYTE defCount;
BYTE needDeclaration : 1;
BYTE isBlockVar : 1;
BYTE isConst : 1;
BYTE isGlobal : 1;
BYTE isEval : 1;
BYTE hasNonLocalReference : 1; // if true, then this symbol needs to be heap-allocated
Expand Down Expand Up @@ -61,6 +62,7 @@ class Symbol
location(Js::Constants::NoRegister),
needDeclaration(false),
isBlockVar(false),
isConst(false),
isGlobal(false),
hasNonLocalReference(false),
isFuncExpr(false),
Expand Down Expand Up @@ -150,6 +152,16 @@ class Symbol
return isBlockVar;
}

void SetIsConst(bool is)
{
isConst = is;
}

bool GetIsConst() const
{
return isConst;
}

void SetIsModuleExportStorage(bool is)
{
isModuleExportStorage = is;
Expand Down
2 changes: 2 additions & 0 deletions test/LetConst/AssignmentToConst.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,5 @@ test 23
ReferenceError: Use before declaration
test 24
ReferenceError: Use before declaration
test 25
TypeError: Assignment to const
1 change: 1 addition & 0 deletions test/LetConst/AssignmentToConst.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ try { eval("WScript.Echo('test 21'); const x = 1; {const x = 2; x++;}"); WScript
try { eval("WScript.Echo('test 22'); const x = 1; {const x = 2;} x++;"); WScript.Echo("passed"); } catch (e) { WScript.Echo(e); }
try { eval("WScript.Echo('test 23'); x = 1; {let x = 2;} const x = 10;"); WScript.Echo("passed"); } catch (e) { WScript.Echo(e); }
try { eval("WScript.Echo('test 24'); function f() {x = 1; {let x = 2;} const x = 10;} f();"); WScript.Echo("passed"); } catch (e) { WScript.Echo(e); }
try { eval("WScript.Echo('test 25'); const x = 10; function f() {x = 1; {let x = 2;} } f();"); WScript.Echo("passed"); } catch (e) { WScript.Echo(e); }


7 changes: 7 additions & 0 deletions test/LetConst/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@
<baseline>AssignmentToConst.baseline</baseline>
</default>
</test>
<test>
<default>
<files>AssignmentToConst.js</files>
<baseline>AssignmentToConst.baseline</baseline>
<compile-flags>-force:deferparse</compile-flags>
</default>
</test>
<test>
<default>
<files>DeclOutofBlock.js</files>
Expand Down

0 comments on commit 8f40df4

Please sign in to comment.