[PATCH 5/6] diagnostics: modernize bison's syntax errors

[Akim Demaille]

We used to display the unexpected token first:

    $ bison foo.y
    foo.y:1.8-13: error: syntax error, unexpected %token, expecting character 
literal or identifier or <tag>
        1 | %token %token
          |        ^~~~~~

GCC uses a different format:

    $ gcc-mp-9 foo.c
    foo.c:1:5: error: expected identifier or '(' before ')' token
        1 | int()()()
          |     ^

and so does Clang:

    $ clang-mp-9.0 foo.c
    foo.c:1:5: error: expected identifier or '('
    int()()()
        ^
    1 error generated.

They display the unexpected token last (or not at all).  Also, they
don't waste width with "syntax error".  Let's try that.  It gives, for
the same example as above:

    $ bison foo.y
    foo.y:1.8-13: error: expected character literal or identifier or <tag> 
before %token
        1 | %token %token
          |        ^~~~~~

* src/complain.h, src/complain.c (syntax_error): New.
* src/parse-gram.y (yyreport_syntax_error): Use it.
---
 src/complain.c       | 78 +++++++++++++++++++++++++++++++++++++-------
 src/complain.h       |  5 +++
 src/parse-gram.y     | 53 ++++--------------------------
 tests/diagnostics.at |  2 +-
 tests/input.at       | 18 +++++-----
 5 files changed, 88 insertions(+), 68 deletions(-)

diff --git a/src/complain.c b/src/complain.c
index c8843283..b8ba420e 100644
--- a/src/complain.c
+++ b/src/complain.c
@@ -23,6 +23,7 @@
 #include "system.h"
 
 #include <argmatch.h>
+#include <ctype.h>
 #include <progname.h>
 #include <stdarg.h>
 #include <sys/stat.h>
@@ -263,6 +264,20 @@ severity_prefix (severity s)
 }
 
 
+static void
+severity_print (severity s, FILE *out)
+{
+  if (s != severity_disabled)
+    {
+      const char* style = severity_style (s);
+      begin_use_class (style, out);
+      fprintf (out, "%s:", severity_prefix (s));
+      end_use_class (style, out);
+      fputc (' ', out);
+    }
+}
+
+
 /*-----------.
 | complain.  |
 `-----------*/
@@ -442,16 +457,7 @@ error_message (const location *loc, int *indent, warnings 
flags,
         fprintf (stderr, "%*s", *indent - pos, "");
     }
 
-  const char* style = severity_style (sever);
-
-  if (sever != severity_disabled)
-    {
-      begin_use_class (style, stderr);
-      fprintf (stderr, "%s:", severity_prefix (sever));
-      end_use_class (style, stderr);
-      fputc (' ', stderr);
-    }
-
+  severity_print (sever, stderr);
   vfprintf (stderr, message, args);
   /* Print the type of warning, only if this is not a sub message
      (in which case the prefix is null).  */
@@ -465,7 +471,7 @@ error_message (const location *loc, int *indent, warnings 
flags,
         putc ('\n', stderr);
         flush (stderr);
         if (loc && !(flags & no_caret))
-          location_caret (*loc, style, stderr);
+          location_caret (*loc, severity_style (sever), stderr);
       }
   }
   flush (stderr);
@@ -587,3 +593,53 @@ duplicate_rule_directive (char const *directive,
                    _("previous declaration"));
   fixits_register (&second, "");
 }
