Re: %destructor feedback

[Wolfgang Spraul]

Joel -

> Or perhaps there should be macros named YYRHS_SIZE and YYUSER_DESTRUCTOR.
> Then you could write your own macro to perform cleanup in whatever way you
> like.  For example:
>
>    %{
>      #define CLEANUP {                                \
>        int index;                                     \
>        for (index = 1; index <= YYRHS_SIZE; index+=1) \
>          YYUSER_DESTRUCTOR(index);                    \
>      }
>    %}
>
> Then you could call any of the following:
>
>    CLEANUP; YYABORT;
>    CLEANUP; YYERROR;
>    YYABORT;
>    YYERROR;
>
> So maybe this wouldn't be so bad:
>    if (error) { CLEANUP; YYABORT; }
> What do you think?

If you define YYRHS_SIZE and YYUSER_DESTRUCTOR, why don't you also define 
CLEANUP in the skeleton, as YYCLEANUP?
My biggest 'real' point in all of this was the redundant cleanup code. I had 
hundreds of free_this() free_that() calls and with bison's 2.1 yacc/YYABORT 
behavior I don't need them anymore. Since my code is running in an embedded 
environment, I appreciate every KiB that I can save.
Your CLEANUP macro would put the loop in every action, I'm not sure how well a 
modern compiler would optimize this across a larger switch/case.
I don't understand right now why one would not like to build this 'into' the 
bison skeleton, as just another label, errorlab1_cleanup or something. Maybe 
there are so many labels already you don't want even more?

For bison, I believe the order of importance on this cleanup issue goes like 
this:

1. document cleanup behavior in the manual
2. make cleanup behavior consistent across YYABORT/YYERROR and all skeletons 
(I guess this means _no_ auto cleanup)
3. offer more cleanup flexibility (macros)

Regards,
Wolfgang

On Tuesday 18 October 2005 05:26, you wrote:
> On Tue, 18 Oct 2005, Wolfgang Spraul wrote:
> >>>>    Note that in the future, Bison might also consider that right hand
> >>>>    side members that are not mentioned in the action can be destroyed.
> >>
> >> In other words, if you have an action that does nothing but invoke
> >> YYABORT/YYERROR, clean-up would be automatic.
> >
> > This I think would get messy. Maybe it is possible for bison to tell that
> > $3 and $6 were 'mentioned', and $5 was not, but no, I don't like this.
> > Does 'mentioned' mean only until the point of failure (this would
> > probably be really hard to detect safely), or anywhere in the action?
> > Again, I think this is getting messy.
>
> First, I believe detection of whether a destructor call is needed would be
> per RHS semantic value.  Second, bison would surely not parse the C/C++ to
> see *where* a RHS semantic value is mentioned in an action.  Instead, it
> would simply check whether it is mentioned at all.
>
> For example, let's say you try to destruct $5 in a semantic action
> yourself.  This feature probably wouldn't cause you trouble since the mere
> mention of $5 in your semantic action would tell bison not to touch $5.
>
> I guess it's possible that you may have stored $5 in a data structure in
> some other parser or scanner action.  Then you might try to destruct it
> via that data structure in the current semantic action.  In other words,
> your semantic action might destruct $5 without referencing the string
> `$5'.  In that case, bison wouldn't know to leave it alone, and that would
> lead to a double destructor call. I wonder if anyone ever does this.
> Maybe use of a symbol table could lead to such usage.  Maybe this is such
> an obscure and messy usage that it can be ignored.
>
> Would anyone like to offer corrections on my understanding of this
> proposed feature?
>
> > I think bison 2.1 has a pretty good cleanup behavior now, in all cases.
> > The cases are not well documented, and not consistent. Maybe a
> > compromise would be to offer all four:
> >
> > YYERROR; -> no cleanup
> > YYABORT;  -> no cleanup
>
> Implementing the above consistently in the skeletons would probably only
> be a matter of changing yacc.c's usage of YYABORT.  But, again, I don't
> know about lalr1.cc.  In any case, I see this inconsistency as a bug in
> the current implementation.
>
> > YYERROR_CLEANUP; -> cleanup
> > YYABORT_CLEANUP; -> cleanup
>
> Or perhaps there should be macros named YYRHS_SIZE and YYUSER_DESTRUCTOR.
> Then you could write your own macro to perform cleanup in whatever way you
> like.  For example:
>
>    %{
>      #define CLEANUP {                                \
>        int index;                                     \
>        for (index = 1; index <= YYRHS_SIZE; index+=1) \
>          YYUSER_DESTRUCTOR(index);                    \
>      }
>    %}
>
> Then you could call any of the following:
>
>    CLEANUP; YYABORT;
>    CLEANUP; YYERROR;
>    YYABORT;
>    YYERROR;
>
> It seems like this would be a relatively straight-forward and flexible
> design for both bison users and developers.
>
> > Personally, I am using YYABORT with yacc.c right now, and I'm a happy
> > user of the built-in cleanup. If you change this in bison 2.2, I will
> > either create my own macro, or bring my bison 2.0 cleanup code back.
> > In bison 2.0, my code looked like this:
> >
> > if (error) { free_func1($1); free_func2($2); YYABORT; }
> >
> > in bison 2.1, it now looks like this:
> >
> > if (error) YYABORT;
>
> So maybe this wouldn't be so bad:
>
>    if (error) { CLEANUP; YYABORT; }
>
> What do you think?
>
> > Thanks for your detailed comments, I found them very helpful!
>
> Same.
>
> Joel