Re: user destructor for unresolved GLR semantic value

[Joel E. Denny]

On Wed, 24 Aug 2005, Joel E. Denny wrote:

> 1. I modified b4_yydestruct_generate() incorrectly by leaving off the
> yyuser_destructor param name.  In the patch, replace this line:
>
>   +    [[yybool yyuser_destructor]]]))[
>
> with this one:
>
>   +    [[yybool yyuser_destructor], [yyuser_destructor]]]))[

On third thought, this parameter really wasn't necessary.  I should've
taken the obvious and simple approach: pass NULL for yyvaluep when the
destructor shouldn't be called.

> 2. yydestruct() indirectly invokes the user-supplied YYPRINT() regardless
> of the value of yyuser_destructor.  YYPRINT() may access the semantic
> value.  I'm not sure what to do here.  Should the required interface of
> YYPRINT() be changed to accept a boolean parameter similar to my
> yyuser_destructor?  Should YYPRINT() not be used in this case?

It's clear to me now that YYPRINT()'s only purpose is to print the
semantic value.  If there isn't a semantic value, then there's no point in
invoking YYPRINT().  Thus, yysymprint() can just check whether yyvaluep is
NULL to decide whether to invoke YYPRINT().

> 3. Although my patch prevents the potential seg fault, I believe there's
> still a potential memory leak that it does not address.  That is, if
> !yyresolved, the yyGLRState stores a list of yySemanticOption's.  Each
> yySemanticOption has a yystate member.  That yystate is the last state in
> a RHS potentially containing many semantic values.  For this case of an
> error during a split stack, I cannot find where the user destructors are
> called for those semantic values.  Based on the `well-formed' comment at
> `yyreturn:', I wonder if this might be a conscious decision.  I'll
> continue to explore this, but does anyone already know what's going on?

I still haven't addressed this one.

Sorry for so many posts, but I think I now have a much more reasonable
approach.  The following patch replaces all previous patches in this
thread.  The test case was in my first post.

Joel

Index: data/c.m4
===================================================================
RCS file: /cvsroot/bison/bison/data/c.m4,v
retrieving revision 1.30
diff -p -u -r1.30 c.m4
--- data/c.m4   24 Jul 2005 07:24:22 -0000      1.30
+++ data/c.m4   25 Aug 2005 02:44:53 -0000
@@ -365,6 +365,8 @@ m4_define([b4_yydestruct_generate],
     yymsg = "Deleting";
   YY_SYMBOL_PRINT (yymsg, yytype, yyvaluep, yylocationp);

+  if (!yyvaluep)
+    return;
   switch (yytype)
     {
 ]m4_map([b4_symbol_actions], m4_defn([b4_symbol_destructors]))[
@@ -407,7 +409,7 @@ b4_location_if([  (void) yylocationp;
 ])dnl
 [
 # ifdef YYPRINT
-  if (yytype < YYNTOKENS)
+  if (yytype < YYNTOKENS && yyvaluep)
     YYPRINT (yyoutput, yytoknum[yytype], *yyvaluep);
 # endif
   switch (yytype)
Index: data/glr.c
===================================================================
RCS file: /cvsroot/bison/bison/data/glr.c,v
retrieving revision 1.114
diff -p -u -r1.114 glr.c
--- data/glr.c  22 Aug 2005 02:31:10 -0000      1.114
+++ data/glr.c  25 Aug 2005 02:44:53 -0000
@@ -1983,7 +1983,8 @@ yyrecoverSyntaxError (yyGLRStack* yystac
 ]b4_location_if([[      yystack->yyerror_range[1].yystate.yyloc = 
yys->yyloc;]])[
       yydestruct ("Error: popping",
                  yystos[yys->yylrState],
-                 &yys->yysemantics.yysval]b4_location_if([, &yys->yyloc])[);
+                 ( yys->yyresolved ? &yys->yysemantics.yysval : NULL 
)]b4_location_if([,
+                 &yys->yyloc])[);
       yystack->yytops.yystates[0] = yys->yypred;
       yystack->yynextFree -= 1;
       yystack->yyspaceLeft += 1;
@@ -2185,7 +2186,8 @@ b4_syncline(address@hidden@], address@hidden@])])dnl
 ]b4_location_if([[       yystack.yyerror_range[1].yystate.yyloc = yys->yyloc;]]
 )[         yydestruct ("Cleanup: popping",
                        yystos[yys->yylrState],
-                       &yys->yysemantics.yysval]b4_location_if([, 
&yys->yyloc])[);
+                       ( yys->yyresolved ? &yys->yysemantics.yysval : NULL 
)]b4_location_if([,
+                       &yys->yyloc])[);
            yystates[0] = yys->yypred;
            yystack.yynextFree -= 1;
            yystack.yyspaceLeft += 1;