Re: %destructor feedback

[Wolfgang Spraul]

Joel -

I agree with everything you say, and I don't want to repeat all your comments 
and tell you on each one why I agree.
At lot of these design-issues can be cleanly solved _either_ way, I find that 
a clear documentation usually helps the most. If you have a function(char 
*param), and you document that

A) the function _always_ takes ownership of param, whether it succeeds or not

or

B) the function only takes ownership of param if it succeeds, and not if it 
fails

then it doesn't really matter which way you go. In a larger system, you will 
always find both cases anyway. You are also right about the problem of the 
action already having _processed_ (destructed) one parameter, then failing -> 
how can I tell the caller to only free the _other_ parameters -> not at all! 
In this case, it's better that the implementation takes care of all 
parameters, failure or not.
But then there are other users of bison who I believe would appreciate a way 
to error out of an action and have the caller cleanup all params, in fact 
your support for having bison cleanup params if an action _only_ does 
YYERROR/YYABORT shows this.

> >>    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.

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
YYERROR_CLEANUP; -> cleanup
YYABORT_CLEANUP; -> cleanup

The downside here would be that it's a lot of work to get this implemented & 
tested in all skeletons, and it adds more macros. At least from the 
implementation side, adding two more macros probably means no additional 
overhead, since a goto to a slightly different label would probably be 
enough.
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;

Thanks for your detailed comments, I found them very helpful!
Best Regards,
Wolfgang

On Monday 17 October 2005 19:21, you wrote:
> On Mon, 17 Oct 2005, Wolfgang Spraul wrote:
> > the reason I prefer the automatic cleanup is that I generally like
> > 'atomic' behavior of functions (actions in this case). Atomic = either
> > the function/action _completely_ succeeds, or _completely_ fails. If it
> > fails, the resulting state should be as if the function was never called.
> > If you move the responsibility of cleanup when doing YYERROR/YYABORT into
> > the action, you break this atomic design: If the action fails, it must
> > now return in a different state than when it was called, otherwise the
> > right-hand semantic values will be leaked (never cleaned up by anyone).
>
> Correct me if I'm wrong: in your above argument (not the one below),
> you're arguing for a clean interface for the user semantic actions for
> *the sake of the semantic actions' caller*.  That is, the caller would be
> guaranteed this atomic-like behavior: the function/action always has
> either the full intended effect or no effect at all.  Is this right?
>
> Assuming I'm following you, I appreciate your design.  For some code, it
> seems to me your design would make a lot of sense.  However, I have a few
> counter-arguments for bison:
>
> 1. Again, it appears your above argument is mainly about simplifying the
> interface *for the sake of the caller*.  The caller is bison-generated
> code.  In other words, your above (not below) argument does not seem to
> offer advantages for the bison user.
>
> At the moment, it seems that it would actually be easier to rewrite bison
> to consistently follow my approach since it already follows it in most
> cases: YYERROR in GLR or LALR(1), YYABORT in GLR. I'm not familiar enough
> with the C++ skeleton to comment on it.  Akim?
>
> 2. Again, I believe my design is simpler for the user to remember: in all
> cases, you must clean up RHS semantic values in any semantic action you
> provide.  That is, YYABORT and YYERROR are not exceptions to the rule.  I
> discuss whether this is simpler for the user to *implement* below.
>
> 3. In general (not just bison) for the sake of memory management, I often
> find my design easier for both the caller and the called function/action:
> if the called function/action *ever* takes responsibility for memory that
> the caller passes it, it *always* takes responsibility for the memory
> regardless of success/failure.
>
> For the called function/action's code, this often means that, once I hit
> an error condition, I don't have to somehow undo what I may have already
> done within the function/action: freeing some of that memory or passing
> responsibility for it to some other function.  Instead, I just have to
> guarantee that I fully free the rest of that memory before returning the
> error condition.
>
> For the caller's code, this often means I simply have less work to do.  I
> don't have to think about freeing after the function/action returns
> regardless of the status.  I find this to be better encapsulation.
>
> Note my use of the word `often'.  Sometimes your design would actually be
> easier.  I think it depends on the kind of code you're writing.  For
> situations in which my design would be preferable, your design may prove
> very frustrating in the called function/action.  How do I undo passing
> memory management to another function?  Will I have to rewrite my logic in
> some confusing manner?  For situations in which your design would be
> preferable, the worst that can happen with my design (for bison anyway) is
> some additional memory clean-up code.  Or so it seems to me.  What do you
> think?
>
> > On top of this design issue there is a practical advantage: There must be
> > a built-in cleanup anyway, so the manual cleanup of RHS values is
> > redundant, only increases code size for no good reason.
>
> I think this depends on the semantic action.  For example, I have semantic
> actions that don't call YYABORT until after already storing the RHS
> semantic values in a structure that's responsible for their deletion.
> Thus, in such semantic actions, there's less code if I don't have to
> somehow undo this shift in memory responsibility before invoking YYABORT.
>
> I wonder if my design plus the following feature would help alleviate your
>
> redundancy concern:
> > On Monday 17 October 2005 08:34, Joel E. Denny wrote:
> >> I've also noticed that the bison documentation says:
> >>
> >>    Note that in the future, Bison might also consider that right hand
> >>    side members that are not mentioned in the action can be destroyed.
> >>
> >> If this is implemented, I believe that it should be implemented
> >> consistently for all three cases: the user's semantic action completes
> >> successfully, it calls YYABORT, or it calls YYERROR.
>
> In other words, if you have an action that does nothing but invoke
> YYABORT/YYERROR, clean-up would be automatic.
>
> Joel