Re: [PATCH] yysyntax_error: make it manage its own memory.

[Joel E. Denny]

On Thu, 10 Sep 2009, Joel E. Denny wrote:

> I'd like to push this patch to master and branch-2.5.  I need it for that 
> lookahead correction implementation I spoke about recently, but I think 
> it's worthwhile on its own as well.  Any objections?

I just took a look at the other skeletons, and I'm now pretty convinced 
this is a step in the right direction.  However, I'll wait a day or so in 
case someone knows a reason yacc.c needs to keep the current design.

> I haven't looked at 
> porting this to the other skeletons yet, but would there be any objections 
> there?

Nevermind.  For C++ and Java, we don't have to worry about string memory 
management, of course.  glr.c's approach is already similar to the patch 
below but it uses longjmp for failures (yuck!) and it doesn't try to hang 
onto whatever memory it previously allocated for messages.

> >From d3334adb83a8b87cfa7528ab184d0dbbe7283858 Mon Sep 17 00:00:00 2001
> From: Joel E. Denny <address@hidden>
> Date: Mon, 7 Sep 2009 22:01:16 -0400
> Subject: [PATCH] yysyntax_error: make it manage its own memory.
> 
> This improves readability, eliminates the need to invoke the
> function twice and thus repeat the lookahead collection, and
> prepares for future extensions that will make those repetitions
> more expensive and that will require additional memory
> management in yysyntax_error.
> * data/yacc.c (yysyntax_error): Add arguments for message
> buffer variables stored in the parser.  Instead of size, return
> status similar to yyparse status but indicating success of
> message creation.  Import memory management code from...
> (yyparse, yypush_parse): ... here.
> ---
>  ChangeLog        |   14 ++
>  data/yacc.c      |  124 +++++-----
>  src/parse-gram.c |  664 
> +++++++++++++++++++++++++++---------------------------
>  src/parse-gram.h |   10 +-
>  4 files changed, 415 insertions(+), 397 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 6c97582..be87163 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,17 @@
> +2009-09-07  Joel E. Denny  <address@hidden>
> +
> +     yysyntax_error: make it manage its own memory.
> +     This improves readability, eliminates the need to invoke the
> +     function twice and thus repeat the lookahead collection, and
> +     prepares for future extensions that will make those repetitions
> +     more expensive and that will require additional memory
> +     management in yysyntax_error.
> +     * data/yacc.c (yysyntax_error): Add arguments for message
> +     buffer variables stored in the parser.  Instead of size, return
> +     status similar to yyparse status but indicating success of
> +     message creation.  Import memory management code from...
> +     (yyparse, yypush_parse): ... here.
> +
>  2009-09-10  Joel E. Denny  <address@hidden>
>  
>       Clean up yacc.c a little.
> diff --git a/data/yacc.c b/data/yacc.c
> index 26c5996..013165b 100644
> --- a/data/yacc.c
> +++ b/data/yacc.c
> @@ -889,20 +889,21 @@ yytnamerr (char *yyres, const char *yystr)
>  }
>  # endif
>  
> -/* Copy into YYRESULT an error message about the unexpected token
> -   YYTOKEN while in state YYSTATE.  Return the number of bytes copied,
> -   including the terminating null byte.  If YYRESULT is null, do not
> -   copy anything; just return the number of bytes that would be
> -   copied.  As a special case, return 0 if an ordinary "syntax error"
> -   message will do.  Return YYSIZE_MAXIMUM if overflow occurs during
> -   size calculation.  */
> -static YYSIZE_T
> -yysyntax_error (char *yyresult, int yystate, int yytoken)
> +/* Copy into *YYMSG, which is of size *YYMSG_ALLOC, an error message
> +   about the unexpected token YYTOKEN while in state YYSTATE.  If *YYMSG
> +   is not large enough for the message, free *YYMSG iff it's not
> +   YYMSGBUF, allocate a new buffer, and update *YYMSG and *YYMSG_ALLOC
> +   accordingly.  Return 0 if *YYMSG was successfully written.  Return 1
> +   if an ordinary "syntax error" message will suffice instead.  Return 2
> +   if memory could not be allocated for the error message.  */
> +static int
> +yysyntax_error (YYSIZE_T *yymsg_alloc, char **yymsg, char *yymsgbuf,
> +                int yystate, int yytoken)
>  {
>    int yyn = yypact[yystate];
>  
>    if (! (YYPACT_NINF < yyn && yyn <= YYLAST))
> -    return 0;
> +    return 1;
>    else
>      {
>        YYSIZE_T yysize0 = yytnamerr (0, yytname[yytoken]);
> @@ -946,7 +947,7 @@ yysyntax_error (char *yyresult, int yystate, int yytoken)
>           yysize = yysize1;
>         }
>  
> -        switch (yycount)
> +      switch (yycount)
>          {
>  #define YYCASE_(N, S)                           \
>            case N:                               \
> @@ -965,28 +966,47 @@ yysyntax_error (char *yyresult, int yystate, int 
> yytoken)
>        yysize = yysize1;
>  
>        if (yysize_overflow)
> -     return YYSIZE_MAXIMUM;
> +        yysize = YYSIZE_MAXIMUM;
>  
> -      if (yyresult)
> -     {
> -       /* Avoid sprintf, as that infringes on the user's name space.
> -          Don't have undefined behavior even if the translation
> -          produced a string with the wrong number of "%s"s.  */
> -       char *yyp = yyresult;
> -       int yyi = 0;
> -       while ((*yyp = *yyformat) != '\0')
> -            if (*yyp == '%' && yyformat[1] == 's' && yyi < yycount)
> -              {
> -                yyp += yytnamerr (yyp, yyarg[yyi++]);
> -                yyformat += 2;
> -              }
> -            else
> -              {
> -                yyp++;
> -                yyformat++;
> -              }
> -     }
> -      return yysize;
> +      if (*yymsg_alloc < yysize && *yymsg_alloc < YYSTACK_ALLOC_MAXIMUM)
> +        {
> +          YYSIZE_T yyalloc = 2 * yysize;
> +          if (! (yysize <= yyalloc && yyalloc <= YYSTACK_ALLOC_MAXIMUM))
> +            yyalloc = YYSTACK_ALLOC_MAXIMUM;
> +          if (*yymsg != yymsgbuf)
> +            YYSTACK_FREE (*yymsg);
> +          *yymsg = (char *) YYSTACK_ALLOC (yyalloc);
> +          if (*yymsg)
> +            *yymsg_alloc = yyalloc;
> +          else
> +            {
> +              *yymsg = yymsgbuf;
> +              *yymsg_alloc = sizeof yymsgbuf;
> +            }
> +        }
> +
> +      if (! (0 < yysize && yysize <= *yymsg_alloc))
> +        return 2;
> +
> +      /* Avoid sprintf, as that infringes on the user's name space.
> +         Don't have undefined behavior even if the translation
> +         produced a string with the wrong number of "%s"s.  */
> +      {
> +        char *yyp = *yymsg;
> +        int yyi = 0;
> +        while ((*yyp = *yyformat) != '\0')
> +          if (*yyp == '%' && yyformat[1] == 's' && yyi < yycount)
> +            {
> +              yyp += yytnamerr (yyp, yyarg[yyi++]);
> +              yyformat += 2;
> +            }
> +          else
> +            {
> +              yyp++;
> +              yyformat++;
> +            }
> +        return 0;
> +      }
>      }
>  }
>  #endif /* YYERROR_VERBOSE */
> @@ -1432,35 +1452,17 @@ yyerrlab:
>        yyerror (]b4_yyerror_args[YY_("syntax error"));
>  #else
>        {
> -     YYSIZE_T yysize = yysyntax_error (0, yystate, yytoken);
> -     if (yymsg_alloc < yysize && yymsg_alloc < YYSTACK_ALLOC_MAXIMUM)
> -       {
> -         YYSIZE_T yyalloc = 2 * yysize;
> -         if (! (yysize <= yyalloc && yyalloc <= YYSTACK_ALLOC_MAXIMUM))
> -           yyalloc = YYSTACK_ALLOC_MAXIMUM;
> -         if (yymsg != yymsgbuf)
> -           YYSTACK_FREE (yymsg);
> -         yymsg = (char *) YYSTACK_ALLOC (yyalloc);
> -         if (yymsg)
> -           yymsg_alloc = yyalloc;
> -         else
> -           {
> -             yymsg = yymsgbuf;
> -             yymsg_alloc = sizeof yymsgbuf;
> -           }
> -       }
> -
> -     if (0 < yysize && yysize <= yymsg_alloc)
> -       {
> -         (void) yysyntax_error (yymsg, yystate, yytoken);
> -         yyerror (]b4_yyerror_args[yymsg);
> -       }
> -     else
> -       {
> -         yyerror (]b4_yyerror_args[YY_("syntax error"));
> -         if (yysize != 0)
> -           goto yyexhaustedlab;
> -       }
> +        int yysyntax_error_result =
> +          yysyntax_error (&yymsg_alloc, &yymsg, yymsgbuf, yystate,
> +                          yytoken);
> +        if (yysyntax_error_result == 0)
> +          yyerror (]b4_yyerror_args[yymsg);
> +        else
> +          {
> +            yyerror (]b4_yyerror_args[YY_("syntax error"));
> +            if (yysyntax_error_result == 2)
> +              goto yyexhaustedlab;
> +          }
>        }
>  #endif
>      }
> 
> 
> 
>