[PATCH] diagnostics: factor and enhance messages about duplicate rule directives

[Akim Demaille]

When reporting a duplicate directive on a rule, point to its first
occurrence:

one.y:11.10-15: error: only one %empty allowed per rule
   %empty {} %empty
             ^^^^^^
one.y:11.3-8: previous declaration
   %empty {} %empty
   ^^^^^^

And consistently discard the second one.

* src/complain.h, src/complain.c (duplicate_directive): New.
* src/reader.c: Use it where appropriate.
* src/symlist.h, src/symlist.c (symbol_list): Add a dprec_location member.
* tests/actions.at: Adjust expected output.
---
 src/complain.c   | 10 ++++++++++
 src/complain.h   |  4 ++++
 src/reader.c     | 28 ++++++++++++++++++++--------
 src/symlist.c    |  2 ++
 src/symlist.h    |  1 +
 tests/actions.at | 11 +++++++----
 6 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/src/complain.c b/src/complain.c
index 44e1b1e..2e87097 100644
--- a/src/complain.c
+++ b/src/complain.c
@@ -365,3 +365,13 @@ deprecated_directive (location const *loc, char const 
*old, char const *upd)
               _("deprecated directive: %s, use %s"),
               quote (old), quote_n (1, upd));
 }
+
+void
+duplicate_directive (char const *directive,
+                     location first, location second)
+{
+  unsigned i = 0;
+  complain (&second, complaint, _("only one %s allowed per rule"), directive);
+  i += SUB_INDENT;
+  complain_indent (&first, complaint, &i, _("previous declaration"));
+}
diff --git a/src/complain.h b/src/complain.h
index a433184..4239bf6 100644
--- a/src/complain.h
+++ b/src/complain.h
@@ -123,6 +123,10 @@ void complain_indent (location const *loc, warnings flags, 
unsigned *indent,
 void deprecated_directive (location const *loc,
                            char const *obsolete, char const *updated);
 
+/** Report a repeated directive for a rule.  */
+void duplicate_directive (char const *directive,
+                          location first, location second);
+
 /** Warnings treated as errors shouldn't stop the execution as regular errors
     should (because due to their nature, it is safe to go on). Thus, there are
     three possible execution statuses.  */
diff --git a/src/reader.c b/src/reader.c
index 5960e24..dbf4e95 100644
--- a/src/reader.c
+++ b/src/reader.c
@@ -442,8 +442,10 @@ grammar_current_rule_prec_set (symbol *precsym, location 
loc)
      token.  */
   symbol_class_set (precsym, token_sym, loc, false);
   if (current_rule->ruleprec)
-    complain (&loc, complaint, _("only one %s allowed per rule"), "%prec");
-  current_rule->ruleprec = precsym;
+    duplicate_directive ("%prec",
+                         current_rule->ruleprec->location, loc);
+  else
+    current_rule->ruleprec = precsym;
 }
 
 /* Set %empty for the current rule. */
@@ -456,7 +458,8 @@ grammar_current_rule_empty_set (location loc)
   if (warning_is_unset (Wempty_rule))
     warning_argmatch ("empty-rule", 0, 0);
   if (current_rule->percent_empty_loc.start.file)
-    complain (&loc, complaint, _("only one %s allowed per rule"), "%empty");
+    duplicate_directive ("%empty",
+                         current_rule->percent_empty_loc, loc);
   else
     current_rule->percent_empty_loc = loc;
 }
@@ -473,8 +476,13 @@ grammar_current_rule_dprec_set (int dprec, location loc)
     complain (&loc, complaint, _("%s must be followed by positive number"),
               "%dprec");
   else if (current_rule->dprec != 0)
-    complain (&loc, complaint, _("only one %s allowed per rule"), "%dprec");
-  current_rule->dprec = dprec;
+    duplicate_directive ("%dprec",
+                         current_rule->dprec_location, loc);
+  else
+    {
+      current_rule->dprec = dprec;
+      current_rule->dprec_location = loc;
+    }
 }
 
 /* Attach a merge function NAME with argument type TYPE to current
@@ -487,9 +495,13 @@ grammar_current_rule_merge_set (uniqstr name, location loc)
     complain (&loc, Wother, _("%s affects only GLR parsers"),
               "%merge");
   if (current_rule->merger != 0)
-    complain (&loc, complaint, _("only one %s allowed per rule"), "%merge");
-  current_rule->merger = get_merge_function (name);
-  current_rule->merger_declaration_location = loc;
+    duplicate_directive ("%merge",
+                         current_rule->merger_declaration_location, loc);
+  else
+    {
+      current_rule->merger = get_merge_function (name);
+      current_rule->merger_declaration_location = loc;
+    }
 }
 
 /* Attach SYM to the current rule.  If needed, move the previous
diff --git a/src/symlist.c b/src/symlist.c
index 005d808..50915c1 100644
--- a/src/symlist.c
+++ b/src/symlist.c
@@ -47,7 +47,9 @@ symbol_list_sym_new (symbol *sym, location loc)
   res->percent_empty_loc = empty_location;
   code_props_none_init (&res->action_props);
   res->dprec = 0;
+  res->dprec_location = empty_location;
   res->merger = 0;
+  res->merger_declaration_location = empty_location;
 
   res->next = NULL;
 
diff --git a/src/symlist.h b/src/symlist.h
index 9b0f117..c369f69 100644
--- a/src/symlist.h
+++ b/src/symlist.h
@@ -83,6 +83,7 @@ typedef struct symbol_list
   location percent_empty_loc;
 
   int dprec;
+  location dprec_location;
   int merger;
   location merger_declaration_location;
 
diff --git a/tests/actions.at b/tests/actions.at
index aeff37c..3021c7a 100644
--- a/tests/actions.at
+++ b/tests/actions.at
@@ -114,14 +114,17 @@ AT_SETUP([Invalid uses of %empty])
 AT_DATA_GRAMMAR([[one.y]],
 [[%%
 exp:
-  %empty %empty {}
+  %empty {} %empty
 ;
 ]])
 
 AT_BISON_CHECK([-fcaret one.y], [1], [],
-[[one.y:11.10-15: error: only one %empty allowed per rule
-   %empty %empty {}
-          ^^^^^^
+[[one.y:11.13-18: error: only one %empty allowed per rule
+   %empty {} %empty
+             ^^^^^^
+one.y:11.3-8: previous declaration
+   %empty {} %empty
+   ^^^^^^
 ]])
 
 AT_DATA_GRAMMAR([[two.y]],
-- 
1.8.1.3