Make ParseNodeParamPattern a real ParseNodeUni and thus the down cast is valid.

Fix walking the ast with ParamPattern in pnodewalk
Do a dummy walk of the ast in ByteCode Generator to do some basic validation of our walker
Give up on knopList being a fnopBin as it has side-effects in ByteCodeEmitter::MayHaveSideEffectOnNode and ByteCodeGenerator::AssignRegisters

Author: Cellule

lib/Parser/Parse.cpp

@@ -1043,7 +1043,6 @@ ParseNodeSuperCall * Parser::CreateSuperCallNode(ParseNodeSpecialName * pnode1,
     Assert(!this->m_deferringAST);
     Assert(pnode1 && pnode1->isSuper);
 
-    DebugOnly(VerifyNodeSize(knopSuperCall, sizeof(ParseNodeSuperCall)));
     ParseNodeSuperCall* pnode = Anew(&m_nodeAllocator, ParseNodeSuperCall, knopCall, pnode1->ichMin, pnode2 == nullptr ? pnode1->ichLim : pnode2->ichLim, pnode1, pnode2);
     AddAstSize(sizeof(ParseNodeSuperCall));
     return pnode;
@@ -13266,7 +13265,7 @@ void PrintPnodeWIndent(ParseNode *pnode, int indentAmt) {
         //PTNODE(knopMod        , "%"            ,Mod     ,Bin  ,fnopBin)
     case knopMod:
         Indent(indentAmt);
-        Output::Print(_u("%\n"));
+        Output::Print(_u("%%\n"));
         PrintPnodeWIndent(pnode->AsParseNodeBin()->pnode1, indentAmt + INDENT_SIZE);
         PrintPnodeWIndent(pnode->AsParseNodeBin()->pnode2, indentAmt + INDENT_SIZE);
         break;

lib/Parser/pnodewalk.h

@@ -173,10 +173,19 @@ class ParseNodeWalker : public WalkerPolicy
     {
         ResultType result;
         // For ordering, arguments are considered prior to the function and the body after.
-        for (ParseNodePtr* argNode = &(pnode->pnodeParams); *argNode != nullptr; argNode = &((*argNode)->AsParseNodeVar()->pnodeNext))
+        ParseNodePtr argNode = pnode->pnodeParams;
+        while (argNode)
         {
-            result = *argNode == pnode->pnodeParams ? WalkFirstChild(*argNode, context) : WalkNthChild(pnode, *argNode, context);
+            result = argNode == pnode->pnodeParams ? WalkFirstChild(argNode, context) : WalkNthChild(pnode, argNode, context);
             if (!ContinueWalk(result)) return result;
+            if (argNode->nop == knopParamPattern)
+            {
+                argNode = argNode->AsParseNodeParamPattern()->pnodeNext;
+            }
+            else
+            {
+                argNode = argNode->AsParseNodeVar()->pnodeNext;
+            }
         }
 
         if (pnode->pnodeRest != nullptr)

lib/Parser/ptlist.h

@@ -117,7 +117,7 @@ PTNODE(knopGetMember  , "get"              , Nop      , Bin         , fnopBin
 /***************************************************************************
 General nodes.
 ***************************************************************************/
-PTNODE(knopList       , "<list>"           , Nop      , Bin         , fnopBin|fnopNotExprStmt, ""                          )
+PTNODE(knopList       , "<list>"           , Nop      , Bin         , fnopBinList|fnopNotExprStmt, ""                          )
 PTNODE(knopVarDecl    , "varDcl"           , Nop      , Var         , fnopNotExprStmt|fnopAllowDefer, "VarDecl"                       )
 PTNODE(knopConstDecl  , "constDcl"         , Nop      , Var         , fnopNotExprStmt|fnopAllowDefer, "ConstDecl"                     )
 PTNODE(knopLetDecl    , "letDcl"           , Nop      , Var         , fnopNotExprStmt|fnopAllowDefer, "LetDecl"                       )
@@ -151,7 +151,7 @@ PTNODE(knopTryFinally , "try-finally"      , Nop      , TryFinally  , fnopNotExp
 PTNODE(knopObjectPattern, "{} = "          , Nop      , Uni         , fnopUni                , "ObjectAssignmentPattern"       )
 PTNODE(knopObjectPatternMember, "{:} = "   , Nop      , Bin         , fnopBin                , "ObjectAssignmentPatternMember" )
 PTNODE(knopArrayPattern, "[] = "           , Nop      , ArrLit      , fnopUni                , "ArrayAssignmentPattern"        )
-PTNODE(knopParamPattern, "({[]})"          , Nop      , ParamPattern, fnopNone               , "DestructurePattern"            )
+PTNODE(knopParamPattern, "({[]})"          , Nop      , ParamPattern, fnopUni                , "DestructurePattern"            )
 PTNODE(knopExportDefault, "export default" , Nop      , ExportDefault,fnopNone               , "ExportDefault"                 )
 
 

lib/Parser/ptree.cpp

@@ -19,13 +19,13 @@ ParseNode::ParseNode(OpCode nop, charcount_t ichMin, charcount_t ichLim)
 
 ParseNodeUni * ParseNode::AsParseNodeUni()
 {
-    Assert(((this->Grfnop() & fnopUni) && this->nop != knopParamPattern) || this->nop == knopThrow);
+    Assert((this->Grfnop() & fnopUni) || this->nop == knopThrow);
     return reinterpret_cast<ParseNodeUni *>(this);
 }
 
 ParseNodeBin * ParseNode::AsParseNodeBin()
 {
-    Assert(this->Grfnop() & fnopBin);
+    Assert((this->Grfnop() & fnopBin) || this->nop == knopList);
     return reinterpret_cast<ParseNodeBin *>(this);
 }
 
@@ -485,7 +485,7 @@ ParseNodeWith::ParseNodeWith(OpCode nop, charcount_t ichMin, charcount_t ichLim)
 }
 
 ParseNodeParamPattern::ParseNodeParamPattern(OpCode nop, charcount_t ichMin, charcount_t ichLim)
-    : ParseNode(nop, ichMin, ichLim)
+    : ParseNodeUni(nop, ichMin, ichLim, nullptr)
 {
 }
 

lib/Parser/ptree.h

@@ -24,8 +24,9 @@ const uint fnopContinue = 0x0080; // continue can be used within this statement
 const uint fnopCleanup  = 0x0100; // requires cleanup (eg, with or for-in).
 const uint fnopJump     = 0x0200;
 const uint fnopNotExprStmt = 0x0400;
+const uint fnopBinList  = 0x0800;
 const uint fnopExprMask = (fnopLeaf|fnopUni|fnopBin);
-const uint fnopAllowDefer = 0x0800; // allow to be created during defer parse
+const uint fnopAllowDefer = 0x1000; // allow to be created during defer parse
 
 /***************************************************************************
 Flags for classifying parse nodes.
@@ -826,13 +827,12 @@ class ParseNodeWith : public ParseNodeStmt
 };
 
 // Destructure pattern for function/catch parameter
-class ParseNodeParamPattern : public ParseNode
+class ParseNodeParamPattern : public ParseNodeUni
 {
 public:
     ParseNodeParamPattern(OpCode nop, charcount_t ichMin, charcount_t ichLim);
 
     ParseNodePtr pnodeNext;
-    ParseNodePtr pnode1;
     Js::RegSlot location;
     DISABLE_SELF_CAST(ParseNodeParamPattern);
 };

lib/Runtime/ByteCode/ByteCodeGenerator.cpp

@@ -6,6 +6,10 @@
 #include "FormalsUtil.h"
 #include "Library/StackScriptFunction.h"
 
+#if DBG
+#include "pnodewalk.h"
+#endif
+
 void PreVisitBlock(ParseNodeBlock *pnodeBlock, ByteCodeGenerator *byteCodeGenerator);
 void PostVisitBlock(ParseNodeBlock *pnodeBlock, ByteCodeGenerator *byteCodeGenerator);
 
@@ -1941,6 +1945,16 @@ void ByteCodeGenerator::Generate(__in ParseNode *pnode, uint32 grfscr, __in Byte
     __inout Js::ParseableFunctionInfo ** ppRootFunc, __in uint sourceIndex,
     __in bool forceNoNative, __in Parser* parser, Js::ScriptFunction **functionRef)
 {
+#if DBG
+    struct WalkerPolicyTest : public WalkerPolicyBase<bool, ParseNodeWalker<WalkerPolicyTest>*>
+    {
+        inline bool ContinueWalk(ResultType) { return ThreadContext::IsCurrentStackAvailable(Js::Constants::MinStackByteCodeVisitor); }
+        virtual ResultType WalkChild(ParseNode *pnode, ParseNodeWalker<WalkerPolicyTest>* walker) { return ContinueWalk(true) && walker->Walk(pnode, walker); }
+    };
+    ParseNodeWalker<WalkerPolicyTest> walker;
+    // Just walk the ast to see if our walker encounters any problems
+    walker.Walk(pnode, &walker);
+#endif
     Js::ScriptContext * scriptContext = byteCodeGenerator->scriptContext;
 
 #ifdef PROFILE_EXEC