From a54d293317f5a5d70419e57eb8a4386ba8d9e7b3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Apr 2015 17:43:30 -0700 Subject: [PATCH] Include 'goal' non-terminal names in conflict description --- .../parse_conflict_manager_spec.cc | 46 +++++++++++-------- .../build_tables/build_parse_table.cc | 12 ++--- .../build_tables/parse_conflict_manager.cc | 29 +++++++++--- .../build_tables/parse_conflict_manager.h | 5 +- 4 files changed, 60 insertions(+), 32 deletions(-) diff --git a/spec/compiler/build_tables/parse_conflict_manager_spec.cc b/spec/compiler/build_tables/parse_conflict_manager_spec.cc index 17a88b7e..34b8e506 100644 --- a/spec/compiler/build_tables/parse_conflict_manager_spec.cc +++ b/spec/compiler/build_tables/parse_conflict_manager_spec.cc @@ -54,16 +54,21 @@ describe("ParseConflictManager", []() { }); describe(".resolve", [&]() { + ParseItemSet item_set({ + { ParseItem(Symbol(0), blank(), { Symbol(0, SymbolOptionToken) }), set() }, // in_progress_rule1 + { ParseItem(Symbol(3), blank(), {}), set() }, // other_rule1 + }); + describe("errors", [&]() { ParseAction error = ParseAction::Error(); ParseAction non_error = ParseAction::Shift(2, { 0 }); it("favors non-errors and reports no conflict", [&]() { - result = conflict_manager->resolve(non_error, error, sym1); + result = conflict_manager->resolve(non_error, error, sym1, item_set); AssertThat(get<0>(result), IsTrue()); AssertThat(get<1>(result), Equals(ConflictTypeNone)); - result = conflict_manager->resolve(error, non_error, sym1); + result = conflict_manager->resolve(error, non_error, sym1, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeNone)); }); @@ -75,11 +80,11 @@ describe("ParseConflictManager", []() { ParseAction reduce = ParseAction::Reduce(sym2, 1, 1, AssociativityLeft, 0); it("favors the shift and reports the conflict as resolved", [&]() { - result = conflict_manager->resolve(shift, reduce, sym1); + result = conflict_manager->resolve(shift, reduce, sym1, item_set); AssertThat(get<0>(result), IsTrue()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); - result = conflict_manager->resolve(reduce, shift, sym1); + result = conflict_manager->resolve(reduce, shift, sym1, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); }); @@ -92,11 +97,11 @@ describe("ParseConflictManager", []() { ParseAction reduce = ParseAction::Reduce(sym2, 1, 3, AssociativityLeft, 0); it("favors the reduce and reports the conflict as resolved", [&]() { - result = conflict_manager->resolve(shift, reduce, sym1); + result = conflict_manager->resolve(shift, reduce, sym1, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); - result = conflict_manager->resolve(reduce, shift, sym1); + result = conflict_manager->resolve(reduce, shift, sym1, item_set); AssertThat(get<0>(result), IsTrue()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); }); @@ -107,11 +112,11 @@ describe("ParseConflictManager", []() { ParseAction reduce = ParseAction::Reduce(sym2, 1, 0, AssociativityLeft, 0); it("favors the reduce and reports the conflict as resolved", [&]() { - result = conflict_manager->resolve(reduce, shift, sym1); + result = conflict_manager->resolve(reduce, shift, sym1, item_set); AssertThat(get<0>(result), IsTrue()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); - result = conflict_manager->resolve(shift, reduce, sym1); + result = conflict_manager->resolve(shift, reduce, sym1, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); }); @@ -122,11 +127,11 @@ describe("ParseConflictManager", []() { ParseAction reduce = ParseAction::Reduce(sym2, 1, 0, AssociativityRight, 0); it("favors the shift, and reports the conflict as resolved", [&]() { - result = conflict_manager->resolve(reduce, shift, sym1); + result = conflict_manager->resolve(reduce, shift, sym1, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); - result = conflict_manager->resolve(shift, reduce, sym1); + result = conflict_manager->resolve(shift, reduce, sym1, item_set); AssertThat(get<0>(result), IsTrue()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); }); @@ -142,19 +147,21 @@ describe("ParseConflictManager", []() { Symbol(4), })); - result = conflict_manager->resolve(reduce, shift, lookahead_sym); + result = conflict_manager->resolve(reduce, shift, lookahead_sym, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeError)); AssertThat(get<2>(result), Equals( + "Within: in_progress_rule1\n" "Lookahead: lookahead_token\n" "Possible Actions:\n" "* Shift (Precedence 0)\n" "* Reduce other_rule1 other_rule2 -> reduced_rule (Precedence 0)" )); - result = conflict_manager->resolve(shift, reduce, lookahead_sym); + result = conflict_manager->resolve(shift, reduce, lookahead_sym, item_set); AssertThat(get<0>(result), IsTrue()); AssertThat(get<2>(result), Equals( + "Within: in_progress_rule1\n" "Lookahead: lookahead_token\n" "Possible Actions:\n" "* Shift (Precedence 0)\n" @@ -173,14 +180,15 @@ describe("ParseConflictManager", []() { Symbol(4), })); - result = conflict_manager->resolve(reduce, shift, lookahead_sym); + result = conflict_manager->resolve(reduce, shift, lookahead_sym, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeError)); - result = conflict_manager->resolve(shift, reduce, lookahead_sym); + result = conflict_manager->resolve(shift, reduce, lookahead_sym, item_set); AssertThat(get<0>(result), IsTrue()); AssertThat(get<1>(result), Equals(ConflictTypeError)); AssertThat(get<2>(result), Equals( + "Within: in_progress_rule1\n" "Lookahead: lookahead_token\n" "Possible Actions:\n" "* Shift (Precedences 0, 3)\n" @@ -196,11 +204,11 @@ describe("ParseConflictManager", []() { ParseAction right = ParseAction::Reduce(sym2, 1, 3, AssociativityLeft, 0); it("favors that action", [&]() { - result = conflict_manager->resolve(left, right, sym1); + result = conflict_manager->resolve(left, right, sym1, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); - result = conflict_manager->resolve(right, left, sym1); + result = conflict_manager->resolve(right, left, sym1, item_set); AssertThat(get<0>(result), IsTrue()); AssertThat(get<1>(result), Equals(ConflictTypeResolved)); }); @@ -220,20 +228,22 @@ describe("ParseConflictManager", []() { Symbol(4), })); - result = conflict_manager->resolve(right, left, lookahead_sym); + result = conflict_manager->resolve(right, left, lookahead_sym, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeError)); AssertThat(get<2>(result), Equals( + "Within: in_progress_rule1\n" "Lookahead: lookahead_token\n" "Possible Actions:\n" "* Reduce other_rule1 other_rule2 -> reduced_rule (Precedence 0)\n" "* Reduce other_rule2 -> other_rule1 (Precedence 0)" )); - result = conflict_manager->resolve(left, right, lookahead_sym); + result = conflict_manager->resolve(left, right, lookahead_sym, item_set); AssertThat(get<0>(result), IsFalse()); AssertThat(get<1>(result), Equals(ConflictTypeError)); AssertThat(get<2>(result), Equals( + "Within: in_progress_rule1\n" "Lookahead: lookahead_token\n" "Possible Actions:\n" "* Reduce other_rule2 -> other_rule1 (Precedence 0)\n" diff --git a/src/compiler/build_tables/build_parse_table.cc b/src/compiler/build_tables/build_parse_table.cc index 5dc63c05..697d2569 100644 --- a/src/compiler/build_tables/build_parse_table.cc +++ b/src/compiler/build_tables/build_parse_table.cc @@ -93,7 +93,7 @@ class ParseTableBuilder { ParseAction new_action = ParseAction::Shift(0, precedence_values_for_item_set(next_item_set)); - if (should_add_action(state_id, symbol, new_action)) { + if (should_add_action(state_id, item_set, symbol, new_action)) { ParseStateId new_state_id = add_parse_state(next_item_set); new_action.state_index = new_state_id; parse_table.add_action(state_id, symbol, new_action); @@ -115,7 +115,7 @@ class ParseTableBuilder { conflict_manager.get_production_id(item.consumed_symbols)); for (const auto &lookahead_sym : lookahead_symbols) - if (should_add_action(state_id, lookahead_sym, action)) + if (should_add_action(state_id, item_set, lookahead_sym, action)) parse_table.add_action(state_id, lookahead_sym, action); } } @@ -147,22 +147,22 @@ class ParseTableBuilder { for (const auto &pair : actions) { const Symbol &lookahead_sym = pair.first; ParseAction reduce_extra = ParseAction::ReduceExtra(ubiquitous_symbol); - if (should_add_action(shift_state_id, lookahead_sym, reduce_extra)) + if (should_add_action(shift_state_id, ParseItemSet(), lookahead_sym, reduce_extra)) parse_table.add_action(shift_state_id, lookahead_sym, reduce_extra); } } } } - bool should_add_action(ParseStateId state_id, const Symbol &symbol, - const ParseAction &action) { + bool should_add_action(ParseStateId state_id, const ParseItemSet &item_set, + const Symbol &symbol, const ParseAction &action) { auto ¤t_actions = parse_table.states[state_id].actions; auto current_action = current_actions.find(symbol); if (current_action == current_actions.end()) return true; auto result = conflict_manager.resolve(action, current_action->second, - symbol); + symbol, item_set); switch (get<1>(result)) { case ConflictTypeResolved: diff --git a/src/compiler/build_tables/parse_conflict_manager.cc b/src/compiler/build_tables/parse_conflict_manager.cc index 601705f9..e93fff6a 100644 --- a/src/compiler/build_tables/parse_conflict_manager.cc +++ b/src/compiler/build_tables/parse_conflict_manager.cc @@ -23,9 +23,10 @@ ParseConflictManager::ParseConflictManager(const SyntaxGrammar &syntax_grammar, tuple ParseConflictManager::resolve(const ParseAction &new_action, const ParseAction &old_action, - const rules::Symbol &symbol) const { + const rules::Symbol &symbol, + const ParseItemSet &item_set) const { if (new_action.type < old_action.type) { - auto opposite = resolve(old_action, new_action, symbol); + auto opposite = resolve(old_action, new_action, symbol, item_set); return make_tuple(!get<0>(opposite), get<1>(opposite), get<2>(opposite)); } @@ -49,10 +50,10 @@ ParseConflictManager::resolve(const ParseAction &new_action, case rules::AssociativityRight: return make_tuple(false, ConflictTypeResolved, ""); default: - return make_tuple(false, ConflictTypeError, conflict_description(new_action, old_action, symbol)); + return make_tuple(false, ConflictTypeError, conflict_description(new_action, old_action, symbol, item_set)); } } else { - return make_tuple(false, ConflictTypeError, conflict_description(new_action, old_action, symbol)); + return make_tuple(false, ConflictTypeError, conflict_description(new_action, old_action, symbol, item_set)); } } @@ -65,7 +66,7 @@ ParseConflictManager::resolve(const ParseAction &new_action, } else if (new_precedence < old_precedence) { return make_tuple(false, ConflictTypeResolved, ""); } else { - return make_tuple(false, ConflictTypeError, conflict_description(new_action, old_action, symbol)); + return make_tuple(false, ConflictTypeError, conflict_description(new_action, old_action, symbol, item_set)); } } @@ -85,10 +86,26 @@ size_t ParseConflictManager::get_production_id(const vector &symb return iter - begin; } +string ParseConflictManager::item_set_description(const ParseItemSet &item_set) const { + string result; + bool started = false; + for (const auto &pair : item_set) { + const ParseItem &item = pair.first; + if (!item.consumed_symbols.empty()) { + if (started) result += ", "; + result += symbol_name(item.lhs); + started = true; + } + } + return result; +} + string ParseConflictManager::conflict_description(const ParseAction &new_action, const ParseAction &old_action, - const rules::Symbol &symbol) const { + const rules::Symbol &symbol, + const ParseItemSet &item_set) const { return + "Within: " + item_set_description(item_set) + "\n" "Lookahead: " + symbol_name(symbol) + "\n" + "Possible Actions:\n" "* " + action_description(old_action) + "\n" + diff --git a/src/compiler/build_tables/parse_conflict_manager.h b/src/compiler/build_tables/parse_conflict_manager.h index 05281348..1a282db8 100644 --- a/src/compiler/build_tables/parse_conflict_manager.h +++ b/src/compiler/build_tables/parse_conflict_manager.h @@ -29,12 +29,13 @@ class ParseConflictManager { ParseConflictManager(const SyntaxGrammar &, const LexicalGrammar &); size_t get_production_id(const std::vector &); std::tuple resolve( - const ParseAction &, const ParseAction &, const rules::Symbol &) const; + const ParseAction &, const ParseAction &, const rules::Symbol &, const ParseItemSet &) const; private: std::string symbol_name(const rules::Symbol &) const; + std::string item_set_description(const ParseItemSet &) const; std::string action_description(const ParseAction &) const; - std::string conflict_description(const ParseAction &, const ParseAction &, const rules::Symbol &) const; + std::string conflict_description(const ParseAction &, const ParseAction &, const rules::Symbol &, const ParseItemSet &) const; }; } // namespace build_tables