help-bison
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: %merge confusion


From: Jot Dot
Subject: Re: %merge confusion
Date: Sun, 17 Jan 2021 14:41:55 -0700 (MST)

regarding:

>> I wished Bison would spit out prototypes in the Parser class for each merge
>> routine
>> in the form:
>> 
>> class Parser
>> { ...
>>  ...
>>  YYSTYPE stmtMerge(YYSTYPE y0, YYSTYPE y1); // One for each different merge
>>  routine used
>> };
>> 
>> and we just supply the implementation.
>> 
>> Conceptually, this is how I envision merge routines *should* have been 
>> handled.
>> I would not need any additional parameter as I could access the %parse-param
>> directly
>> whenever I need to. In my case, I only need it in my merge routine.
>> The %merge-param substitutes for this in a "backwards compatible way".
> 
> I agree with you here.  This is something we should do in glr2.cc.  It
> will be a problem for backward compatibility though, so I have to
> think about this carefully.

My original line of thinking was to introduce a "%merge-param" in order to
pass values to merge routines. I now believe this is not a good way of doing 
this.
I can think of several reasons why not to implement this proposed feature.

This can all be solved by the addition of a directive to generate the user's
merge signature in the generated parser class, as shown above. This is a
"simple" way to cover *all the concerns I had* regarding that "%merge-param".

My reasoning is that any parameters needed by a merge routine should simply be
accessed thru the %parse-param values that were set up by the user.
After all, if the merge routines are conceptually thought of as part of the 
parser,
then the optional parse parameters should suffice.

Anything more than that, then I would strongly have to agree with a quote
from you where you mentioned:
> I'm not convinced you actually need this.  I feel your semantic values
> are too "weak".  I mean, I believe what you do need is to use richer
> types for semantic values, into which you can plug that additional
> information.

Too bad that this was not thought of when implementing the directive:
%define api.pure full
as I believe the merge routines technically fall under this directive.
I would have used this as the "switch" to generate the merge signatures
in the parser class. You simply can't add this in now, as existing users'
code would break and need changing.
So maybe some sort of %define merge pure or something similar would suffice.

> Yes, I had not understood that this additional data was your
> %parse-param.  This LGTM.  If I were to make that change soon
> in glr2.cc (make merger members of the parser class), would you
> use a beta to try it?  AFAICT you are still using 3.7.4, and none
> of the betas I provided.

I used your beta which solved the MSVC failure on 26K+ line lengths. That 
worked great.
This was for only one part of my project which I have "frozen" at the moment 
until other
parts of the project have caught up.

While reworking portions to use glr, I thought it was best to use the latest 
official
version (3.7.4) so we wouldn't be chasing version numbers in each of my posts.
I was also happy to work with the existing glr implementation.

I noticed you have done work with api.value.type=union and I could re-code my
stuff to try it. I don't know the state of your modifications so I was
wary to try anything until you gave an official nod.

Right now, I have several project issues that are "doing my head in" so it 
would be
a nice diversion trying out any new feature that is applicable - I don't mind
changing my parse file. I've noticed you've been working with bison for
at least 20+ years. I may not have your skill set, but I would be happy to
contribute by doing a bit extra work on my part - as small as it may be.

Just specify what directives you want me to test out and it doesn't take long 
for
me to rework my files.

Thanks :)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]