[Merge chakra-core/ChakraCore@345870f035] [MERGE #3979 @boingoing] Re…

…move 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.
nodejs · Jan 9, 2018 · 4749007 · 4749007
1 parent 599be2f
commit 4749007
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 16 deletions.
diff --git a/deps/chakrashim/core/lib/Parser/Hash.h b/deps/chakrashim/core/lib/Parser/Hash.h
@@ -243,11 +243,7 @@ struct Ident
         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.
@@ -309,7 +305,6 @@ struct Ident
                 return newRef;
             }
 
-            Assert(ref->prev->id <= ref->id || isReparseLambdaParams);
             prevRef = ref;
             ref = ref->prev;
         }

diff --git a/deps/chakrashim/core/lib/Parser/Parse.cpp b/deps/chakrashim/core/lib/Parser/Parse.cpp
@@ -9057,11 +9057,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);
@@ -9092,11 +9088,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);

diff --git a/deps/chakrashim/core/test/Basics/SpecialSymbolCapture.js b/deps/chakrashim/core/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");
         }
     },
     {