Skip to content

Commit 127f153

Browse files
committed
ParseNode cleanup
Only use parse node opcode flag fnopBin and fnopUni on ParseNodeBin and ParseNodeUni respectively. This avoid potential type confusion on the ParseNode type. Luckily, the types where we had confusion had and equivalent structure and thus was correctly used. Remove fnopBinList as it serves no purpose. Remove knopSuperCall as it was used only for debug verification that wasn't enforcing anything. Fixes OS#16252562
1 parent 587db51 commit 127f153

File tree

8 files changed

+56
-48
lines changed

8 files changed

+56
-48
lines changed

lib/Parser/ptlist.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ PTNODE(knopLt , "<" , OP(Lt) , Bin , fnopBin|fn
7777
PTNODE(knopLe , "<=" , OP(Le) , Bin , fnopBin|fnopRel , "LessThanEqualOper" )
7878
PTNODE(knopGe , ">=" , OP(Ge) , Bin , fnopBin|fnopRel , "GreaterThanEqualOper" )
7979
PTNODE(knopGt , ">" , OP(Gt) , Bin , fnopBin|fnopRel , "GreaterThanOper" )
80-
PTNODE(knopCall , "()" , Nop , Call , fnopBin , "CallExpr" )
80+
PTNODE(knopCall , "()" , Nop , Call , fnopNone , "CallExpr" )
8181
PTNODE(knopDot , "." , Nop , Bin , fnopBin , "DotOper" )
8282
PTNODE(knopAsg , "=" , Nop , Bin , fnopBin|fnopAsg , "AssignmentOper" )
8383
PTNODE(knopInstOf , "instanceof" , IsInst , Bin , fnopBin|fnopRel , "InstanceOfExpr" )
@@ -90,9 +90,9 @@ PTNODE(knopLogAnd , "&&" , Nop , Bin , fnopBin
9090
PTNODE(knopLsh , "<<" , Shl_A , Bin , fnopBin , "LeftShiftOper" )
9191
PTNODE(knopRsh , ">>" , Shr_A , Bin , fnopBin , "RightShiftOper" )
9292
PTNODE(knopRs2 , ">>>" , ShrU_A , Bin , fnopBin , "UnsignedRightShiftOper" )
93-
PTNODE(knopNew , "new" , Nop , Call , fnopBin , "NewExpr" )
93+
PTNODE(knopNew , "new" , Nop , Call , fnopNone , "NewExpr" )
9494
PTNODE(knopIndex , "[]" , Nop , Bin , fnopBin , "IndexOper" )
95-
PTNODE(knopQmark , "?" , Nop , Tri , fnopBin , "IfExpr" )
95+
PTNODE(knopQmark , "?" , Nop , Tri , fnopNone , "IfExpr" )
9696

9797
// ___compact range : do not add or remove in this range.
9898
// Gen code of OP_LclAsg*,.. depends on parallel tables with this range
@@ -117,7 +117,7 @@ PTNODE(knopGetMember , "get" , Nop , Bin , fnopBin
117117
/***************************************************************************
118118
General nodes.
119119
***************************************************************************/
120-
PTNODE(knopList , "<list>" , Nop , Bin , fnopBinList|fnopNotExprStmt, "" )
120+
PTNODE(knopList , "<list>" , Nop , Bin , fnopBin|fnopNotExprStmt, "" )
121121
PTNODE(knopVarDecl , "varDcl" , Nop , Var , fnopNotExprStmt|fnopAllowDefer, "VarDecl" )
122122
PTNODE(knopConstDecl , "constDcl" , Nop , Var , fnopNotExprStmt|fnopAllowDefer, "ConstDecl" )
123123
PTNODE(knopLetDecl , "letDcl" , Nop , Var , fnopNotExprStmt|fnopAllowDefer, "LetDecl" )
@@ -151,9 +151,8 @@ PTNODE(knopTryFinally , "try-finally" , Nop , TryFinally , fnopNotExp
151151
PTNODE(knopObjectPattern, "{} = " , Nop , Uni , fnopUni , "ObjectAssignmentPattern" )
152152
PTNODE(knopObjectPatternMember, "{:} = " , Nop , Bin , fnopBin , "ObjectAssignmentPatternMember" )
153153
PTNODE(knopArrayPattern, "[] = " , Nop , ArrLit , fnopUni , "ArrayAssignmentPattern" )
154-
PTNODE(knopParamPattern, "({[]})" , Nop , ParamPattern, fnopUni , "DestructurePattern" )
154+
PTNODE(knopParamPattern, "({[]})" , Nop , ParamPattern, fnopNone , "DestructurePattern" )
155155
PTNODE(knopExportDefault, "export default" , Nop , ExportDefault,fnopNone , "ExportDefault" )
156-
PTNODE(knopSuperCall , "super call" , Nop , SuperCall , fnopBin , "SuperCall" )
157156

158157

159158
#undef PTNODE

lib/Parser/ptree.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ ParseNodeUni * ParseNode::AsParseNodeUni()
2525

2626
ParseNodeBin * ParseNode::AsParseNodeBin()
2727
{
28-
Assert(((this->Grfnop() & fnopBin) && this->nop != knopQmark && this->nop != knopCall && this->nop != knopNew) || this->nop == knopList);
28+
Assert(this->Grfnop() & fnopBin);
2929
return reinterpret_cast<ParseNodeBin *>(this);
3030
}
3131

lib/Parser/ptree.h

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,8 @@ const uint fnopContinue = 0x0080; // continue can be used within this statement
2424
const uint fnopCleanup = 0x0100; // requires cleanup (eg, with or for-in).
2525
const uint fnopJump = 0x0200;
2626
const uint fnopNotExprStmt = 0x0400;
27-
const uint fnopBinList = 0x0800;
2827
const uint fnopExprMask = (fnopLeaf|fnopUni|fnopBin);
29-
const uint fnopAllowDefer = 0x1000; // allow to be created during defer parse
28+
const uint fnopAllowDefer = 0x0800; // allow to be created during defer parse
3029

3130
/***************************************************************************
3231
Flags for classifying parse nodes.
@@ -66,7 +65,7 @@ enum PNodeFlags : ushort
6665
};
6766

6867
/***************************************************************************
69-
Data structs for ParseNodes.
68+
Data structs for ParseNodes.
7069
***************************************************************************/
7170
class ParseNodeUni;
7271
class ParseNodeBin;
@@ -371,7 +370,7 @@ class ParseNodeArrLit : public ParseNodeUni
371370
{
372371
public:
373372
ParseNodeArrLit(OpCode nop, charcount_t ichMin, charcount_t ichLim);
374-
373+
375374
uint count;
376375
uint spreadCount;
377376
BYTE arrayOfTaggedInts:1; // indicates that array initializer nodes are all tagged ints
@@ -833,9 +832,8 @@ class ParseNodeParamPattern : public ParseNode
833832
ParseNodeParamPattern(OpCode nop, charcount_t ichMin, charcount_t ichLim);
834833

835834
ParseNodePtr pnodeNext;
836-
Js::RegSlot location;
837835
ParseNodePtr pnode1;
838-
836+
Js::RegSlot location;
839837
DISABLE_SELF_CAST(ParseNodeParamPattern);
840838
};
841839

@@ -923,7 +921,7 @@ class ParseNodeReturn : public ParseNodeStmt
923921
DISABLE_SELF_CAST(ParseNodeReturn);
924922
};
925923

926-
// try-catch-finally
924+
// try-catch-finally
927925
class ParseNodeTryFinally : public ParseNodeStmt
928926
{
929927
public:

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -79,22 +79,19 @@ BOOL MayHaveSideEffectOnNode(ParseNode *pnode, ParseNode *pnodeSE)
7979
else if (fnop & fnopBin)
8080
{
8181
// pnodeSE is a binary (or ternary) op, so recurse to the sources (if present).
82-
if (pnodeSE->nop == knopQmark)
83-
{
84-
return MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeTri()->pnode1) ||
85-
MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeTri()->pnode2) ||
86-
MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeTri()->pnode3);
87-
}
88-
else if (pnodeSE->nop == knopCall || pnodeSE->nop == knopNew)
89-
{
90-
return MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeCall()->pnodeTarget) ||
91-
(pnodeSE->AsParseNodeCall()->pnodeArgs && MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeCall()->pnodeArgs));
92-
}
93-
else
94-
{
95-
return MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeBin()->pnode1) ||
96-
(pnodeSE->AsParseNodeBin()->pnode2 && MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeBin()->pnode2));
97-
}
82+
return MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeBin()->pnode1) ||
83+
(pnodeSE->AsParseNodeBin()->pnode2 && MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeBin()->pnode2));
84+
}
85+
else if (pnodeSE->nop == knopQmark)
86+
{
87+
return MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeTri()->pnode1) ||
88+
MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeTri()->pnode2) ||
89+
MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeTri()->pnode3);
90+
}
91+
else if (pnodeSE->nop == knopCall || pnodeSE->nop == knopNew)
92+
{
93+
return MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeCall()->pnodeTarget) ||
94+
(pnodeSE->AsParseNodeCall()->pnodeArgs && MayHaveSideEffectOnNode(pnode, pnodeSE->AsParseNodeCall()->pnodeArgs));
9895
}
9996
else if (pnodeSE->nop == knopList)
10097
{
@@ -9958,7 +9955,6 @@ void TrackIntConstantsOnGlobalObject(ByteCodeGenerator *byteCodeGenerator, Symbo
99589955
void TrackMemberNodesInObjectForIntConstants(ByteCodeGenerator *byteCodeGenerator, ParseNodePtr objNode)
99599956
{
99609957
Assert(objNode->nop == knopObject);
9961-
Assert(ParseNode::Grfnop(objNode->nop) & fnopUni);
99629958

99639959
ParseNodePtr memberList = objNode->AsParseNodeUni()->pnode1;
99649960

@@ -10020,7 +10016,7 @@ void TrackGlobalIntAssignments(ParseNodePtr pnode, ByteCodeGenerator * byteCodeG
1002010016
Assert(lhs && rhs);
1002110017

1002210018
// Don't track other than integers and objects with member nodes.
10023-
if (rhs->nop == knopObject && (ParseNode::Grfnop(rhs->nop) & fnopUni))
10019+
if (rhs->nop == knopObject)
1002410020
{
1002510021
TrackMemberNodesInObjectForIntConstants(byteCodeGenerator, rhs);
1002610022
}

lib/Runtime/Language/AsmJsUtils.h

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,29 +70,16 @@ namespace Js {
7070
static bool ParseVarOrConstStatement( AsmJSParser &parser, ParseNode **var );
7171
static inline bool IsNumericLiteral(ParseNode* node) { return node && (node->nop == knopInt || node->nop == knopFlt); }
7272
static inline bool IsFroundNumericLiteral(ParseNode* node) { return node && (IsNumericLiteral(node) || IsNegativeZero(node)); }
73-
static inline ParseNode* GetUnaryNode( ParseNode* node ){Assert(IsNodeUnary(node));return node->AsParseNodeUni()->pnode1;}
74-
static inline ParseNode* GetBinaryLeft( ParseNode* node ){Assert(IsNodeBinary(node));return node->AsParseNodeBin()->pnode1;}
75-
static inline ParseNode* GetBinaryRight( ParseNode* node ){Assert(IsNodeBinary(node));return node->AsParseNodeBin()->pnode2;}
73+
static inline ParseNode* GetUnaryNode( ParseNode* node ){return node->AsParseNodeUni()->pnode1;}
74+
static inline ParseNode* GetBinaryLeft( ParseNode* node ){return node->AsParseNodeBin()->pnode1;}
75+
static inline ParseNode* GetBinaryRight( ParseNode* node ){return node->AsParseNodeBin()->pnode2;}
7676
static inline ParseNode* DotBase( ParseNode *node );
7777
static inline bool IsDotMember( ParseNode *node );
7878
static inline PropertyName DotMember( ParseNode *node );
7979
// Get the VarDecl from the node or nullptr if unable to find
8080
static ParseNode* GetVarDeclList(ParseNode* node);
8181
// Goes through the nodes until the end of the list of VarDecl
8282
static void ReachEndVarDeclList( ParseNode** node );
83-
84-
// nop utils
85-
static inline bool IsNodeBinary (ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & (fnopBin|fnopBinList)); }
86-
static inline bool IsNodeUnary (ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopUni ); }
87-
static inline bool IsNodeConst (ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopConst ); }
88-
static inline bool IsNodeLeaf (ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopLeaf ); }
89-
static inline bool IsNodeRelational(ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopRel ); }
90-
static inline bool IsNodeAssignment(ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopAsg ); }
91-
static inline bool IsNodeBreak (ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopBreak ); }
92-
static inline bool IsNodeContinue (ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopContinue ); }
93-
static inline bool IsNodeCleanUp (ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopCleanup ); }
94-
static inline bool IsNodeJump (ParseNode* pnode){ return pnode && !!(ParseNode::Grfnop(pnode->nop) & fnopJump ); }
95-
static inline bool IsNodeExpression(ParseNode* pnode){ return pnode && !(ParseNode::Grfnop(pnode->nop) & fnopNotExprStmt); }
9683
};
9784

9885
bool ParserWrapper::IsNameDeclaration( ParseNode *node )

test/AsmJs/bug16252562.baseline

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
2+
bug16252562.js(9, 3)
3+
Asm.js Compilation Error function : AsmModule::foo
4+
Unhandled parse opcode for asm.js
5+
6+
Asm.js compilation failed.
7+
new.target: function () {console.log(`new.target: ${new.target}`);}

test/AsmJs/bug16252562.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
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 AsmModule(stdlib, foreign) {
7+
'use asm';
8+
var Bar = foreign.Bar;
9+
function foo() {
10+
return new Bar();
11+
}
12+
return foo;
13+
}
14+
AsmModule(this, {Bar() {console.log(`new.target: ${new.target}`);}})();

test/AsmJs/rlexe.xml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,4 +1068,11 @@
10681068
<compile-flags>-testtrace:asmjs -maic:1</compile-flags>
10691069
</default>
10701070
</test>
1071+
<test>
1072+
<default>
1073+
<files>bug16252562.js</files>
1074+
<baseline>bug16252562.baseline</baseline>
1075+
<compile-flags>-testtrace:asmjs</compile-flags>
1076+
</default>
1077+
</test>
10711078
</regress-exe>

0 commit comments

Comments
 (0)