help-bison
[Top][All Lists]
Advanced

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

Re: %merge confusion


From: Akim Demaille
Subject: Re: %merge confusion
Date: Thu, 14 Jan 2021 06:54:54 +0100

Jot,

Sorry for not answering faster.  I don't have enough time :(
I hope that meanwhile you managed to get your project move forward.

> Le 1 janv. 2021 à 23:32, Jot Dot <jotdot@shaw.ca> a écrit :
> 
>>> The incoming arguments are ok but there is no parsing context provided.
>>> It would be (to me) a quite useful extension if you added something like:
>>> %merge-param { something }
> 
>> 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.
> 
> That does make lots of sense. But first please understand where I am coming 
> from:
> 
> Look at from this viewpoint: Conceptually I feel the merge routines are really
> an extension of the parser and not just some abstract user code that is run.

I can understand the feeling, but whatever the merger does, it must,
at the end, return its contribution as any regular rule would.
Because that is exactly what "semantical value" means: per-symbol
information carried by the parser.  Whether in the past the symbol
was coming from a "normal" rule, or from a merge is irrelevant.
It is symbol, period.



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


> FWIW: I don't think you realised what I was doing with my merge routine.
> I *am* returning an 'index' type. The struct was only used to "expand"
> the union and thus "the relevant parts in that struct" was clear of the
> "normal" part of the union. so This way I could *pass in* values.
> Names should be changed to make it clearer.
> 
> Rewritten a bit better:
>>> 
>>> %union {
>>>   gen::index_t index;
>>>   struct parse_param_t {
>>>       char filler[sizeof gen::index_t index]; // Set this to the size of 
>>> the union excluding this struct
>>>       // Following member(s) will never get written over by "normal union 
>>> variable use"
>>>       class MyTree * pResult;
>>>   } parse_param;       // A struct to hold *** data passed in***
>>> }

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.

Cheers!


reply via email to

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