From 160fca6579ee3763192504bfb6125ff8f55a473c Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Wed, 14 Jan 2015 21:09:39 -0800
Subject: [PATCH] Refactor avoidance of redundant repeat rules

---
 spec/compiler/rules/repeat_spec.cc           | 22 ++++++++++++++++++++
 src/compiler/build_tables/build_lex_table.cc | 15 +++----------
 src/compiler/prepare_grammar/parse_regex.cc  |  4 ++--
 src/compiler/rules/repeat.cc                 | 13 +++++++++++-
 src/compiler/rules/repeat.h                  |  1 +
 src/compiler/rules/rules.cc                  |  2 +-
 src/compiler/rules/visitor.cc                |  2 +-
 7 files changed, 42 insertions(+), 17 deletions(-)
 create mode 100644 spec/compiler/rules/repeat_spec.cc

diff --git a/spec/compiler/rules/repeat_spec.cc b/spec/compiler/rules/repeat_spec.cc
new file mode 100644
index 00000000..837d4453
--- /dev/null
+++ b/spec/compiler/rules/repeat_spec.cc
@@ -0,0 +1,22 @@
+#include "compiler/compiler_spec_helper.h"
+#include "compiler/rules/repeat.h"
+#include "compiler/rules/symbol.h"
+
+using namespace rules;
+
+START_TEST
+
+describe("Repeat", []() {
+  describe("constructing repeats", [&]() {
+    it("doesn't create redundant repeats", [&]() {
+      auto sym = make_shared<Symbol>(1);
+      auto repeat = Repeat::build(sym);
+      auto outer_repeat = Repeat::build(repeat);
+
+      AssertThat(repeat, !Equals(sym));
+      AssertThat(outer_repeat, Equals(repeat));
+    });
+  });
+});
+
+END_TEST
diff --git a/src/compiler/build_tables/build_lex_table.cc b/src/compiler/build_tables/build_lex_table.cc
index ac2155e1..52b350cb 100644
--- a/src/compiler/build_tables/build_lex_table.cc
+++ b/src/compiler/build_tables/build_lex_table.cc
@@ -128,18 +128,9 @@ class LexTableBuilder {
 
   rules::rule_ptr separator_rule() const {
     vector<rules::rule_ptr> separators;
-    for (const auto &rule : lex_grammar.separators) {
-
-      // TODO - remove this hack. right now, nested repeats cause
-      // item sets which are equivalent to appear unequal.
-      auto repeat = dynamic_pointer_cast<const rules::Repeat>(rule);
-      if (repeat.get())
-        separators.push_back(repeat->content);
-      else
-        separators.push_back(rule);
-    }
-
-    return rules::repeat(rules::choice(separators));
+    for (const auto &rule : lex_grammar.separators)
+      separators.push_back(rules::repeat(rule));
+    return rules::choice(separators);
   }
 
   set<int> precedence_values_for_item_set(const LexItemSet &item_set) const {
diff --git a/src/compiler/prepare_grammar/parse_regex.cc b/src/compiler/prepare_grammar/parse_regex.cc
index d71e900c..49e23ad7 100644
--- a/src/compiler/prepare_grammar/parse_regex.cc
+++ b/src/compiler/prepare_grammar/parse_regex.cc
@@ -78,11 +78,11 @@ class PatternParser {
       switch (peek()) {
         case '*':
           next();
-          result = make_shared<Repeat>(result);
+          result = Repeat::build(result);
           break;
         case '+':
           next();
-          result = make_shared<Seq>(result, make_shared<Repeat>(result));
+          result = make_shared<Seq>(result, Repeat::build(result));
           break;
         case '?':
           next();
diff --git a/src/compiler/rules/repeat.cc b/src/compiler/rules/repeat.cc
index cf833ac6..71fa32df 100644
--- a/src/compiler/rules/repeat.cc
+++ b/src/compiler/rules/repeat.cc
@@ -1,14 +1,25 @@
 #include "compiler/rules/repeat.h"
+#include <memory>
 #include <string>
 #include "compiler/rules/visitor.h"
 
 namespace tree_sitter {
 namespace rules {
 
+using std::dynamic_pointer_cast;
+using std::make_shared;
 using std::string;
 
 Repeat::Repeat(const rule_ptr content) : content(content) {}
 
+rule_ptr Repeat::build(const rule_ptr &rule) {
+  auto inner_repeat = dynamic_pointer_cast<Repeat>(rule);
+  if (inner_repeat.get())
+    return inner_repeat;
+  else
+    return make_shared<Repeat>(rule);
+}
+
 bool Repeat::operator==(const Rule &rule) const {
   const Repeat *other = dynamic_cast<const Repeat *>(&rule);
   return other && (*other->content == *content);
@@ -16,7 +27,7 @@ bool Repeat::operator==(const Rule &rule) const {
 
 size_t Repeat::hash_code() const { return content->hash_code(); }
 
-rule_ptr Repeat::copy() const { return std::make_shared<Repeat>(*this); }
+rule_ptr Repeat::copy() const { return make_shared<Repeat>(*this); }
 
 string Repeat::to_string() const {
   return string("(repeat ") + content->to_string() + ")";
diff --git a/src/compiler/rules/repeat.h b/src/compiler/rules/repeat.h
index a0f97bd0..fd0862ea 100644
--- a/src/compiler/rules/repeat.h
+++ b/src/compiler/rules/repeat.h
@@ -10,6 +10,7 @@ namespace rules {
 class Repeat : public Rule {
  public:
   explicit Repeat(rule_ptr content);
+  static rule_ptr build(const rule_ptr &rule);
 
   bool operator==(const Rule &other) const;
   size_t hash_code() const;
diff --git a/src/compiler/rules/rules.cc b/src/compiler/rules/rules.cc
index d0970846..20840448 100644
--- a/src/compiler/rules/rules.cc
+++ b/src/compiler/rules/rules.cc
@@ -35,7 +35,7 @@ rule_ptr blank() { return make_shared<Blank>(); }
 rule_ptr choice(const vector<rule_ptr> &rules) { return Choice::build(rules); }
 
 rule_ptr repeat(const rule_ptr &content) {
-  return std::make_shared<Repeat>(content);
+  return Repeat::build(content);
 }
 
 rule_ptr seq(const vector<rule_ptr> &rules) { return Seq::build(rules); }
diff --git a/src/compiler/rules/visitor.cc b/src/compiler/rules/visitor.cc
index da2e4c82..2df87033 100644
--- a/src/compiler/rules/visitor.cc
+++ b/src/compiler/rules/visitor.cc
@@ -33,7 +33,7 @@ rule_ptr IdentityRuleFn::apply_to(const Seq *rule) {
 }
 
 rule_ptr IdentityRuleFn::apply_to(const Repeat *rule) {
-  return std::make_shared<Repeat>(apply(rule->content));
+  return Repeat::build(apply(rule->content));
 }
 
 rule_ptr IdentityRuleFn::apply_to(const Metadata *rule) {