+
+void
+syntax_error (location loc,
+              int argc, const char* argv[])
+{
+  if (complaint_status < status_complaint)
+    complaint_status = status_complaint;
+  assert (argc <= 5);
+  const char *format = NULL;
+  switch (argc)
+    {
+# define CASE(N, S)                         \
+      case N:                               \
+        format = S;                         \
+        break
+    default: /* Avoid compiler warnings. */
+      CASE (0, _("syntax error"));
+      CASE (1, _("unexpected %0$s"));
+      CASE (2, _("expected %1$s before %0$s"));
+      CASE (3, _("expected %1$s or %2$s before %0$s"));
+      CASE (4, _("expected %1$s or %2$s or %3$s before %0$s"));
+      CASE (5, _("expected %1$s or %2$s or %4$s or %5$s before %0$s"));
+# undef CASE
+    }
+  location_print (loc, stderr);
+  fputs (": ", stderr);
+  severity_print (severity_error, stderr);
+
+  while (*format)
+    if (format[0] == '%'
+        && isdigit (format[1])
+        && format[2] == '$'
+        && format[3] == 's'
+        && (format[1] - '0') < argc)
+      {
+        int i = format[1] - '0';
+        const char *style = i == 0 ? "unexpected" : "expected";
+        begin_use_class (style, stderr);
+        fputs (argv[i], stderr);
+        end_use_class (style, stderr);
+        format += 4;
+      }
+    else
+      {
+        fputc (*format, stderr);
+        ++format;
+      }
+  fputc ('\n', stderr);
+  location_caret (loc, "error", stderr);
+}
diff --git a/src/complain.h b/src/complain.h
index 9cb6a606..37e652d1 100644
--- a/src/complain.h
+++ b/src/complain.h
@@ -161,6 +161,11 @@ void duplicate_directive (char const *directive,
 void duplicate_rule_directive (char const *directive,
                                location first, location second);
 
+/** Report a syntax error, where argv[0] is the unexpected
+    token, and argv[1...argc] are the expected ones.  */
+void syntax_error (location loc,
+                   int argc, const char* argv[]);
+
 /** 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/parse-gram.y b/src/parse-gram.y
index 3ed07a31..f04bd25d 100644
--- a/src/parse-gram.y
+++ b/src/parse-gram.y
@@ -804,58 +804,17 @@ epilogue.opt:
 int
 yyreport_syntax_error (const yyparse_context_t *ctx)
 {
-  if (complaint_status < status_complaint)
-    complaint_status = status_complaint;
   enum { YYERROR_VERBOSE_ARGS_MAXIMUM = 5 };
-  /* Internationalized format string. */
-  const char *format = YY_NULLPTR;
   /* Arguments of format: reported tokens (one for the "unexpected",
      one per "expected"). */
   int arg[YYERROR_VERBOSE_ARGS_MAXIMUM];
   int n = yysyntax_error_arguments (ctx, arg, YYERROR_VERBOSE_ARGS_MAXIMUM);
-  switch (n)
-    {
-    case -2:
-      return 2;
-# define YYCASE_(N, S)                      \
-      case N:                               \
-        format = S;                         \
-      break
-    default: /* Avoid compiler warnings. */
-      YYCASE_(0, YY_("syntax error"));
-      YYCASE_(1, YY_("syntax error, unexpected %s"));
-      YYCASE_(2, YY_("syntax error, unexpected %s, expecting %s"));
-      YYCASE_(3, YY_("syntax error, unexpected %s, expecting %s or %s"));
-      YYCASE_(4, YY_("syntax error, unexpected %s, expecting %s or %s or %s"));
-      YYCASE_(5, YY_("syntax error, unexpected %s, expecting %s or %s or %s or 
%s"));
-# undef YYCASE_
-    }
-  location_print (*yyparse_context_location (ctx), stderr);
-  fputs (": ", stderr);
-  begin_use_class ("error", stderr);
-  fputs ("error:", stderr);
-  end_use_class ("error", stderr);
-  fputc (' ', stderr);
-  {
-    int i = 0;
-    while (*format)
-      if (format[0] == '%' && format[1] == 's' && i < n)
-        {
-          const char *style = i == 0 ? "unexpected" : "expected";
-          begin_use_class (style, stderr);
-          fputs (yysymbol_name (arg[i]), stderr);
-          end_use_class (style, stderr);
-          format += 2;
-          ++i;
-        }
-      else
-        {
-          fputc (*format, stderr);
-          ++format;
-        }
-  }
-  fputc ('\n', stderr);
-  location_caret (*yyparse_context_location (ctx), "error", stderr);
+  if (n == -2)
+    return 2;
+  const char *argv[YYERROR_VERBOSE_ARGS_MAXIMUM];
+  for (int i = 0; i < n; ++i)
+    argv[i] = yysymbol_name (arg[i]);
+  syntax_error (*yyparse_context_location (ctx), n, argv);
   return 0;
 }
 
diff --git a/tests/diagnostics.at b/tests/diagnostics.at
index d0c7ff4f..425de411 100644
--- a/tests/diagnostics.at
+++ b/tests/diagnostics.at
@@ -273,7 +273,7 @@ AT_TEST([[Carriage return]],
 [[input.y:10.8-11.0: <error>error:</error> missing '"' at end of line
    10 | %token <error>"</error>
       |        <error>^</error>
-input.y:10.8-11.0: <error>error:</error> syntax error, unexpected 
<unexpected>string</unexpected>, expecting <expected>character 
literal</expected> or <expected>identifier</expected> or 
<expected><tag></expected>
+input.y:10.8-11.0: <error>error:</error> expected <expected>character 
literal</expected> or <expected>identifier</expected> or 
<expected><tag></expected> before <unexpected>string</unexpected>
    10 | %token <error>"</error>
       |        <error>^</error>
 ]])
diff --git a/tests/input.at b/tests/input.at
index 0d115e3a..5b37d715 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -102,7 +102,7 @@ input.y:6.1-17: error: invalid directive: 
'%a-does-not-exist'
 input.y:7.1: error: invalid character: '%'
 input.y:7.2: error: invalid character: '-'
 input.y:8.1-9.0: error: missing '%}' at end of file
-input.y:8.1-9.0: error: syntax error, unexpected %{...%}
+input.y:8.1-9.0: error: unexpected %{...%}
 ]])
 
 AT_CLEANUP
