Re: [PATCH 0/7] Deprecate yysyntax_error_arguments

[Akim Demaille]

> Le 21 mars 2020 à 12:53, Akim Demaille <address@hidden> a écrit :
> 
> This series of commits explores the removal of
> yysyntax_error_arguments, as Adrian suggested
> (https://lists.gnu.org/archive/html/bison-patches/2020-02/msg00069.html).
> I wanted to see in practice what impact it has on the user code, and
> here are a few examples.

I have still not applied these changes, I would be happy to receive
comments.  But there are a few more patches to add.

commit f1eb1e197d7453579f7843315a0aca0a2563ca90
Author: Akim Demaille <address@hidden>
Date:   Thu Mar 26 08:15:35 2020 +0100

    skeletons: make yysyntax_error_arguments a private detail
    
    We could just "inline yysyntax_error_arguments back" in the routines
    it was originally extracted from, but I think the code is nicer to
    read this way.
    
    * data/skeletons/glr.c (yysyntax_error_arguments): Generate only for
    detailed and verbose error messages.
    * data/skeletons/yacc.c: Likewise.
    * data/skeletons/lalr1.cc (parser::context::yysyntax_error_arguments):
    Move as...
    (parser::yysyntax_error_arguments_): this.
    And only for detailed and verbose error messages.

diff --git a/data/skeletons/glr.c b/data/skeletons/glr.c
index aec53417..f6e75502 100644
--- a/data/skeletons/glr.c
+++ b/data/skeletons/glr.c
@@ -2114,13 +2114,39 @@ yyexpected_tokens (const yyGLRStack* yystackp,
           }
     }
   return yycount;
