From cdeecf750f8a3f781459ed817815a537392993c1 Mon Sep 17 00:00:00 2001 From: David Majda Date: Fri, 7 Aug 2015 14:34:19 +0200 Subject: [PATCH] reportLeftRecursion: Change handling of the |visitedRules| array Before this commit, the |reportLeftRecursion| pass was written in functional style, passing the |visitedRules| array around as a parameter and making a new copy each time a rule was visited. This apparently caused performance problems in some deeply recursive grammars. This commit makes it so that there is just one array which is shared across all the visitor functions via a closure and modified as rules are visited. I don't like losing the functional style (it was elegant) but performance is more important. Fixes #203. --- lib/compiler/passes/report-left-recursion.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/compiler/passes/report-left-recursion.js b/lib/compiler/passes/report-left-recursion.js index 4bb12dc..8decd1b 100644 --- a/lib/compiler/passes/report-left-recursion.js +++ b/lib/compiler/passes/report-left-recursion.js @@ -18,22 +18,26 @@ var arrays = require("../../utils/arrays"), * it can lead to left recursion. */ function reportLeftRecursion(ast) { + var visitedRules = []; + var check = visitor.build({ - rule: function(node, visitedRules) { - check(node.expression, visitedRules.concat(node.name)); + rule: function(node) { + visitedRules.push(node.name); + check(node.expression); + visitedRules.pop(node.name); }, - sequence: function(node, visitedRules) { + sequence: function(node) { arrays.every(node.elements, function(element) { if (element.type === "rule_ref") { - check(element, visitedRules); + check(element); } return !asts.alwaysAdvancesOnSuccess(ast, element); }); }, - rule_ref: function(node, visitedRules) { + rule_ref: function(node) { if (arrays.contains(visitedRules, node.name)) { throw new GrammarError( "Left recursion detected for rule \"" + node.name + "\".", @@ -41,11 +45,11 @@ function reportLeftRecursion(ast) { ); } - check(asts.findRule(ast, node.name), visitedRules); + check(asts.findRule(ast, node.name)); } }); - check(ast, []); + check(ast); } module.exports = reportLeftRecursion;