From 1da9f1fdfde2cb051c46cd648759c34cf15bc9a6 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 7 Apr 2014 08:50:00 -0700 Subject: [PATCH] Store rule metadata as a map, not a single number Need to store more than just boolean values --- .../build_tables/check_metadata_spec.cc | 60 -------------- spec/compiler/build_tables/first_set_spec.cc | 2 +- .../build_tables/get_metadata_spec.cc | 81 +++++++++++++++++++ .../build_tables/rule_can_be_blank_spec.cc | 4 +- src/compiler/build_tables/build_tables.cc | 5 +- .../{check_metadata.cc => get_metadata.cc} | 23 +++--- .../{check_metadata.h => get_metadata.h} | 2 +- src/compiler/build_tables/item.cc | 6 +- src/compiler/build_tables/item.h | 2 +- src/compiler/build_tables/item_set_closure.cc | 2 +- src/compiler/rules/metadata.cc | 10 ++- src/compiler/rules/metadata.h | 11 +-- 12 files changed, 120 insertions(+), 88 deletions(-) delete mode 100644 spec/compiler/build_tables/check_metadata_spec.cc create mode 100644 spec/compiler/build_tables/get_metadata_spec.cc rename src/compiler/build_tables/{check_metadata.cc => get_metadata.cc} (51%) rename src/compiler/build_tables/{check_metadata.h => get_metadata.h} (76%) diff --git a/spec/compiler/build_tables/check_metadata_spec.cc b/spec/compiler/build_tables/check_metadata_spec.cc deleted file mode 100644 index 060fcf9f..00000000 --- a/spec/compiler/build_tables/check_metadata_spec.cc +++ /dev/null @@ -1,60 +0,0 @@ -#include "compiler_spec_helper.h" -#include "compiler/rules/metadata.h" -#include "compiler/build_tables/check_metadata.h" - -using namespace rules; -using namespace build_tables; - -START_TEST - -describe("checking if rules have metadata", []() { - MetadataValue value = MetadataValue(1 << 3); - - it("returns true for a compatible metadata rule", [&]() { - auto rule = make_shared(sym("x"), MetadataValue(value | 1)); - AssertThat(check_metadata(rule, value), IsTrue()); - }); - - it("returns false for an incompatible metadata rule", [&]() { - auto rule = make_shared(sym("x"), MetadataValue(1 << 2)); - AssertThat(check_metadata(rule, value), IsFalse()); - }); - - it("returns false for a non-metadata rule", [&]() { - auto rule = sym("x"); - AssertThat(check_metadata(rule, value), IsFalse()); - }); - - it("returns true for a compatible metadata rule preceded by rules that can be blank", [&]() { - auto rule = seq({ - repeat(sym("x")), - make_shared(sym("x"), MetadataValue(value | 1)), - }); - - AssertThat(check_metadata(rule, value), IsTrue()); - }); - - it("returns true for a choice including a compatible metadata rule", [&]() { - auto rule = choice({ - sym("x"), - make_shared(sym("x"), MetadataValue(value | 1)), - }); - - AssertThat(check_metadata(rule, value), IsTrue()); - }); - - it("returns true for a repetition containing a compatible metadata rule", [&]() { - auto rule = repeat(make_shared(sym("x"), MetadataValue(value | 1))); - AssertThat(check_metadata(rule, value), IsTrue()); - }); - - it("returns true for a metadata rule preceded by rules that cannot be blank", [&]() { - auto rule = seq({ - sym("x"), - make_shared(sym("x"), MetadataValue(value | 1)), - }); - AssertThat(check_metadata(rule, value), IsFalse()); - }); -}); - -END_TEST diff --git a/spec/compiler/build_tables/first_set_spec.cc b/spec/compiler/build_tables/first_set_spec.cc index 30b07a49..cc95112c 100644 --- a/spec/compiler/build_tables/first_set_spec.cc +++ b/spec/compiler/build_tables/first_set_spec.cc @@ -86,7 +86,7 @@ describe("computing FIRST sets", []() { }); it("ignores metadata rules", [&]() { - auto rule = make_shared(sym("x"), MetadataValue(1)); + auto rule = make_shared(sym("x"), map()); AssertThat(first_set(rule, null_grammar), Equals(set({ Symbol("x"), diff --git a/spec/compiler/build_tables/get_metadata_spec.cc b/spec/compiler/build_tables/get_metadata_spec.cc new file mode 100644 index 00000000..4f806b6c --- /dev/null +++ b/spec/compiler/build_tables/get_metadata_spec.cc @@ -0,0 +1,81 @@ +#include "compiler_spec_helper.h" +#include "compiler/rules/metadata.h" +#include "compiler/build_tables/get_metadata.h" + +using namespace rules; +using namespace build_tables; + +START_TEST + +describe("getting metadata for rules", []() { + MetadataKey key1 = MetadataKey(100); + MetadataKey key2 = MetadataKey(101); + + describe("when given a metadata rule", [&]() { + auto rule = make_shared(sym("x"), map({ + { key1, 1 }, + { key2, 2 }, + })); + + it("returns the value for the given key", [&]() { + AssertThat(get_metadata(rule, key1), Equals(1)); + AssertThat(get_metadata(rule, key2), Equals(2)); + }); + + it("returns 0 if the rule does not have the key", [&]() { + AssertThat(get_metadata(rule, MetadataKey(3)), Equals(0)); + }); + }); + + describe("when given a non-metadata rule", [&]() { + it("returns 0", [&]() { + auto rule = sym("x"); + AssertThat(get_metadata(rule, key1), Equals(0)); + }); + }); + + it("works for metadata rules preceded by other rules that can be blank", [&]() { + auto rule = seq({ + repeat(sym("x")), + make_shared(sym("x"), map({ + { key1, 1 }, + { key2, 2 }, + })), + }); + + AssertThat(get_metadata(rule, key2), Equals(2)); + }); + + it("works for choices containing metadata rule", [&]() { + auto rule = choice({ + sym("x"), + make_shared(sym("x"), map({ + { key1, 1 }, + { key2, 2 }, + })), + }); + + AssertThat(get_metadata(rule, key2), Equals(1)); + }); + + it("works for repetitions containing metadata rules", [&]() { + auto rule = repeat(make_shared(sym("x"), map({ + { key1, 1 }, + { key2, 2 }, + }))); + AssertThat(get_metadata(rule, key2), Equals(2)); + }); + + it("returns 0 for metadata rules preceded by rules that can't be blank", [&]() { + auto rule = seq({ + sym("x"), + make_shared(sym("y"), map({ + { key1, 1 }, + { key2, 2 }, + })), + }); + AssertThat(get_metadata(rule, key2), Equals(0)); + }); +}); + +END_TEST diff --git a/spec/compiler/build_tables/rule_can_be_blank_spec.cc b/spec/compiler/build_tables/rule_can_be_blank_spec.cc index f5cefc5a..6a2d92a8 100644 --- a/spec/compiler/build_tables/rule_can_be_blank_spec.cc +++ b/spec/compiler/build_tables/rule_can_be_blank_spec.cc @@ -48,10 +48,10 @@ describe("checking if rules can be blank", [&]() { }); it("ignores metadata rules", [&]() { - rule = make_shared(blank(), rules::MetadataValue(0)); + rule = make_shared(blank(), map()); AssertThat(rule_can_be_blank(rule), IsTrue()); - rule = make_shared(sym("one"), rules::MetadataValue(0)); + rule = make_shared(sym("one"), map()); AssertThat(rule_can_be_blank(rule), IsFalse()); }); diff --git a/src/compiler/build_tables/build_tables.cc b/src/compiler/build_tables/build_tables.cc index 9eed8c39..433d86aa 100644 --- a/src/compiler/build_tables/build_tables.cc +++ b/src/compiler/build_tables/build_tables.cc @@ -15,6 +15,7 @@ namespace tree_sitter { using std::pair; using std::string; + using std::map; using std::unordered_map; using std::make_shared; using rules::Symbol; @@ -50,7 +51,7 @@ namespace tree_sitter { void add_token_start(const LexItemSet &item_set, LexStateId state_id) { for (auto &item : item_set) - if (item.has_metadata(rules::START_TOKEN)) + if (item.get_metadata(rules::START_TOKEN)) lex_table.state(state_id).is_token_start = true; } @@ -82,7 +83,7 @@ namespace tree_sitter { rules::rule_ptr after_separators(rules::rule_ptr rule) { return rules::Seq::Build({ make_shared(CharacterSet({ ' ', '\t', '\n', '\r' }).copy()), - make_shared(rule, rules::START_TOKEN) + make_shared(rule, map({ {rules::START_TOKEN, 1} })) }); } diff --git a/src/compiler/build_tables/check_metadata.cc b/src/compiler/build_tables/get_metadata.cc similarity index 51% rename from src/compiler/build_tables/check_metadata.cc rename to src/compiler/build_tables/get_metadata.cc index c279f50c..4c45231d 100644 --- a/src/compiler/build_tables/check_metadata.cc +++ b/src/compiler/build_tables/get_metadata.cc @@ -1,4 +1,4 @@ -#include "check_metadata.h" +#include "get_metadata.h" #include "compiler/rules/seq.h" #include "compiler/rules/choice.h" #include "compiler/rules/repeat.h" @@ -7,10 +7,11 @@ namespace tree_sitter { namespace build_tables { - class HasMetadata : public rules::RuleFn { - rules::MetadataValue metadata_value; + class GetMetadata : public rules::RuleFn { + rules::MetadataKey metadata_key; public: - HasMetadata(rules::MetadataValue value) : metadata_value(value) {} + GetMetadata(rules::MetadataKey key) : + metadata_key(key) {} void visit(const rules::Choice *rule) { value = apply(rule->left) || apply(rule->right); @@ -21,19 +22,21 @@ namespace tree_sitter { } void visit(const rules::Seq *rule) { - bool result = apply(rule->left); - if (rule_can_be_blank(rule->left)) - result = result || apply(rule->right); + int result = apply(rule->left); + if (rule_can_be_blank(rule->left) && result == 0) + result = apply(rule->right); value = result; } void visit(const rules::Metadata *rule) { - value = rule->value & metadata_value; + auto pair = rule->value.find(metadata_key); + if (pair != rule->value.end()) + value = pair->second; } }; - bool check_metadata(const rules::rule_ptr &rule, rules::MetadataValue value) { - return HasMetadata(value).apply(rule); + int get_metadata(const rules::rule_ptr &rule, rules::MetadataKey key) { + return GetMetadata(key).apply(rule); } } } \ No newline at end of file diff --git a/src/compiler/build_tables/check_metadata.h b/src/compiler/build_tables/get_metadata.h similarity index 76% rename from src/compiler/build_tables/check_metadata.h rename to src/compiler/build_tables/get_metadata.h index 810fda3f..08dec9cf 100644 --- a/src/compiler/build_tables/check_metadata.h +++ b/src/compiler/build_tables/get_metadata.h @@ -6,7 +6,7 @@ namespace tree_sitter { namespace build_tables { - bool check_metadata(const rules::rule_ptr &rule, rules::MetadataValue value); + int get_metadata(const rules::rule_ptr &rule, rules::MetadataKey key); } } diff --git a/src/compiler/build_tables/item.cc b/src/compiler/build_tables/item.cc index 106d2b98..0d57711b 100644 --- a/src/compiler/build_tables/item.cc +++ b/src/compiler/build_tables/item.cc @@ -1,6 +1,6 @@ #include "compiler/build_tables/item.h" #include "compiler/build_tables/rule_can_be_blank.h" -#include "compiler/build_tables/check_metadata.h" +#include "compiler/build_tables/get_metadata.h" #include "tree_sitter/compiler.h" namespace tree_sitter { @@ -20,8 +20,8 @@ namespace tree_sitter { return rule_can_be_blank(rule); } - bool Item::has_metadata(rules::MetadataValue value) const { - return check_metadata(rule, value); + int Item::get_metadata(rules::MetadataKey key) const { + return build_tables::get_metadata(rule, key); } ostream& operator<<(ostream &stream, const LexItem &item) { diff --git a/src/compiler/build_tables/item.h b/src/compiler/build_tables/item.h index 1956ccae..6dca4e95 100644 --- a/src/compiler/build_tables/item.h +++ b/src/compiler/build_tables/item.h @@ -15,7 +15,7 @@ namespace tree_sitter { public: Item(const rules::Symbol &lhs, rules::rule_ptr rule); bool is_done() const; - bool has_metadata(rules::MetadataValue) const; + int get_metadata(rules::MetadataKey) const; rules::Symbol lhs; rules::rule_ptr rule; diff --git a/src/compiler/build_tables/item_set_closure.cc b/src/compiler/build_tables/item_set_closure.cc index ff43fde5..a146d6d7 100644 --- a/src/compiler/build_tables/item_set_closure.cc +++ b/src/compiler/build_tables/item_set_closure.cc @@ -25,7 +25,7 @@ namespace tree_sitter { Symbol non_terminal = pair.first; set terminals = pair.second; for (auto &terminal : terminals) { - ParseItem next_item(non_terminal, grammar.rule(non_terminal), {}, terminal); + ParseItem next_item(non_terminal, grammar.rule(non_terminal), 0, terminal); add_item(item_set, next_item, grammar); } } diff --git a/src/compiler/rules/metadata.cc b/src/compiler/rules/metadata.cc index bad7e2fd..b11be955 100644 --- a/src/compiler/rules/metadata.cc +++ b/src/compiler/rules/metadata.cc @@ -6,9 +6,10 @@ namespace tree_sitter { using std::hash; using std::make_shared; + using std::map; namespace rules { - Metadata::Metadata(rule_ptr rule, MetadataValue value) : rule(rule), value(value) {} + Metadata::Metadata(rule_ptr rule, map values) : rule(rule), value(values) {} bool Metadata::operator==(const Rule &rule) const { auto other = dynamic_cast(&rule); @@ -16,7 +17,12 @@ namespace tree_sitter { } size_t Metadata::hash_code() const { - return hash()(value); + size_t result = hash()(value.size()); + for (auto &pair : value) { + result ^= hash()(pair.first); + result ^= hash()(pair.second); + } + return result; } rule_ptr Metadata::copy() const { diff --git a/src/compiler/rules/metadata.h b/src/compiler/rules/metadata.h index 8a4608da..916279b9 100644 --- a/src/compiler/rules/metadata.h +++ b/src/compiler/rules/metadata.h @@ -2,18 +2,19 @@ #define COMPILER_RULES_METADATA_H_ #include +#include #include "compiler/rules/rule.h" namespace tree_sitter { namespace rules { typedef enum { - NONE = 0, - START_TOKEN = 1, - } MetadataValue; + START_TOKEN, + PRECEDENCE + } MetadataKey; class Metadata : public Rule { public: - Metadata(rule_ptr rule, MetadataValue value); + Metadata(rule_ptr rule, std::map value); bool operator==(const Rule& other) const; size_t hash_code() const; @@ -22,7 +23,7 @@ namespace tree_sitter { void accept(Visitor *visitor) const; const rule_ptr rule; - const MetadataValue value; + const std::map value; }; } }