-}
+}]])[
 
+]b4_parse_error_bmatch(
+         [custom],
+[[/* User defined function to report a syntax error.  */
+typedef yyGLRStack yyparse_context_t;
 static int
-yysyntax_error_arguments (const yyGLRStack* yystackp,
-                          int yyarg[], int yyargn) YY_ATTRIBUTE_UNUSED;
+yyreport_syntax_error (const yyGLRStack* yystackp]b4_user_formals[);
+
+/* The token type of the lookahead of this context.  */
+static int
+yyparse_context_token (const yyGLRStack *yystackp) YY_ATTRIBUTE_UNUSED;
 
 static int
+yyparse_context_token (const yyGLRStack *yystackp)
+{
+  YYUSE (yystackp);
+  yySymbol yytoken = yychar == YYEMPTY ? YYEMPTY : YYTRANSLATE (yychar);
+  return yytoken;
+}
+
+]b4_locations_if([[/* The location of the lookahead of this context.  */
+static YYLTYPE *
+yyparse_context_location (const yyGLRStack *yystackp) YY_ATTRIBUTE_UNUSED;
+
+static YYLTYPE *
+yyparse_context_location (const yyGLRStack *yystackp)
+{
+  YYUSE (yystackp);
+  return &yylloc;
+}]])],
+         [detailed\|verbose],
+[[static int
 yysyntax_error_arguments (const yyGLRStack* yystackp,
                           int yyarg[], int yyargn)
 {
@@ -2169,36 +2195,6 @@ yysyntax_error_arguments (const yyGLRStack* yystackp,
 }
 ]])[
 
-]b4_parse_error_case(
-         [custom],
-[[/* User defined function to report a syntax error.  */
-typedef yyGLRStack yyparse_context_t;
-static int
-yyreport_syntax_error (const yyGLRStack* yystackp]b4_user_formals[);
-
-/* The token type of the lookahead of this context.  */
-static int
-yyparse_context_token (const yyGLRStack *yystackp) YY_ATTRIBUTE_UNUSED;
-
-static int
-yyparse_context_token (const yyGLRStack *yystackp)
-{
-  YYUSE (yystackp);
-  yySymbol yytoken = yychar == YYEMPTY ? YYEMPTY : YYTRANSLATE (yychar);
-  return yytoken;
-}
-
-]b4_locations_if([[/* The location of the lookahead of this context.  */
-static YYLTYPE *
-yyparse_context_location (const yyGLRStack *yystackp) YY_ATTRIBUTE_UNUSED;
-
-static YYLTYPE *
-yyparse_context_location (const yyGLRStack *yystackp)
-{
-  YYUSE (yystackp);
-  return &yylloc;
-}]])])[
-
 
 static void
 yyreportSyntaxError (yyGLRStack* yystackp]b4_user_formals[)
diff --git a/data/skeletons/lalr1.cc b/data/skeletons/lalr1.cc
index 75252f6f..27f63d83 100644
--- a/data/skeletons/lalr1.cc
+++ b/data/skeletons/lalr1.cc
@@ -240,6 +240,7 @@ m4_define([b4_shared_declarations],
     {
     public:
       context (const ]b4_parser_class[& yyparser, const symbol_type& yyla);
+      const symbol_type& lookahead () const { return yyla_; }
       int token () const { return yyla_.type_get (); }]b4_locations_if([[
       const location_type& location () const { return yyla_.location; }
 ]])[
@@ -248,8 +249,6 @@ m4_define([b4_shared_declarations],
       /// number of expected tokens (guaranteed to be less than YYNTOKENS).
       int yyexpected_tokens (int yyarg[], int yyargn) const;
 
-      int yysyntax_error_arguments (int yyarg[], int yyargn) const;
-
     private:
       const ]b4_parser_class[& yyparser_;
       const symbol_type& yyla_;
@@ -273,15 +272,19 @@ m4_define([b4_shared_declarations],
 
     /// Stored state numbers (used for stacks).
     typedef ]b4_int_type(0, m4_eval(b4_states_number - 1))[ state_type;
-]b4_parse_error_bmatch([detailed\|verbose], [[
-    /// Generate an error message.
-    /// \param yyctx     the context in which the error occurred.
-    virtual std::string yysyntax_error_ (const context& yyctx) const;
-]])b4_parse_error_bmatch([custom], [[
+]b4_parse_error_bmatch(
+      [custom], [[
     /// Report a syntax error
     /// \param yyctx     the context in which the error occurred.
-    void yyreport_syntax_error (const context& yyctx) const;
-]])[
+    void yyreport_syntax_error (const context& yyctx) const;]],
+      [detailed\|verbose], [[
+    /// The arguments of the error message.
+    int yysyntax_error_arguments_ (const context& yyctx,
+                                   int yyarg[], int yyargn) const;
+
+    /// Generate an error message.
+    /// \param yyctx     the context in which the error occurred.
+    virtual std::string yysyntax_error_ (const context& yyctx) const;]])[
     /// Compute post-reduction state.
     /// \param yystate   the current state
     /// \param yysym     the nonterminal to push on the stack
@@ -1281,49 +1284,7 @@ b4_dollar_popdef])[]dnl
     return yycount;
   }
 
-  int
-  ]b4_parser_class[::context::yysyntax_error_arguments (int yyarg[], int 
yyargn) const
-  {
-    /* There are many possibilities here to consider:
-       - If this state is a consistent state with a default action, then
-         the only way this function was invoked is if the default action
-         is an error action.  In that case, don't check for expected
-         tokens because there are none.
-       - The only way there can be no lookahead present (in yyla) is
-         if this state is a consistent state with a default action.
-         Thus, detecting the absence of a lookahead is sufficient to
-         determine that there is no unexpected or expected token to
-         report.  In that case, just report a simple "syntax error".
-       - Don't assume there isn't a lookahead just because this state is
-         a consistent state with a default action.  There might have
-         been a previous inconsistent state, consistent state with a
-         non-default action, or user semantic action that manipulated
-         yyla.  (However, yyla is currently not documented for 
users.)]b4_lac_if([[
-         In the first two cases, it might appear that the current syntax
-         error should have been detected in the previous state when
-         yy_lac_check was invoked.  However, at that time, there might
-         have been a different syntax error that discarded a different
-         initial context during error recovery, leaving behind the
-         current lookahead.]], [[
-       - Of course, the expected token list depends on states to have
-         correct lookahead information, and it depends on the parser not
-         to perform extra reductions after fetching a lookahead from the
-         scanner and before detecting a syntax error.  Thus, state merging
-         (from LALR or IELR) and default reductions corrupt the expected
-         token list.  However, the list is correct for canonical LR with
-         one exception: it will still contain any token that will not be
-         accepted due to an error action in a later state.]])[
-    */
-
-    if (!yyla_.empty ())
-      {
-        yyarg[0] = yyla_.type_get ();
-        int yyn = yyexpected_tokens (yyarg ? yyarg + 1 : yyarg, yyargn - 1);
-        return yyn + 1;
-      }
-    return 0;
-  }]])b4_lac_if([[
-
+]])b4_lac_if([[
   bool
   ]b4_parser_class[::yy_lac_check_ (int yytoken) const
   {
@@ -1463,6 +1424,50 @@ b4_dollar_popdef])[]dnl
       }
   }]])b4_parse_error_bmatch([detailed\|verbose], [[
 
+  int
+  ]b4_parser_class[::yysyntax_error_arguments_ (const context& yyctx,
+                                                int yyarg[], int yyargn) const
+  {
+    /* There are many possibilities here to consider:
+       - If this state is a consistent state with a default action, then
+         the only way this function was invoked is if the default action
+         is an error action.  In that case, don't check for expected
+         tokens because there are none.
+       - The only way there can be no lookahead present (in yyla) is
+         if this state is a consistent state with a default action.
+         Thus, detecting the absence of a lookahead is sufficient to
+         determine that there is no unexpected or expected token to
+         report.  In that case, just report a simple "syntax error".
+       - Don't assume there isn't a lookahead just because this state is
+         a consistent state with a default action.  There might have
+         been a previous inconsistent state, consistent state with a
+         non-default action, or user semantic action that manipulated
+         yyla.  (However, yyla is currently not documented for 
users.)]b4_lac_if([[
+         In the first two cases, it might appear that the current syntax
+         error should have been detected in the previous state when
+         yy_lac_check was invoked.  However, at that time, there might
+         have been a different syntax error that discarded a different
+         initial context during error recovery, leaving behind the
+         current lookahead.]], [[
+       - Of course, the expected token list depends on states to have
+         correct lookahead information, and it depends on the parser not
+         to perform extra reductions after fetching a lookahead from the
+         scanner and before detecting a syntax error.  Thus, state merging
+         (from LALR or IELR) and default reductions corrupt the expected
+         token list.  However, the list is correct for canonical LR with
+         one exception: it will still contain any token that will not be
+         accepted due to an error action in a later state.]])[
+    */
+
+    if (!yyctx.lookahead ().empty ())
+      {
+        yyarg[0] = yyctx.token ();
+        int yyn = yyctx.yyexpected_tokens (yyarg ? yyarg + 1 : yyarg, yyargn - 
1);
+        return yyn + 1;
+      }
+    return 0;
+  }
+
   // Generate an error message.
   std::string
   ]b4_parser_class[::yysyntax_error_ (const context& yyctx) const
@@ -1471,7 +1476,7 @@ b4_dollar_popdef])[]dnl
     enum { YYARGS_MAX = 5 };
     // Arguments of yyformat.
     int yyarg[YYARGS_MAX];
-    int yycount = yyctx.yysyntax_error_arguments (yyarg, YYARGS_MAX);
+    int yycount = yysyntax_error_arguments_ (yyctx, yyarg, YYARGS_MAX);
 
     char const* yyformat = YY_NULLPTR;
     switch (yycount)
diff --git a/data/skeletons/yacc.c b/data/skeletons/yacc.c
index 11898aa8..14f8caf6 100644
--- a/data/skeletons/yacc.c
+++ b/data/skeletons/yacc.c
@@ -1197,63 +1197,9 @@ yyexpected_tokens (const yyparse_context_t *yyctx,
 {
   return yypstate_expected_tokens (yyctx->yyps, yyarg, yyargn);
 }]])[
-
-static int
-yysyntax_error_arguments (const yyparse_context_t *yyctx,
-                          int yyarg[], int yyargn) YY_ATTRIBUTE_UNUSED;
-
-static int
-yysyntax_error_arguments (const yyparse_context_t *yyctx,
-                          int yyarg[], int yyargn)
-{
-  /* Actual size of YYARG. */
-  int yycount = 0;
-  /* There are many possibilities here to consider:
-     - If this state is a consistent state with a default action, then
-       the only way this function was invoked is if the default action
-       is an error action.  In that case, don't check for expected
-       tokens because there are none.
-     - The only way there can be no lookahead present (in yychar) is if
-       this state is a consistent state with a default action.  Thus,
-       detecting the absence of a lookahead is sufficient to determine
-       that there is no unexpected or expected token to report.  In that
-       case, just report a simple "syntax error".
-     - Don't assume there isn't a lookahead just because this state is a
-       consistent state with a default action.  There might have been a
-       previous inconsistent state, consistent state with a non-default
-       action, or user semantic action that manipulated yychar.]b4_lac_if([[
-       In the first two cases, it might appear that the current syntax
-       error should have been detected in the previous state when yy_lac
-       was invoked.  However, at that time, there might have been a
-       different syntax error that discarded a different initial context
-       during error recovery, leaving behind the current lookahead.]], [[
-     - Of course, the expected token list depends on states to have
-       correct lookahead information, and it depends on the parser not
-       to perform extra reductions after fetching a lookahead from the
-       scanner and before detecting a syntax error.  Thus, state merging
-       (from LALR or IELR) and default reductions corrupt the expected
-       token list.  However, the list is correct for canonical LR with
-       one exception: it will still contain any token that will not be
-       accepted due to an error action in a later state.]])[
-  */
-  if (yyctx->yytoken != YYEMPTY)
-    {
-      int yyn;]b4_lac_if([[
-      YYDPRINTF ((stderr, "Constructing syntax error message\n"));]])[
-      yyarg[yycount++] = yyctx->yytoken;
-      yyn = yyexpected_tokens (yyctx, yyarg ? yyarg + 1 : yyarg, yyargn - 1);
-      if (yyn == -2)
-        return -2;]b4_lac_if([[
-      else if (yyn == 0)
-        YYDPRINTF ((stderr, "No expected tokens.\n"));]])[
-      else
-        yycount += yyn;
-    }
-  return yycount;
-}
 ]])[
 
-]b4_parse_error_case(
+]b4_parse_error_bmatch(
          [custom],
 [[/* The token type of the lookahead of this context.  */
 static int
@@ -1278,8 +1224,7 @@ yyparse_context_location (const yyparse_context_t *yyctx)
 /* User defined function to report a syntax error.  */
 static int
 yyreport_syntax_error (const yyparse_context_t *yyctx]b4_user_formals[);]],
-         [simple],
-[[]],
+         [detailed\|verbose],
 [[#ifndef yystrlen
 # if defined __GLIBC__ && defined _STRING_H
 #  define yystrlen(S) (YY_CAST (YYPTRDIFF_T, strlen (S)))
@@ -1369,6 +1314,56 @@ yytnamerr (char *yyres, const char *yystr)
 #endif
 ]])[
 
+static int
+yysyntax_error_arguments (const yyparse_context_t *yyctx,
+                          int yyarg[], int yyargn)
+{
+  /* Actual size of YYARG. */
+  int yycount = 0;
+  /* There are many possibilities here to consider:
+     - If this state is a consistent state with a default action, then
+       the only way this function was invoked is if the default action
+       is an error action.  In that case, don't check for expected
+       tokens because there are none.
+     - The only way there can be no lookahead present (in yychar) is if
+       this state is a consistent state with a default action.  Thus,
+       detecting the absence of a lookahead is sufficient to determine
+       that there is no unexpected or expected token to report.  In that
+       case, just report a simple "syntax error".
+     - Don't assume there isn't a lookahead just because this state is a
+       consistent state with a default action.  There might have been a
+       previous inconsistent state, consistent state with a non-default
+       action, or user semantic action that manipulated yychar.]b4_lac_if([[
+       In the first two cases, it might appear that the current syntax
+       error should have been detected in the previous state when yy_lac
+       was invoked.  However, at that time, there might have been a
+       different syntax error that discarded a different initial context
+       during error recovery, leaving behind the current lookahead.]], [[
+     - Of course, the expected token list depends on states to have
+       correct lookahead information, and it depends on the parser not
+       to perform extra reductions after fetching a lookahead from the
+       scanner and before detecting a syntax error.  Thus, state merging
+       (from LALR or IELR) and default reductions corrupt the expected
+       token list.  However, the list is correct for canonical LR with
+       one exception: it will still contain any token that will not be
+       accepted due to an error action in a later state.]])[
+  */
+  if (yyctx->yytoken != YYEMPTY)
+    {
+      int yyn;]b4_lac_if([[
+      YYDPRINTF ((stderr, "Constructing syntax error message\n"));]])[
+      yyarg[yycount++] = yyctx->yytoken;
+      yyn = yyexpected_tokens (yyctx, yyarg ? yyarg + 1 : yyarg, yyargn - 1);
+      if (yyn == -2)
+        return -2;]b4_lac_if([[
+      else if (yyn == 0)
+        YYDPRINTF ((stderr, "No expected tokens.\n"));]])[
+      else
+        yycount += yyn;
+    }
+  return yycount;
+}
+
 /* Copy into *YYMSG, which is of size *YYMSG_ALLOC, an error message
    about the unexpected token YYTOKEN for the state stack whose top is
    YYSSP.]b4_lac_if([[  In order to see if a particular token T is a