[MERGE #5400 @akroshg] Fix for destructuring declaration completion result.

Merge pull request #5400 from akroshg:fix5196

The detstructuring declaration node is a normal assignment node, so it was returing the RHS as completion result. In order to fix that marking that node as Destructuring declaration node
and check that before emitting the return bytecode.

Author: akroshg

lib/Parser/Parse.cpp

@@ -9156,6 +9156,7 @@ ParseNodePtr Parser::ParseVariableDeclaration(
             if (pnodeThis != nullptr)
             {
                 pnodeThis->ichMin = ichMin;
+                pnodeThis->SetIsPatternDeclaration();
             }
         }
         else

lib/Parser/ptree.cpp

@@ -12,6 +12,7 @@ ParseNode::ParseNode(OpCode nop, charcount_t ichMin, charcount_t ichLim)
     this->isUsed = true;
     this->notEscapedUse = false;
     this->isInList = false;
+    this->isPatternDeclaration = false;
     this->isCallApplyTargetLoad = false;
     this->ichMin = ichMin;
     this->ichLim = ichLim;

lib/Parser/ptree.h

@@ -210,6 +210,9 @@ class ParseNode
     bool IsCallApplyTargetLoad() { return isCallApplyTargetLoad; }
     void SetIsCallApplyTargetLoad() { isCallApplyTargetLoad = true; }    
 
+    bool IsPatternDeclaration() { return isPatternDeclaration; }
+    void SetIsPatternDeclaration() { isPatternDeclaration = true; }
+
     bool IsUserIdentifier();
 
     bool IsVarLetOrConst() const
@@ -241,6 +244,9 @@ class ParseNode
     bool notEscapedUse : 1;         // Currently, only used by child of knopComma
     bool isCallApplyTargetLoad : 1;
 
+    // Use by bytecodegen to identify the current node is a destructuring pattern declaration node.
+    bool isPatternDeclaration : 1;
+
 public:
     ushort grfpn;
 

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

@@ -2438,7 +2438,7 @@ void ByteCodeGenerator::EmitGlobalBody(FuncInfo *funcInfo)
     // return value.
     ParseNode *pnode = funcInfo->root->pnodeBody;
     ParseNode *pnodeLastVal = funcInfo->root->AsParseNodeProg()->pnodeLastValStmt;
-    if (pnodeLastVal == nullptr)
+    if (pnodeLastVal == nullptr || pnodeLastVal->IsPatternDeclaration())
     {
         // We're not guaranteed to compute any values, so fix up the return register at the top
         // in case.
@@ -11716,7 +11716,7 @@ void Emit(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator, FuncInfo *func
         break;
     }
 
-    if (fReturnValue && IsExpressionStatement(pnode, byteCodeGenerator->GetScriptContext()))
+    if (fReturnValue && IsExpressionStatement(pnode, byteCodeGenerator->GetScriptContext()) && !pnode->IsPatternDeclaration())
     {
         // If this statement may produce the global function's return value, copy its result to the return register.
         // fReturnValue implies global function, which implies that "return" is a parse error.

test/Bugs/misc_bugs.js

@@ -154,7 +154,17 @@ var tests = [
 
         var obj2 = {__proto__ : p}; // This should not call the getPrototypeOf
     }
-  }
+  },
+  {
+    name: "Destructuring declaration should return undefined",
+    body: function () {
+        assert.areEqual(undefined, eval("var {x} = {};"));
+        assert.areEqual(undefined, eval("let {x,y} = {};"));
+        assert.areEqual(undefined, eval("const [z] = [];"));
+        assert.areEqual(undefined, eval("let {x} = {}, y = 1, {z} = {};"));
+        assert.areEqual([1], eval("let {x} = {}; [x] = [1]"));
+    }
+  },
 
 ];