parser: improve the error message for symbol class redefinition

[Akim Demaille]

commit 20b07467938cf880a1d30eb30d6e191843a21fec
Author: Akim Demaille <address@hidden>
Date:   Sun Dec 9 08:14:35 2018 +0100

    parser: improve the error message for symbol class redefinition
    
    Currently our error messages include both "symbol redeclared" and
    "symbol redefined", and they mean something different.  This is
    obscure, let's make this clearer.
    
    I think the idea between 'definition' vs. 'declaration' is that in the
    case of the nonterminals, the actual definition is its set of rules,
    so %nterm would be about declaration.  The case of %token is less
    clear.
    
    * src/symtab.c (complain_class_redefined): New.
    (symbol_class_set): Use it.
    Simplify the logic of this function to clearly skip its body when the
    preconditions are not met.
    * tests/input.at (Symbol class redefinition): New.

diff --git a/TODO b/TODO
index 7b16bf64..3236da46 100644
--- a/TODO
+++ b/TODO
@@ -7,6 +7,11 @@ Several features are not available in all the backends.
 - token constructors: Java and C
 
 * Short term
+** consistency
+nonterminal vs non terminal vs non-terminal
+token vs terminal
+redeclaration vs redefinition
+
 ** yacc.c
 Now that ylwrap is fixed, we should include foo.tab.h from foo.tab.c rather
 than duplicating it.
diff --git a/src/symtab.c b/src/symtab.c
index 5a9c7ac6..f6e29584 100644
--- a/src/symtab.c
+++ b/src/symtab.c
@@ -302,6 +302,19 @@ semantic_type_redeclaration (semantic_type *s, const char 
*what, location first,
                    _("previous declaration"));
 }
 
+static void
+complain_class_redeclared (symbol *sym, symbol_class class, location second)
+{
+  unsigned i = 0;
+  complain_indent (&second, complaint, &i,
+                   class == token_sym
+                   ? _("symbol %s redeclared as a token")
+                   : _("symbol %s redeclared as a nonterminal"), sym->tag);
+  i += SUB_INDENT;
+  complain_indent (&sym->location, complaint, &i,
+                   _("previous definition"));
+}
+
 
 void
 symbol_location_as_lhs_set (symbol *sym, location loc)
@@ -430,27 +443,26 @@ symbol_precedence_set (symbol *sym, int prec, assoc a, 
location loc)
 void
 symbol_class_set (symbol *sym, symbol_class class, location loc, bool 
declaring)
 {
-  bool warned = false;
-  if (sym->content->class != unknown_sym && sym->content->class != class)
+  aver (class != unknown_sym);
+  sym_content *s = sym->content;
+  if (s->class != unknown_sym && s->class != class)
+    complain_class_redeclared (sym, class, loc);
+  else
     {
-      complain (&loc, complaint, _("symbol %s redefined"), sym->tag);
-      /* Don't report both "redefined" and "redeclared".  */
-      warned = true;
-    }
+      s->class = class;
 
-  if (class == nterm_sym && sym->content->class != nterm_sym)
-    sym->content->number = nvars++;
-  else if (class == token_sym && sym->content->number == NUMBER_UNDEFINED)
-    sym->content->number = ntokens++;
+      if (class == nterm_sym && s->class != nterm_sym)
+        s->number = nvars++;
+      else if (class == token_sym && s->number == NUMBER_UNDEFINED)
+        s->number = ntokens++;
 
-  sym->content->class = class;
-
-  if (declaring)
-    {
-      if (sym->content->status == declared && !warned)
-        complain (&loc, Wother, _("symbol %s redeclared"), sym->tag);
-      else
-        sym->content->status = declared;
+      if (declaring)
+        {
+          if (s->status == declared)
+            complain (&loc, Wother, _("symbol %s redeclared"), sym->tag);
+          else
+            s->status = declared;
+        }
     }
 }
 
diff --git a/tests/input.at b/tests/input.at
index 5e3cbe3d..28dbe570 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -483,6 +483,45 @@ AT_CHECK_UNUSED_VALUES([1], [1])
 AT_CLEANUP
 
 
+## --------------------------- ##
+## Symbol class redefinition.  ##
+## --------------------------- ##
+
+AT_SETUP([Symbol class redefinition])
+
+AT_DATA([[input.y]],
+[[%token FOO
+%nterm FOO BAR
+%token BAR
+%%
+FOO: BAR
+BAR:
+]])
+
+AT_BISON_CHECK([-fcaret input.y], [1], [],
+[[input.y:2.1-6: warning: deprecated directive, use '%type' [-Wdeprecated]
+ %nterm FOO BAR
+ ^^^^^^
+input.y:2.8-10: error: symbol FOO redeclared as a nonterminal
+ %nterm FOO BAR
+        ^^^
+input.y:1.8-10:     previous definition
+ %token FOO
+        ^^^
+input.y:3.8-10: error: symbol BAR redeclared as a token
+ %token BAR
+        ^^^
+input.y:2.12-14:    previous definition
+ %nterm FOO BAR
+            ^^^
+input.y:5.1-3: error: rule given for FOO, which is a token
+ FOO: BAR
+ ^^^
+]])
+
+AT_CLEANUP
+
+
 ## --------------------------------------------- ##
 ## Default %printer and %destructor redeclared.  ##
 ## --------------------------------------------- ##