@@ -124,7 +124,7 @@ AT_DATA([input.y],
 ]])
 
 AT_BISON_CHECK([input.y], [1], [],
-[[input.y:3.1-15: error: syntax error, unexpected %initial-action, expecting 
{...}
+[[input.y:3.1-15: error: expected {...} before %initial-action
 ]])
 
 AT_CLEANUP
@@ -281,16 +281,16 @@ input.y:3.17-24: error: nonterminals cannot be given a 
string alias
 input.y:4.8-10: error: character literals cannot be nonterminals
     4 | %nterm '+' '*';
       |        ^~~
-input.y:5.8-15: error: syntax error, unexpected string, expecting character 
literal or identifier or <tag>
+input.y:5.8-15: error: expected character literal or identifier or <tag> 
before string
     5 | %nterm "number";
       |        ^~~~~~~~
-input.y:6.8-13: error: syntax error, unexpected string, expecting character 
literal or identifier or <tag>
+input.y:6.8-13: error: expected character literal or identifier or <tag> 
before string
     6 | %token "tok1" 1;
       |        ^~~~~~
-input.y:7.14: error: syntax error, unexpected integer literal
+input.y:7.14: error: unexpected integer literal
     7 | %left "tok2" 2;
       |              ^
-input.y:8.14: error: syntax error, unexpected integer literal
+input.y:8.14: error: unexpected integer literal
     8 | %type "tok3" 3;
       |              ^
 ]])
@@ -1277,7 +1277,7 @@ AT_SETUP([Torturing the Scanner])
 AT_BISON_OPTION_PUSHDEFS
 AT_DATA([input.y], [])
 AT_BISON_CHECK([input.y], [1], [],
-[[input.y:1.1: error: syntax error, unexpected end of file
+[[input.y:1.1: error: unexpected end of file
 ]])
 
 
@@ -1285,7 +1285,7 @@ AT_DATA([input.y],
 [{}
 ])
 AT_BISON_CHECK([-fcaret  input.y], [1], [],
-[[input.y:1.1-2: error: syntax error, unexpected {...}
+[[input.y:1.1-2: error: unexpected {...}
     1 | {}
       | ^~
 ]])
@@ -1655,7 +1655,7 @@ input.y:16.11-17.0: error: missing '"' at end of line
 input.y:19.13-20.0: error: missing '}' at end of file
    19 | %destructor { free ($$)
       |             ^~~~~~~~~~~
-input.y:20.1: error: syntax error, unexpected end of file
+input.y:20.1: error: unexpected end of file
 ]])
 
 AT_CLEANUP
-- 
2.25.0