From 7872ddd21b429f61cb6699c917bc9dc8b501bf37 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 16 Mar 2015 11:59:45 -0700 Subject: [PATCH] Improve reduce/reduce conflict error message --- project.gyp | 4 +- .../action_takes_precedence_spec.cc | 221 ------------------ .../build_tables/build_conflict_spec.cc | 81 ------- .../build_tables/lex_conflict_manager_spec.cc | 66 ++++++ .../parse_conflict_manager_spec.cc | 181 ++++++++++++++ .../build_tables/action_takes_precedence.cc | 101 -------- .../build_tables/action_takes_precedence.h | 30 --- src/compiler/build_tables/build_conflict.cc | 75 ------ src/compiler/build_tables/build_conflict.h | 23 -- src/compiler/build_tables/build_lex_table.cc | 10 +- .../build_tables/build_parse_table.cc | 55 ++--- .../build_tables/lex_conflict_manager.cc | 49 ++++ .../build_tables/lex_conflict_manager.h | 24 ++ .../build_tables/parse_conflict_manager.cc | 111 +++++++++ .../build_tables/parse_conflict_manager.h | 44 ++++ 15 files changed, 497 insertions(+), 578 deletions(-) delete mode 100644 spec/compiler/build_tables/action_takes_precedence_spec.cc delete mode 100644 spec/compiler/build_tables/build_conflict_spec.cc create mode 100644 spec/compiler/build_tables/lex_conflict_manager_spec.cc create mode 100644 spec/compiler/build_tables/parse_conflict_manager_spec.cc delete mode 100644 src/compiler/build_tables/action_takes_precedence.cc delete mode 100644 src/compiler/build_tables/action_takes_precedence.h delete mode 100644 src/compiler/build_tables/build_conflict.cc delete mode 100644 src/compiler/build_tables/build_conflict.h create mode 100644 src/compiler/build_tables/lex_conflict_manager.cc create mode 100644 src/compiler/build_tables/lex_conflict_manager.h create mode 100644 src/compiler/build_tables/parse_conflict_manager.cc create mode 100644 src/compiler/build_tables/parse_conflict_manager.h diff --git a/project.gyp b/project.gyp index 281f7fd8..7e566d8f 100644 --- a/project.gyp +++ b/project.gyp @@ -10,8 +10,6 @@ 'externals/utf8proc', ], 'sources': [ - 'src/compiler/build_tables/action_takes_precedence.cc', - 'src/compiler/build_tables/build_conflict.cc', 'src/compiler/build_tables/build_lex_table.cc', 'src/compiler/build_tables/build_parse_table.cc', 'src/compiler/build_tables/build_tables.cc', @@ -21,7 +19,9 @@ 'src/compiler/build_tables/item_set_closure.cc', 'src/compiler/build_tables/item_set_transitions.cc', 'src/compiler/build_tables/lex_item.cc', + 'src/compiler/build_tables/lex_conflict_manager.cc', 'src/compiler/build_tables/parse_item.cc', + 'src/compiler/build_tables/parse_conflict_manager.cc', 'src/compiler/build_tables/rule_can_be_blank.cc', 'src/compiler/build_tables/rule_transitions.cc', 'src/compiler/compile.cc', diff --git a/spec/compiler/build_tables/action_takes_precedence_spec.cc b/spec/compiler/build_tables/action_takes_precedence_spec.cc deleted file mode 100644 index b8a80611..00000000 --- a/spec/compiler/build_tables/action_takes_precedence_spec.cc +++ /dev/null @@ -1,221 +0,0 @@ -#include "compiler/compiler_spec_helper.h" -#include "compiler/rules/built_in_symbols.h" -#include "compiler/parse_table.h" -#include "compiler/build_tables/action_takes_precedence.h" -#include "compiler/syntax_grammar.h" - -using namespace rules; -using namespace build_tables; - -START_TEST - -describe("action_takes_precedence", []() { - bool update; - - describe("lexical conflicts", [&]() { - Symbol sym1(0, SymbolOptionToken); - Symbol sym2(1, SymbolOptionToken); - Symbol sym3(2, SymbolOptionToken); - - it("favors non-errors over lexical errors", [&]() { - update = action_takes_precedence(LexAction::Advance(2, {0}), LexAction::Error()); - AssertThat(update, IsTrue()); - - update = action_takes_precedence(LexAction::Error(), LexAction::Advance(2, {0})); - AssertThat(update, IsFalse()); - }); - - describe("accept-token/advance conflicts", [&]() { - it("prefers the advance", [&]() { - update = action_takes_precedence(LexAction::Advance(1, { 0 }), LexAction::Accept(sym3, 3)); - AssertThat(update, IsTrue()); - - update = action_takes_precedence(LexAction::Accept(sym3, 3), LexAction::Advance(1, { 0 })); - AssertThat(update, IsFalse()); - }); - }); - - describe("accept-token/accept-token conflicts", [&]() { - describe("when one token has a higher precedence than the other", [&]() { - it("prefers the token with the higher precedence", [&]() { - update = action_takes_precedence(LexAction::Accept(sym2, 0), LexAction::Accept(sym3, 2)); - AssertThat(update, IsFalse()); - - update = action_takes_precedence(LexAction::Accept(sym3, 2), LexAction::Accept(sym2, 0)); - AssertThat(update, IsTrue()); - }); - }); - - describe("when both tokens have the same precedence", [&]() { - it("prefers the token listed earlier in the grammar", [&]() { - update = action_takes_precedence(LexAction::Accept(sym2, 0), LexAction::Accept(sym1, 0)); - AssertThat(update, IsFalse()); - - update = action_takes_precedence(LexAction::Accept(sym1, 0), LexAction::Accept(sym2, 0)); - AssertThat(update, IsTrue()); - }); - }); - }); - }); - - describe("parsing conflicts", [&]() { - pair result; - Symbol sym1(0); - Symbol sym2(1); - - describe("errors", [&]() { - ParseAction error = ParseAction::Error(); - ParseAction non_error = ParseAction::Shift(2, { 0 }); - - it("favors non-errors", [&]() { - result = action_takes_precedence(non_error, error, sym1); - AssertThat(result.first, IsTrue()); - - result = action_takes_precedence(error, non_error, sym1); - AssertThat(result.first, IsFalse()); - }); - - it("is not a conflict", [&]() { - result = action_takes_precedence(non_error, error, sym1); - AssertThat(result.second, Equals(ConflictTypeNone)); - - result = action_takes_precedence(error, non_error, sym1); - AssertThat(result.second, Equals(ConflictTypeNone)); - }); - }); - - describe("shift/reduce conflicts", [&]() { - describe("when the shift has higher precedence", [&]() { - ParseAction shift = ParseAction::Shift(2, { 3 }); - ParseAction reduce = ParseAction::Reduce(sym2, 1, 1, 0); - - it("is not a conflict", [&]() { - result = action_takes_precedence(shift, reduce, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - - result = action_takes_precedence(reduce, shift, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - }); - - it("favors the shift", [&]() { - result = action_takes_precedence(shift, reduce, sym1); - AssertThat(result.first, IsTrue()); - - result = action_takes_precedence(reduce, shift, sym1); - AssertThat(result.first, IsFalse()); - }); - }); - - describe("when the reduce has higher precedence", [&]() { - ParseAction shift = ParseAction::Shift(2, { 1 }); - ParseAction reduce = ParseAction::Reduce(sym2, 1, 3, 0); - - it("is not a conflict", [&]() { - result = action_takes_precedence(shift, reduce, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - - result = action_takes_precedence(reduce, shift, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - }); - - it("favors the reduce", [&]() { - result = action_takes_precedence(shift, reduce, sym1); - AssertThat(result.first, IsFalse()); - - result = action_takes_precedence(reduce, shift, sym1); - AssertThat(result.first, IsTrue()); - }); - }); - - describe("when the precedences are equal", [&]() { - ParseAction shift = ParseAction::Shift(2, { 0 }); - ParseAction reduce = ParseAction::Reduce(sym2, 1, 0, 0); - - // TODO: Add associativity annotations. These should be errors. - it("is a conflict", [&]() { - result = action_takes_precedence(reduce, shift, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - - result = action_takes_precedence(shift, reduce, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - }); - - it("favors the shift", [&]() { - result = action_takes_precedence(reduce, shift, sym1); - AssertThat(result.first, IsFalse()); - - result = action_takes_precedence(shift, reduce, sym1); - AssertThat(result.first, IsTrue()); - }); - }); - - describe("when the shift has conflicting precedences compared to the reduce", [&]() { - ParseAction shift = ParseAction::Shift(2, { 0, 1, 3 }); - ParseAction reduce = ParseAction::Reduce(sym2, 1, 2, 0); - - // TODO: Add associativity annotations. These should be errors. - it("is a conflict", [&]() { - result = action_takes_precedence(reduce, shift, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - - result = action_takes_precedence(shift, reduce, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - }); - - it("favors the shift", [&]() { - result = action_takes_precedence(reduce, shift, sym1); - AssertThat(result.first, IsFalse()); - - result = action_takes_precedence(shift, reduce, sym1); - AssertThat(result.first, IsTrue()); - }); - }); - }); - - describe("reduce/reduce conflicts", [&]() { - describe("when one action has higher precedence", [&]() { - ParseAction left = ParseAction::Reduce(sym2, 1, 0, 0); - ParseAction right = ParseAction::Reduce(sym2, 1, 3, 0); - - it("favors that action", [&]() { - result = action_takes_precedence(left, right, sym1); - AssertThat(result.first, IsFalse()); - - result = action_takes_precedence(right, left, sym1); - AssertThat(result.first, IsTrue()); - }); - - it("is not a conflict", [&]() { - result = action_takes_precedence(left, right, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - - result = action_takes_precedence(right, left, sym1); - AssertThat(result.second, Equals(ConflictTypeResolved)); - }); - }); - - describe("when the actions have the same precedence", [&]() { - ParseAction left = ParseAction::Reduce(sym1, 1, 0, 0); - ParseAction right = ParseAction::Reduce(sym2, 1, 0, 0); - - it("returns false", [&]() { - result = action_takes_precedence(left, right, sym1); - AssertThat(result.first, IsFalse()); - - result = action_takes_precedence(right, left, sym1); - AssertThat(result.first, IsFalse()); - }); - - it("records a conflict", [&]() { - result = action_takes_precedence(left, right, sym1); - AssertThat(result.second, Equals(ConflictTypeError)); - - result = action_takes_precedence(right, left, sym1); - AssertThat(result.second, Equals(ConflictTypeError)); - }); - }); - }); - }); -}); - -END_TEST diff --git a/spec/compiler/build_tables/build_conflict_spec.cc b/spec/compiler/build_tables/build_conflict_spec.cc deleted file mode 100644 index 0ccbe6d5..00000000 --- a/spec/compiler/build_tables/build_conflict_spec.cc +++ /dev/null @@ -1,81 +0,0 @@ -#include "compiler/compiler_spec_helper.h" -#include "compiler/build_tables/build_conflict.h" -#include "compiler/syntax_grammar.h" -#include "compiler/lexical_grammar.h" - -using namespace rules; -using namespace build_tables; - -START_TEST - -describe("build_conflict", []() { - string conflict; - - SyntaxGrammar parse_grammar({ - { "in_progress_rule1", i_token(0) }, - { "in_progress_rule2", i_token(0) }, - { "reduced_rule", i_token(0) }, - { "other_ruel1", i_token(0) }, - { "other_rule2", i_token(0) }, - }, {}, { Symbol(2, SymbolOptionToken) }); - - LexicalGrammar lex_grammar({ - { "other_token", pattern("[a-b]") }, - { "lookahead_token", pattern("[c-d]") }, - }, {}); - - it("uses the given item-set to determine which symbols are involved in the shift", [&]() { - conflict = build_conflict( - ParseAction::Shift(2, set()), - ParseAction::Reduce(Symbol(2), 1, 0, 0), // reduced_rule - ParseItemSet({ - { - ParseItem(Symbol(0), blank(), { Symbol(101) }), // in_progress_rule1 - set({ Symbol(2, SymbolOptionToken) }) - }, - { - ParseItem(Symbol(1), blank(), { Symbol(101) }), // in_progress_rule2 - set({ Symbol(2, SymbolOptionToken) }) - }, - { - ParseItem(Symbol(3), blank(), {}), // other_rule1 - set({ Symbol(2, SymbolOptionToken) }) - }, - }), - Symbol(1, SymbolOptionToken), // lookahead_token - parse_grammar, lex_grammar - ); - - AssertThat(conflict, Equals( - "lookahead_token: " - "shift ( in_progress_rule1 in_progress_rule2 ) / " - "reduce ( reduced_rule )")); - }); - - it("always puts shift actions before reduce actions", [&]() { - conflict = build_conflict( - ParseAction::Reduce(Symbol(2), 1, 0, 0), // reduced_rule - ParseAction::Shift(2, set()), - ParseItemSet({ - { - ParseItem(Symbol(0), blank(), { Symbol(101) }), // in_progress_rule1 - set({ Symbol(2, SymbolOptionToken) }) - }, - { - ParseItem(Symbol(1), blank(), { Symbol(101) }), // in_progress_rule2 - set({ Symbol(2, SymbolOptionToken) }) - }, - }), - Symbol(1, SymbolOptionToken), // lookahead_token - parse_grammar, lex_grammar - ); - - AssertThat(conflict, Equals( - "lookahead_token: " - "shift ( in_progress_rule1 in_progress_rule2 ) / " - "reduce ( reduced_rule )")); - }); -}); - - -END_TEST diff --git a/spec/compiler/build_tables/lex_conflict_manager_spec.cc b/spec/compiler/build_tables/lex_conflict_manager_spec.cc new file mode 100644 index 00000000..95193385 --- /dev/null +++ b/spec/compiler/build_tables/lex_conflict_manager_spec.cc @@ -0,0 +1,66 @@ +#include "compiler/compiler_spec_helper.h" +#include "compiler/rules/built_in_symbols.h" +#include "compiler/parse_table.h" +#include "compiler/build_tables/lex_conflict_manager.h" +#include "compiler/syntax_grammar.h" + +using namespace rules; +using namespace build_tables; + +START_TEST + +describe("LexConflictManager", []() { + LexicalGrammar lexical_grammar({ + { "other_token", pattern("[a-b]") }, + { "lookahead_token", pattern("[c-d]") }, + }, {}); + + LexConflictManager conflict_manager(lexical_grammar); + + bool update; + Symbol sym1(0, SymbolOptionToken); + Symbol sym2(1, SymbolOptionToken); + Symbol sym3(2, SymbolOptionToken); + + it("favors non-errors over lexical errors", [&]() { + update = conflict_manager.resolve(LexAction::Advance(2, {0}), LexAction::Error()); + AssertThat(update, IsTrue()); + + update = conflict_manager.resolve(LexAction::Error(), LexAction::Advance(2, {0})); + AssertThat(update, IsFalse()); + }); + + describe("accept-token/advance conflicts", [&]() { + it("prefers the advance", [&]() { + update = conflict_manager.resolve(LexAction::Advance(1, { 0 }), LexAction::Accept(sym3, 3)); + AssertThat(update, IsTrue()); + + update = conflict_manager.resolve(LexAction::Accept(sym3, 3), LexAction::Advance(1, { 0 })); + AssertThat(update, IsFalse()); + }); + }); + + describe("accept-token/accept-token conflicts", [&]() { + describe("when one token has a higher precedence than the other", [&]() { + it("prefers the token with the higher precedence", [&]() { + update = conflict_manager.resolve(LexAction::Accept(sym2, 0), LexAction::Accept(sym3, 2)); + AssertThat(update, IsFalse()); + + update = conflict_manager.resolve(LexAction::Accept(sym3, 2), LexAction::Accept(sym2, 0)); + AssertThat(update, IsTrue()); + }); + }); + + describe("when both tokens have the same precedence", [&]() { + it("prefers the token listed earlier in the grammar", [&]() { + update = conflict_manager.resolve(LexAction::Accept(sym2, 0), LexAction::Accept(sym1, 0)); + AssertThat(update, IsFalse()); + + update = conflict_manager.resolve(LexAction::Accept(sym1, 0), LexAction::Accept(sym2, 0)); + AssertThat(update, IsTrue()); + }); + }); + }); +}); + +END_TEST diff --git a/spec/compiler/build_tables/parse_conflict_manager_spec.cc b/spec/compiler/build_tables/parse_conflict_manager_spec.cc new file mode 100644 index 00000000..3520d422 --- /dev/null +++ b/spec/compiler/build_tables/parse_conflict_manager_spec.cc @@ -0,0 +1,181 @@ +#include "compiler/compiler_spec_helper.h" +#include "compiler/rules/built_in_symbols.h" +#include "compiler/parse_table.h" +#include "compiler/build_tables/parse_conflict_manager.h" +#include "compiler/syntax_grammar.h" + +using namespace rules; +using namespace build_tables; + +START_TEST + +describe("ParseConflictManager", []() { + SyntaxGrammar syntax_grammar({ + { "in_progress_rule1", i_token(0) }, + { "in_progress_rule2", i_token(0) }, + { "reduced_rule", i_token(0) }, + { "other_rule1", i_token(0) }, + { "other_rule2", i_token(0) }, + }, {}, { Symbol(2, SymbolOptionToken) }); + + LexicalGrammar lexical_grammar({ + { "other_token", pattern("[a-b]") }, + { "lookahead_token", pattern("[c-d]") }, + }, {}); + + tuple result; + Symbol sym1(0); + Symbol sym2(1); + Symbol lookahead_sym(1, SymbolOptionToken); + ParseConflictManager *conflict_manager; + + before_each([&]() { + conflict_manager = new ParseConflictManager(syntax_grammar, lexical_grammar); + }); + + after_each([&]() { + delete conflict_manager; + }); + + describe(".get_production_id", [&]() { + it("returns different IDs for different productions", [&]() { + + }); + }); + + describe(".resolve", [&]() { + 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); + AssertThat(get<0>(result), IsTrue()); + AssertThat(get<1>(result), Equals(ConflictTypeNone)); + + result = conflict_manager->resolve(error, non_error, sym1); + AssertThat(get<0>(result), IsFalse()); + AssertThat(get<1>(result), Equals(ConflictTypeNone)); + }); + }); + + describe("shift/reduce conflicts", [&]() { + describe("when the shift has higher precedence", [&]() { + ParseAction shift = ParseAction::Shift(2, { 3 }); + ParseAction reduce = ParseAction::Reduce(sym2, 1, 1, 0); + + it("favors the shift and reports the conflict as resolved", [&]() { + result = conflict_manager->resolve(shift, reduce, sym1); + AssertThat(get<0>(result), IsTrue()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + + result = conflict_manager->resolve(reduce, shift, sym1); + AssertThat(get<0>(result), IsFalse()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + }); + }); + + describe("when the reduce has higher precedence", [&]() { + ParseAction shift = ParseAction::Shift(2, { 1 }); + ParseAction reduce = ParseAction::Reduce(sym2, 1, 3, 0); + + it("favors the reduce and reports the conflict as resolved", [&]() { + result = conflict_manager->resolve(shift, reduce, sym1); + AssertThat(get<0>(result), IsFalse()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + + result = conflict_manager->resolve(reduce, shift, sym1); + AssertThat(get<0>(result), IsTrue()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + }); + }); + + describe("when the precedences are equal", [&]() { + ParseAction shift = ParseAction::Shift(2, { 0 }); + ParseAction reduce = ParseAction::Reduce(sym2, 1, 0, 0); + + // TODO: Add associativity annotations. These should be errors. + it("favors the shift, and reports the conflict as resolved", [&]() { + result = conflict_manager->resolve(reduce, shift, sym1); + AssertThat(get<0>(result), IsFalse()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + + result = conflict_manager->resolve(shift, reduce, sym1); + AssertThat(get<0>(result), IsTrue()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + }); + }); + + describe("when the shift has conflicting precedences compared to the reduce", [&]() { + ParseAction shift = ParseAction::Shift(2, { 0, 1, 3 }); + ParseAction reduce = ParseAction::Reduce(sym2, 1, 2, 0); + + // TODO: Add associativity annotations. These should be errors. + it("favors the shift, and reports the conflict as resolved", [&]() { + result = conflict_manager->resolve(reduce, shift, sym2); + AssertThat(get<0>(result), IsFalse()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + + result = conflict_manager->resolve(shift, reduce, sym1); + AssertThat(get<0>(result), IsTrue()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + }); + }); + }); + + describe("reduce/reduce conflicts", [&]() { + describe("when one action has higher precedence", [&]() { + ParseAction left = ParseAction::Reduce(sym2, 1, 0, 0); + ParseAction right = ParseAction::Reduce(sym2, 1, 3, 0); + + it("favors that action", [&]() { + result = conflict_manager->resolve(left, right, sym1); + AssertThat(get<0>(result), IsFalse()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + + result = conflict_manager->resolve(right, left, sym1); + AssertThat(get<0>(result), IsTrue()); + AssertThat(get<1>(result), Equals(ConflictTypeResolved)); + }); + }); + + describe("when the actions have the same precedence", [&]() { + it("returns false and reports a conflict", [&]() { + ParseAction left = ParseAction::Reduce(Symbol(2), 1, 0, 0); + ParseAction right = ParseAction::Reduce(Symbol(3), 1, 0, 0); + + left.production_id = conflict_manager->get_production_id(vector({ + Symbol(3), + Symbol(4), + })); + + right.production_id = conflict_manager->get_production_id(vector({ + Symbol(4), + })); + + result = conflict_manager->resolve(right, left, lookahead_sym); + AssertThat(get<0>(result), IsFalse()); + AssertThat(get<1>(result), Equals(ConflictTypeError)); + AssertThat(get<2>(result), Equals( + "Lookahead: lookahead_token\n" + "Possible Actions:\n" + "* Reduce other_rule1 other_rule2 -> reduced_rule\n" + "* Reduce other_rule2 -> other_rule1\n" + )); + + result = conflict_manager->resolve(left, right, lookahead_sym); + AssertThat(get<0>(result), IsFalse()); + AssertThat(get<1>(result), Equals(ConflictTypeError)); + AssertThat(get<2>(result), Equals( + "Lookahead: lookahead_token\n" + "Possible Actions:\n" + "* Reduce other_rule2 -> other_rule1\n" + "* Reduce other_rule1 other_rule2 -> reduced_rule\n" + )); + }); + }); + }); + }); +}); + +END_TEST diff --git a/src/compiler/build_tables/action_takes_precedence.cc b/src/compiler/build_tables/action_takes_precedence.cc deleted file mode 100644 index 9670cf94..00000000 --- a/src/compiler/build_tables/action_takes_precedence.cc +++ /dev/null @@ -1,101 +0,0 @@ -#include "compiler/build_tables/action_takes_precedence.h" - -#include - -namespace tree_sitter { -namespace build_tables { - -using std::pair; - -pair -action_takes_precedence(const ParseAction &new_action, - const ParseAction &old_action, - const rules::Symbol &symbol) { - if (new_action.type < old_action.type) { - auto opposite = - action_takes_precedence(old_action, new_action, symbol); - return { !opposite.first, opposite.second }; - } - - switch (old_action.type) { - case ParseActionTypeError: - return { true, ConflictTypeNone }; - - case ParseActionTypeShift: - if (new_action.type == ParseActionTypeReduce) { - int min_precedence = *old_action.precedence_values.begin(); - int max_precedence = *old_action.precedence_values.rbegin(); - int new_precedence = *new_action.precedence_values.rbegin(); - if (new_precedence < min_precedence) - return { false, ConflictTypeResolved }; - else if (new_precedence > max_precedence) - return { true, ConflictTypeResolved }; - else { - - // TODO: Add associativity annotations. In the event of a precedence - // tie, return ConflictTypeError unless there is an associativity - // annotation to break the tie. - return { false, ConflictTypeResolved }; - } - } - break; - - case ParseActionTypeReduce: - if (new_action.type == ParseActionTypeReduce) { - int old_precedence = *old_action.precedence_values.begin(); - int new_precedence = *new_action.precedence_values.begin(); - if (new_precedence > old_precedence) { - return { true, ConflictTypeResolved }; - } else if (new_precedence < old_precedence) { - return { false, ConflictTypeResolved }; - } else { - return { false, ConflictTypeError }; - } - } - - default: - break; - } - - return { false, ConflictTypeNone }; -} - -bool action_takes_precedence(const LexAction &new_action, - const LexAction &old_action) { - if (new_action.type < old_action.type) - return !action_takes_precedence(old_action, new_action); - - switch (old_action.type) { - case LexActionTypeError: - return true; - - case LexActionTypeAccept: { - int old_precedence = *old_action.precedence_values.begin(); - - switch (new_action.type) { - case LexActionTypeAccept: { - int new_precedence = *new_action.precedence_values.begin(); - if (new_precedence > old_precedence) - return true; - else if (new_precedence < old_precedence) - return false; - else - return new_action.symbol.index < old_action.symbol.index; - } - - case LexActionTypeAdvance: - return true; - - default: - return false; - } - return true; - } - - default: - return false; - } -} - -} // namespace build_tables -} // namespace tree_sitter diff --git a/src/compiler/build_tables/action_takes_precedence.h b/src/compiler/build_tables/action_takes_precedence.h deleted file mode 100644 index 09273898..00000000 --- a/src/compiler/build_tables/action_takes_precedence.h +++ /dev/null @@ -1,30 +0,0 @@ -#ifndef COMPILER_BUILD_TABLES_RESOLVE_PARSE_ACTION_H_ -#define COMPILER_BUILD_TABLES_RESOLVE_PARSE_ACTION_H_ - -#include -#include "tree_sitter/compiler.h" -#include "compiler/parse_table.h" -#include "compiler/rules/symbol.h" -#include "compiler/syntax_grammar.h" - -namespace tree_sitter { -namespace build_tables { - -enum ConflictType { - ConflictTypeNone, - ConflictTypeResolved, - ConflictTypeError -}; - -std::pair -action_takes_precedence(const ParseAction &new_action, - const ParseAction &old_action, - const rules::Symbol &symbol); - -bool action_takes_precedence(const LexAction &new_action, - const LexAction &old_action); - -} // namespace build_tables -} // namespace tree_sitter - -#endif // COMPILER_BUILD_TABLES_RESOLVE_PARSE_ACTION_H_ diff --git a/src/compiler/build_tables/build_conflict.cc b/src/compiler/build_tables/build_conflict.cc deleted file mode 100644 index bb37fa2a..00000000 --- a/src/compiler/build_tables/build_conflict.cc +++ /dev/null @@ -1,75 +0,0 @@ -#include "compiler/build_tables/build_conflict.h" -#include "compiler/rules/symbol.h" -#include "compiler/rules/built_in_symbols.h" -#include "compiler/lexical_grammar.h" -#include "compiler/syntax_grammar.h" -#include "compiler/build_tables/parse_item.h" -#include -#include -#include - -namespace tree_sitter { -namespace build_tables { - -using std::sort; -using std::string; -using std::vector; -using rules::Symbol; - -static string symbol_name(const Symbol &symbol, const SyntaxGrammar &grammar, - const LexicalGrammar &lex_grammar) { - if (symbol.is_built_in()) { - if (symbol == rules::ERROR()) - return "ERROR"; - else if (symbol == rules::END_OF_INPUT()) - return "END_OF_INPUT"; - else - return ""; - } - - if (symbol.is_token()) - return lex_grammar.rule_name(symbol); - else - return grammar.rule_name(symbol); -} - -static string action_description(const ParseAction &action, - const ParseItemSet &item_set, - const SyntaxGrammar &grammar, - const LexicalGrammar &lex_grammar) { - switch (action.type) { - case ParseActionTypeShift: { - string result("shift ("); - vector symbol_names; - for (const auto &item : item_set) - if (!item.first.consumed_symbols.empty()) - symbol_names.push_back(symbol_name(item.first.lhs, grammar, lex_grammar)); - sort(symbol_names.begin(), symbol_names.end()); - for (string symbol_name : symbol_names) - result += " " + symbol_name; - return result + " )"; - } - case ParseActionTypeReduce: - return "reduce ( " + symbol_name(action.symbol, grammar, lex_grammar) + - " )"; - default: - return ""; - } -} - -string build_conflict(const ParseAction &left, const ParseAction &right, - const ParseItemSet &item_set, const Symbol &sym, - const SyntaxGrammar &grammar, - const LexicalGrammar &lex_grammar) { - if (right < left) - return build_conflict(right, left, item_set, sym, grammar, lex_grammar); - - return symbol_name(sym, grammar, lex_grammar) + - ": " + - action_description(left, item_set, grammar, lex_grammar) + - " / " + - action_description(right, item_set, grammar, lex_grammar); -} - -} // namespace build_tables -} // namespace tree_sitter diff --git a/src/compiler/build_tables/build_conflict.h b/src/compiler/build_tables/build_conflict.h deleted file mode 100644 index 51c2c891..00000000 --- a/src/compiler/build_tables/build_conflict.h +++ /dev/null @@ -1,23 +0,0 @@ -#ifndef COMPILER_BUILD_TABLES_BUILD_CONFLICT_H_ -#define COMPILER_BUILD_TABLES_BUILD_CONFLICT_H_ - -#include "tree_sitter/compiler.h" -#include "compiler/parse_table.h" -#include "compiler/rules/symbol.h" -#include "compiler/build_tables/parse_item.h" - -namespace tree_sitter { - -class SyntaxGrammar; -class LexicalGrammar; - -namespace build_tables { - -std::string build_conflict(const ParseAction &, const ParseAction &, - const ParseItemSet &, const rules::Symbol &, - const SyntaxGrammar &, const LexicalGrammar &); - -} // namespace build_tables -} // namespace tree_sitter - -#endif // COMPILER_BUILD_TABLES_BUILD_CONFLICT_H_ diff --git a/src/compiler/build_tables/build_lex_table.cc b/src/compiler/build_tables/build_lex_table.cc index 063ee88a..64b280b6 100644 --- a/src/compiler/build_tables/build_lex_table.cc +++ b/src/compiler/build_tables/build_lex_table.cc @@ -4,7 +4,7 @@ #include #include #include -#include "compiler/build_tables/action_takes_precedence.h" +#include "compiler/build_tables/lex_conflict_manager.h" #include "compiler/build_tables/item_set_transitions.h" #include "compiler/build_tables/lex_item.h" #include "compiler/parse_table.h" @@ -30,13 +30,14 @@ using rules::Symbol; class LexTableBuilder { const LexicalGrammar lex_grammar; + const LexConflictManager conflict_manager; ParseTable *parse_table; unordered_map lex_state_ids; LexTable lex_table; public: LexTableBuilder(ParseTable *parse_table, const LexicalGrammar &lex_grammar) - : lex_grammar(lex_grammar), parse_table(parse_table) {} + : lex_grammar(lex_grammar), conflict_manager(lex_grammar), parse_table(parse_table) {} LexTable build() { for (auto &parse_state : parse_table->states) { @@ -96,8 +97,7 @@ class LexTableBuilder { LexStateId new_state_id = add_lex_state(new_item_set); auto action = LexAction::Advance( new_state_id, precedence_values_for_item_set(new_item_set)); - if (action_takes_precedence(action, - lex_table.state(state_id).default_action)) + if (conflict_manager.resolve(action, lex_table.state(state_id).default_action)) lex_table.state(state_id).actions[rule] = action; } } @@ -107,7 +107,7 @@ class LexTableBuilder { if (item.is_done()) { auto current_action = lex_table.state(state_id).default_action; auto new_action = LexAction::Accept(item.lhs, item.precedence()); - if (action_takes_precedence(new_action, current_action)) + if (conflict_manager.resolve(new_action, current_action)) lex_table.state(state_id).default_action = new_action; } } diff --git a/src/compiler/build_tables/build_parse_table.cc b/src/compiler/build_tables/build_parse_table.cc index 93939125..47b61218 100644 --- a/src/compiler/build_tables/build_parse_table.cc +++ b/src/compiler/build_tables/build_parse_table.cc @@ -7,8 +7,7 @@ #include "compiler/parse_table.h" #include "compiler/build_tables/item_set_closure.h" #include "compiler/build_tables/item_set_transitions.h" -#include "compiler/build_tables/action_takes_precedence.h" -#include "compiler/build_tables/build_conflict.h" +#include "compiler/build_tables/parse_conflict_manager.h" #include "compiler/build_tables/parse_item.h" #include "compiler/lexical_grammar.h" #include "compiler/syntax_grammar.h" @@ -19,6 +18,7 @@ namespace tree_sitter { namespace build_tables { using std::find; +using std::get; using std::pair; using std::vector; using std::set; @@ -30,9 +30,8 @@ using rules::Symbol; class ParseTableBuilder { const SyntaxGrammar grammar; - const LexicalGrammar lex_grammar; + ParseConflictManager conflict_manager; unordered_map parse_state_ids; - vector> productions; vector> item_sets_to_process; ParseTable parse_table; std::set conflicts; @@ -40,7 +39,7 @@ class ParseTableBuilder { public: ParseTableBuilder(const SyntaxGrammar &grammar, const LexicalGrammar &lex_grammar) - : grammar(grammar), lex_grammar(lex_grammar) {} + : grammar(grammar), conflict_manager(grammar, lex_grammar) {} pair build() { auto start_symbol = grammar.rules.empty() @@ -63,7 +62,7 @@ class ParseTableBuilder { if (!conflicts.empty()) return { parse_table, - new GrammarError(GrammarErrorTypeParseConflict, *conflicts.begin()) + new GrammarError(GrammarErrorTypeParseConflict, "Unresolved conflict.\n\n" + *conflicts.begin()) }; } @@ -96,7 +95,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, next_item_set)) { + if (should_add_action(state_id, 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); @@ -114,10 +113,10 @@ class ParseTableBuilder { (item.lhs == rules::START()) ? ParseAction::Accept() : ParseAction::Reduce(item.lhs, item.consumed_symbols.size(), - item.precedence(), get_production_id(item.consumed_symbols)); + item.precedence(), conflict_manager.get_production_id(item.consumed_symbols)); for (const auto &lookahead_sym : lookahead_symbols) - if (should_add_action(state_id, lookahead_sym, action, ParseItemSet())) + if (should_add_action(state_id, lookahead_sym, action)) parse_table.add_action(state_id, lookahead_sym, action); } } @@ -149,8 +148,7 @@ 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, - ParseItemSet())) + if (should_add_action(shift_state_id, lookahead_sym, reduce_extra)) parse_table.add_action(shift_state_id, lookahead_sym, reduce_extra); } } @@ -158,17 +156,16 @@ class ParseTableBuilder { } bool should_add_action(ParseStateId state_id, const Symbol &symbol, - const ParseAction &action, - const ParseItemSet &item_set) { + 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 = action_takes_precedence(action, current_action->second, - symbol); + auto result = conflict_manager.resolve(action, current_action->second, + symbol); - switch (result.second) { + switch (get<1>(result)) { case ConflictTypeResolved: if (action.type == ParseActionTypeReduce) parse_table.fragile_production_ids.insert(action.production_id); @@ -176,13 +173,13 @@ class ParseTableBuilder { parse_table.fragile_production_ids.insert(current_action->second.production_id); break; case ConflictTypeError: - record_conflict(symbol, current_action->second, action, item_set); + conflicts.insert(get<2>(result)); break; default: break; } - return result.first; + return get<0>(result); } set precedence_values_for_item_set(const ParseItemSet &item_set) { @@ -194,28 +191,6 @@ class ParseTableBuilder { } return result; } - - int get_production_id(const vector &symbols) { - auto begin = productions.begin(); - auto end = productions.end(); - auto iter = find(begin, end, symbols); - if (iter == end) { - productions.push_back(symbols); - return productions.size() - 1; - } - return iter - begin; - } - - void record_conflict(const Symbol &sym, const ParseAction &left, - const ParseAction &right, const ParseItemSet &item_set) { - conflicts.insert(build_conflict(left, right, item_set, sym, grammar, lex_grammar)); - } - - vector conflicts_vector() const { - vector result; - result.insert(result.end(), conflicts.begin(), conflicts.end()); - return result; - } }; pair build_parse_table( diff --git a/src/compiler/build_tables/lex_conflict_manager.cc b/src/compiler/build_tables/lex_conflict_manager.cc new file mode 100644 index 00000000..b82951e8 --- /dev/null +++ b/src/compiler/build_tables/lex_conflict_manager.cc @@ -0,0 +1,49 @@ +#include "compiler/build_tables/lex_conflict_manager.h" +#include "compiler/parse_table.h" +#include "compiler/rules/built_in_symbols.h" +#include + +namespace tree_sitter { +namespace build_tables { + +LexConflictManager::LexConflictManager(const LexicalGrammar &grammar) : + grammar(grammar) {} + +bool LexConflictManager::resolve(const LexAction &new_action, const LexAction &old_action) const { + if (new_action.type < old_action.type) + return !resolve(old_action, new_action); + + switch (old_action.type) { + case LexActionTypeError: + return true; + + case LexActionTypeAccept: { + int old_precedence = *old_action.precedence_values.begin(); + + switch (new_action.type) { + case LexActionTypeAccept: { + int new_precedence = *new_action.precedence_values.begin(); + if (new_precedence > old_precedence) + return true; + else if (new_precedence < old_precedence) + return false; + else + return new_action.symbol.index < old_action.symbol.index; + } + + case LexActionTypeAdvance: + return true; + + default: + return false; + } + return true; + } + + default: + return false; + } +} + +} // namespace build_tables +} // namespace tree_sitter diff --git a/src/compiler/build_tables/lex_conflict_manager.h b/src/compiler/build_tables/lex_conflict_manager.h new file mode 100644 index 00000000..cb26b896 --- /dev/null +++ b/src/compiler/build_tables/lex_conflict_manager.h @@ -0,0 +1,24 @@ +#ifndef COMPILER_BUILD_TABLES_LEX_CONFLICT_MANAGER_H_ +#define COMPILER_BUILD_TABLES_LEX_CONFLICT_MANAGER_H_ + +#include "tree_sitter/compiler.h" +#include "compiler/lexical_grammar.h" + +namespace tree_sitter { + +class LexAction; + +namespace build_tables { + +class LexConflictManager { + const LexicalGrammar grammar; + + public: + LexConflictManager(const LexicalGrammar &); + bool resolve(const LexAction &, const LexAction &) const; +}; + +} // namespace build_tables +} // namespace tree_sitter + +#endif // COMPILER_BUILD_TABLES_LEX_CONFLICT_MANAGER_H_ diff --git a/src/compiler/build_tables/parse_conflict_manager.cc b/src/compiler/build_tables/parse_conflict_manager.cc new file mode 100644 index 00000000..f6341a84 --- /dev/null +++ b/src/compiler/build_tables/parse_conflict_manager.cc @@ -0,0 +1,111 @@ +#include "compiler/build_tables/parse_conflict_manager.h" +#include "compiler/parse_table.h" +#include "compiler/rules/built_in_symbols.h" +#include + +namespace tree_sitter { +namespace build_tables { + +using std::get; +using std::make_tuple; +using std::string; +using std::tuple; +using std::vector; + +ParseConflictManager::ParseConflictManager(const SyntaxGrammar &syntax_grammar, + const LexicalGrammar &lexical_grammar) : + syntax_grammar(syntax_grammar), + lexical_grammar(lexical_grammar) {} + +tuple +ParseConflictManager::resolve(const ParseAction &new_action, + const ParseAction &old_action, + const rules::Symbol &symbol) const { + if (new_action.type < old_action.type) { + auto opposite = resolve(old_action, new_action, symbol); + return { !get<0>(opposite), get<1>(opposite), get<2>(opposite) }; + } + + switch (old_action.type) { + case ParseActionTypeError: + return make_tuple(true, ConflictTypeNone, ""); + + case ParseActionTypeShift: + if (new_action.type == ParseActionTypeReduce) { + int min_precedence = *old_action.precedence_values.begin(); + int max_precedence = *old_action.precedence_values.rbegin(); + int new_precedence = *new_action.precedence_values.rbegin(); + if (new_precedence < min_precedence) + return make_tuple(false, ConflictTypeResolved, ""); + else if (new_precedence > max_precedence) + return make_tuple(true, ConflictTypeResolved, ""); + else { + + // TODO: Add associativity annotations. In the event of a precedence + // tie, return ConflictTypeError unless there is an associativity + // annotation to break the tie. + return make_tuple(false, ConflictTypeResolved, ""); + } + } + break; + + case ParseActionTypeReduce: + if (new_action.type == ParseActionTypeReduce) { + int old_precedence = *old_action.precedence_values.begin(); + int new_precedence = *new_action.precedence_values.begin(); + if (new_precedence > old_precedence) { + return make_tuple(true, ConflictTypeResolved, ""); + } else if (new_precedence < old_precedence) { + return make_tuple(false, ConflictTypeResolved, ""); + } else { + string message = + "Lookahead: " + symbol_name(symbol) + "\n" + + "Possible Actions:\n" + "* " + action_description(old_action) + "\n" + + "* " + action_description(new_action) + "\n"; + return make_tuple(false, ConflictTypeError, message); + } + } + + default: + break; + } + + return make_tuple(false, ConflictTypeNone, ""); +} + +size_t ParseConflictManager::get_production_id(const vector &symbols) { + auto begin = productions.begin(); + auto end = productions.end(); + auto iter = find(begin, end, symbols); + if (iter == end) { + productions.push_back(symbols); + return productions.size() - 1; + } + return iter - begin; +} + +string ParseConflictManager::symbol_name(const rules::Symbol &symbol) const { + if (symbol.is_built_in()) { + if (symbol == rules::ERROR()) + return "ERROR"; + else if (symbol == rules::END_OF_INPUT()) + return "END_OF_INPUT"; + else + return ""; + } else if (symbol.is_token()) + return lexical_grammar.rule_name(symbol); + else + return syntax_grammar.rule_name(symbol); +} + +string ParseConflictManager::action_description(const ParseAction &action) const { + string result = "Reduce"; + for (const rules::Symbol &symbol : productions[action.production_id]) + result += " " + symbol_name(symbol); + result += " -> " + symbol_name(action.symbol); + return result; +} + +} // namespace build_tables +} // namespace tree_sitter diff --git a/src/compiler/build_tables/parse_conflict_manager.h b/src/compiler/build_tables/parse_conflict_manager.h new file mode 100644 index 00000000..3917d5d0 --- /dev/null +++ b/src/compiler/build_tables/parse_conflict_manager.h @@ -0,0 +1,44 @@ +#ifndef COMPILER_BUILD_TABLES_PARSE_CONFLICT_MANAGER_H_ +#define COMPILER_BUILD_TABLES_PARSE_CONFLICT_MANAGER_H_ + +#include +#include "tree_sitter/compiler.h" +#include "compiler/syntax_grammar.h" +#include "compiler/lexical_grammar.h" +#include "compiler/build_tables/parse_item.h" + +namespace tree_sitter { + +class ParseAction; +namespace rules { class Symbol; } + +namespace build_tables { + +enum ConflictType { + ConflictTypeNone, + ConflictTypeResolved, + ConflictTypeError +}; + +class ParseConflictManager { + const SyntaxGrammar syntax_grammar; + const LexicalGrammar lexical_grammar; + std::vector> productions; + + public: + ParseConflictManager(const SyntaxGrammar &, const LexicalGrammar &); + + size_t get_production_id(const std::vector &); + + std::tuple + resolve(const ParseAction &, const ParseAction &, const rules::Symbol &) const; + + private: + std::string symbol_name(const rules::Symbol &) const; + std::string action_description(const ParseAction &) const; +}; + +} // namespace build_tables +} // namespace tree_sitter + +#endif // COMPILER_BUILD_TABLES_PARSE_CONFLICT_MANAGER_H_