[1.7>master] [MERGE #3979 @boingoing] Remove an assert in `FindOrAddPidRef`

Merge pull request #3979 from boingoing:RemoveFindOrAddPidRefAssert

We loosened this assert last week to handle cases of the PidRefStack
being out-of-order in the case of reparsing lambda parameters but there
are additional cases, apparently. Looks like we should just remove this
assert entirely.

lib/Parser/Hash.h

@@ -230,11 +230,7 @@ public:
         return prevRef;
     }
 
-    PidRefStack * FindOrAddPidRef(ArenaAllocator *alloc, int scopeId, Js::LocalFunctionId funcId
-#if DBG
-        , bool isReparseLambdaParams = false
-#endif
-    )
+    PidRefStack * FindOrAddPidRef(ArenaAllocator *alloc, int scopeId, Js::LocalFunctionId funcId)
     {
         // If the stack is empty, or we are pushing to the innermost scope already,
         // we can go ahead and push a new PidRef on the stack.
@@ -296,7 +292,6 @@ public:
                 return newRef;
             }
 
-            Assert(ref->prev->id <= ref->id || isReparseLambdaParams);
             prevRef = ref;
             ref = ref->prev;
         }

lib/Parser/Parse.cpp

@@ -9115,11 +9115,7 @@ PidRefStack* Parser::PushPidRef(IdentPtr pid)
         // NOTE: the phase check is here to protect perf. See OSG 1020424.
         // In some LS AST-rewrite cases we lose a lot of perf searching the PID ref stack rather
         // than just pushing on the top. This hasn't shown up as a perf issue in non-LS benchmarks.
-        return pid->FindOrAddPidRef(&m_nodeAllocator, GetCurrentBlock()->sxBlock.blockId, GetCurrentFunctionNode()->sxFnc.functionId
-#if DBG
-            , this->m_reparsingLambdaParams
-#endif
-        );
+        return pid->FindOrAddPidRef(&m_nodeAllocator, GetCurrentBlock()->sxBlock.blockId, GetCurrentFunctionNode()->sxFnc.functionId);
     }
 
     Assert(GetCurrentBlock() != nullptr);
@@ -9150,11 +9146,7 @@ PidRefStack* Parser::PushPidRef(IdentPtr pid)
 
 PidRefStack* Parser::FindOrAddPidRef(IdentPtr pid, int scopeId, Js::LocalFunctionId funcId)
 {
-    PidRefStack *ref = pid->FindOrAddPidRef(&m_nodeAllocator, scopeId, funcId
-#if DBG
-        , this->m_reparsingLambdaParams
-#endif
-    );
+    PidRefStack *ref = pid->FindOrAddPidRef(&m_nodeAllocator, scopeId, funcId);
     if (ref == NULL)
     {
         Error(ERRnoMemory);

test/Basics/SpecialSymbolCapture.js

@@ -889,6 +889,7 @@ var tests = [
         body: function() {
             // Failure causes an assert to fire
             WScript.LoadScript(`(a = function() { this }, b = (this)) => {}`);
+            assert.throws(() => WScript.LoadScript(`[ a = function () { this; } ((this)) = 1 ] = []`), ReferenceError, "Not a valid destructuring assignment but should not fire assert", "Invalid left-hand side in assignment");
         }
     },
     {