bugfix and performance improvement (#48)

* add bugfix and tests

* update version

CMakeLists.txt

@@ -3,7 +3,7 @@ cmake_minimum_required(VERSION 3.14 FATAL_ERROR)
 # ---- Project ----
 
 project(LarsParser 
-  VERSION 1.9
+  VERSION 1.10
   LANGUAGES CXX
 )
 

README.md

@@ -49,7 +49,7 @@ With [CPM](https://github.com/TheLartians/CPM), lars::parser can be added to you
 ```cmake
 CPMAddPackage(
   NAME LarsParser
-  VERSION 1.9
+  VERSION 1.10
   GIT_REPOSITORY https://github.com/TheLartians/Parser.git
 )
 

source/parser.cpp

@@ -58,7 +58,8 @@ namespace {
   private:
     size_t position;
     using CacheKey = std::tuple<size_t,peg::Rule*>;
-    std::unordered_map<CacheKey, std::shared_ptr<SyntaxTree>, lars::TupleHasher<CacheKey>> cache;
+    using Cache = std::unordered_map<CacheKey, std::shared_ptr<SyntaxTree>, lars::TupleHasher<CacheKey>>;
+    Cache cache;
     std::shared_ptr<SyntaxTree> errorTree;
 
   public:
@@ -120,6 +121,10 @@ namespace {
     void addToCache(const std::shared_ptr<SyntaxTree> &tree) {
       cache[std::make_pair(tree->begin, tree->rule.get())] = tree;
     }
+
+    const Cache &getCache(){
+      return cache;
+    }
 
     void removeFromCache(const std::shared_ptr<SyntaxTree> &tree){
       auto it = cache.find(std::make_pair(tree->begin, tree->rule.get()));
@@ -156,8 +161,8 @@ namespace {
   bool parse(const std::shared_ptr<peg::GrammarNode> &node, State &state);
 
   std::shared_ptr<SyntaxTree> parseRule(const std::shared_ptr<peg::Rule> &rule, State &state, bool useCache = true) {
-    INCREASE_INDENT;
     PARSER_TRACE("enter rule " << rule->name);
+    INCREASE_INDENT;
 
     if (useCache && rule->cacheable) {
       auto cached = state.getCached(rule);
@@ -190,6 +195,7 @@ namespace {
     auto saved = state.save();
     state.stack.push_back(syntaxTree);
     syntaxTree->valid = parse(rule->node, state);
+    syntaxTree->end = state.getPosition();
     syntaxTree->active = false;
     state.stack.pop_back();
 
@@ -200,6 +206,13 @@ namespace {
         while (true) {
           State recursionState(state.string, syntaxTree->begin);
           recursionState.trackError(state.getErrorTree());
+          // Copy the cache except the currect position to the recursion state
+          // TODO: keeping the current state and modifying the cache in place is probably much more efficient.
+          for (auto &cached: state.getCache()) {
+            if (std::get<0>(cached.first) != syntaxTree->begin) {
+              recursionState.addToCache(cached.second);
+            }
+          }
           recursionState.addToCache(syntaxTree);
           auto tmp = parseRule(rule, recursionState, false);
           state.trackError(recursionState.getErrorTree());
@@ -218,7 +231,6 @@ namespace {
       }
 
       state.addInnerSyntaxTree(syntaxTree);
-      syntaxTree->end = state.getPosition();
     } else {
       state.trackError(syntaxTree);
       state.load(saved);

tests/parser.cpp

@@ -140,21 +140,33 @@ TEST_CASE("Evaluation"){
   REQUIRE(calculator.run("  1 + 2*3*1 +4 * 5  ") == 27);
   REQUIRE_THROWS(calculator.run("1+2*"));
 }
-
+#include <iostream>
 TEST_CASE("Left recursion"){
   ParserGenerator<float> calculator;
   calculator.setSeparatorRule("Whitespace", "[\t ]");
-  calculator.setStart(calculator.setRule("Expression", "Sum | Number"));
+  calculator.setStart(calculator.setRule("Expression", "Sum | Atomic"));
   calculator.setRule("Sum", "Addition | Product");
-  calculator.setRule("Addition", "Sum '+' Product", [](auto e){ return e[0].evaluate() + e[1].evaluate(); });
-  calculator.setRule("Product", "Multiplication | Number");
-  calculator.setRule("Multiplication", "Product '*' Number", [](auto e){ return e[0].evaluate() * e[1].evaluate(); });
+  calculator.setRule("NegativeSummand", "'-' Product", [](auto e){ return -e[0].evaluate(); });
+  calculator.setRule("Addition", "Sum ('+' Product | NegativeSummand)", [](auto e){ return e[0].evaluate() + e[1].evaluate(); });
+  calculator.setRule("Product", "Multiplication | Atomic");
+  calculator.setRule("Multiplication", "Product '*' Atomic", [](auto e){ return e[0].evaluate() * e[1].evaluate(); });
+  calculator.setRule("Atomic", "Number | Negative | Brackets");
   calculator.setProgramRule("Number", peg::createFloatProgram());
+  calculator.setRule("Negative", "'-' Atomic", [](auto e){ return -e[0].evaluate(); });
+  calculator.setRule("Brackets", "'(' Expression ')'");
+
   REQUIRE(calculator.run("42") == 42);
   REQUIRE(calculator.run("1+2") == 3);
+  REQUIRE(calculator.run("1+2-3-5") == -5);
   REQUIRE(calculator.run("2 * 3") == 6);
   REQUIRE(calculator.run("1 + 2*3") == 7);
   REQUIRE(calculator.run("  1 + 2*3*1 +4 * 5  ") == 27);
+
+  REQUIRE(calculator.run("-42") == -42);
+  REQUIRE(calculator.run("--42") == 42);
+  REQUIRE(calculator.run("---42") == -42);
+  REQUIRE(calculator.run("----------------------------------------------------42") == 42);
+
   REQUIRE_THROWS(calculator.run("1+2*"));
 }