From 119c67dd7845cc4a747d2f1f1a39db5bb1a07f6d Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Wed, 2 Aug 2017 15:13:30 -0700
Subject: [PATCH] Fix conflict reporting for shift/reduce conflicts w/ multiple
 reductions

We were failing to rule out shift actions with lower precedence.

Signed-off-by: Philip Turnbull <philipturnbull@github.com>
---
 .../build_tables/build_parse_table.cc         |  7 +++
 .../expected_error.txt                        | 14 +++++
 .../partially_resolved_conflict/grammar.json  | 58 +++++++++++++++++++
 .../partially_resolved_conflict/readme.txt    |  1 +
 4 files changed, 80 insertions(+)
 create mode 100644 test/fixtures/test_grammars/partially_resolved_conflict/expected_error.txt
 create mode 100644 test/fixtures/test_grammars/partially_resolved_conflict/grammar.json
 create mode 100644 test/fixtures/test_grammars/partially_resolved_conflict/readme.txt

diff --git a/src/compiler/build_tables/build_parse_table.cc b/src/compiler/build_tables/build_parse_table.cc
index 00ecba5f..cfb40c2d 100644
--- a/src/compiler/build_tables/build_parse_table.cc
+++ b/src/compiler/build_tables/build_parse_table.cc
@@ -544,6 +544,13 @@ class ParseTableBuilder {
                (shift_precedence.max == reduction_precedence &&
                 shift_precedence.min < reduction_precedence)) {
         entry.actions.pop_back();
+        for (auto item_iter = conflicting_items.begin(); item_iter != conflicting_items.end();) {
+          if (item_iter->is_done()) {
+            ++item_iter;
+          } else {
+            item_iter = conflicting_items.erase(item_iter);
+          }
+        }
       }
 
       // If the shift action has the same precedence as the reduce actions,
diff --git a/test/fixtures/test_grammars/partially_resolved_conflict/expected_error.txt b/test/fixtures/test_grammars/partially_resolved_conflict/expected_error.txt
new file mode 100644
index 00000000..201bdf98
--- /dev/null
+++ b/test/fixtures/test_grammars/partially_resolved_conflict/expected_error.txt
@@ -0,0 +1,14 @@
+Unresolved conflict for symbol sequence:
+
+  '!'  expression  •  '<'  …
+
+Possible interpretations:
+
+  1:  (unary_a  '!'  expression)  •  '<'  …
+  2:  (unary_b  '!'  expression)  •  '<'  …
+
+Possible resolutions:
+
+  1:  Specify a higher precedence in `unary_a` than in the other rules.
+  2:  Specify a higher precedence in `unary_b` than in the other rules.
+  3:  Add a conflict for these rules: `unary_a` `unary_b`
diff --git a/test/fixtures/test_grammars/partially_resolved_conflict/grammar.json b/test/fixtures/test_grammars/partially_resolved_conflict/grammar.json
new file mode 100644
index 00000000..4d64b4eb
--- /dev/null
+++ b/test/fixtures/test_grammars/partially_resolved_conflict/grammar.json
@@ -0,0 +1,58 @@
+{
+  "name": "partially_resolved_conflict",
+
+  "rules": {
+    "expression": {
+      "type": "CHOICE",
+      "members": [
+        {"type": "SYMBOL", "name": "binary"},
+        {"type": "SYMBOL", "name": "identifier"}
+      ]
+    },
+
+    "unary_a": {
+      "type": "PREC",
+      "value": 2,
+      "content": {
+        "type": "SEQ",
+        "members": [
+          {"type": "STRING", "value": "!"},
+          {"type": "SYMBOL", "name": "expression"}
+        ]
+      }
+    },
+
+    "unary_b": {
+      "type": "PREC",
+      "value": 2,
+      "content": {
+        "type": "SEQ",
+        "members": [
+          {"type": "STRING", "value": "!"},
+          {"type": "SYMBOL", "name": "expression"}
+        ]
+      }
+    },
+
+    "binary": {
+      "type": "SEQ",
+      "members": [
+        {
+          "type": "CHOICE",
+          "members": [
+            {"type": "SYMBOL", "name": "unary_a"},
+            {"type": "SYMBOL", "name": "unary_b"},
+            {"type": "SYMBOL", "name": "expression"}
+          ]
+        },
+        {"type": "STRING", "value": "<"},
+        {"type": "SYMBOL", "name": "expression"}
+      ]
+    },
+
+    "identifier": {
+      "type": "PATTERN",
+      "value": "\\a+"
+    }
+  }
+}
\ No newline at end of file
diff --git a/test/fixtures/test_grammars/partially_resolved_conflict/readme.txt b/test/fixtures/test_grammars/partially_resolved_conflict/readme.txt
new file mode 100644
index 00000000..2ec13846
--- /dev/null
+++ b/test/fixtures/test_grammars/partially_resolved_conflict/readme.txt
@@ -0,0 +1 @@
+This grammar has a conflict with three possible actions: a shift in the middle of the `binary` rule and two reductions: one for `unary_a` and one for `unary_b`. Both `unary_a` and `unary_b` have a higher precedence than `binary`, therefore we can rule out the interpretation where a `binary` occurs *inside* of a `unary_a` or `unary_b`, so the error message (and suggested `conflict`) should not include that interpretation.
\ No newline at end of file