Fixes side effects when backtracking in a CSTParser.

Fixes #789
Chevrotain · Sep 12, 2019 · 1dc658e · 1dc658e
1 parent 10de5d2
commit 1dc658e
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 7 deletions.
diff --git a/packages/chevrotain/docs/changes/CHANGELOG.md b/packages/chevrotain/docs/changes/CHANGELOG.md
@@ -4,6 +4,10 @@
 
 -   [Improve duplicate DSL methods suffix error message](https://github.com/SAP/chevrotain/issues/1020)
 
+#### Bug Fixes
+
+-   [Unexpected Side Effects When backtracking in a CstParser](https://github.com/SAP/chevrotain/issues/789)
+
 ## 6.3.1 (9-10-2019)
 
 #### Bug Fixes

diff --git a/packages/chevrotain/src/parse/parser/traits/recognizer_engine.ts b/packages/chevrotain/src/parse/parser/traits/recognizer_engine.ts
@@ -1001,7 +1001,7 @@ export class RecognizerEngine {
     }
 
     isBackTracking(this: MixedInParser): boolean {
-        return !isEmpty(this.isBackTrackingStack)
+        return this.isBackTrackingStack.length !== 0
     }
 
     getCurrRuleFullName(this: MixedInParser): string {

diff --git a/packages/chevrotain/src/parse/parser/traits/tree_builder.ts b/packages/chevrotain/src/parse/parser/traits/tree_builder.ts
@@ -265,12 +265,19 @@ export class TreeBuilder {
         ruleCstResult: CstNode,
         ruleName: string
     ): void {
-        const node = this.CST_STACK[this.CST_STACK.length - 1]
-
-        addNoneTerminalToCst(node, ruleName, ruleCstResult)
-
-        // This is only used when **both** error recovery and CST Output are enabled.
-        this.setNodeLocationFromNode(node.location, ruleCstResult.location)
+        // Avoid side effects due to back tracking
+        // TODO: This costs a 2-3% in performance, A flag on IParserConfig
+        //   could be used to get rid of this conditional, but not sure its worth the effort
+        //   and API complexity.
+        if (this.isBackTracking() !== true) {
+            const preCstNode = this.CST_STACK[this.CST_STACK.length - 1]
+            addNoneTerminalToCst(preCstNode, ruleName, ruleCstResult)
+            // This is only used when **both** error recovery and CST Output are enabled.
+            this.setNodeLocationFromNode(
+                preCstNode.location,
+                ruleCstResult.location
+            )
+        }
     }
 
     getBaseCstVisitorConstructor(