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: Wed, 30 Dec 2020 02:18:20 -0700 (MST)

recap: me looking for a working glr solution:

>> Alright. I took out the 'api.value.type union' and put in:
>> %union {
>>  gen::index_t index;
>>  merge_t merge;       // A struct to hold my merge info
>> }
>> 
>> All my tokens are of the form: %token <index> tok1 tok2 tok3 ...
>> All my rules are of the form: %type <index> rule1 rule2 rule3 ...
>> 
>> I get the same yyuserMerge as before. This time, it is using
>> the new type of the rule that the merge is in (%type <index> rule)
>> 
>>  case 1: yy0->index = stmtMerge (*yy0, *yy1); break;
> 
> Which is good.  That is what is expected.


I'm confused. This does not work. It can't compile.
The return value for my stmtMerge is what Bison wants: YYSTYPE.
My 'index' is of type 'gen::index_t' (which is unsigned short).

Stripped down, it is saying "assign this union to this unsigned short".

Valid syntax would either be:
  case 1: *yy0 = stmtMerge (*yy0, *yy1); break;
          ^^^^ changed lhs to match rhs
or
  case 1: *yy0->index = stmtMerge (*yy0, *yy1).index; break;
                                              ^^^^^^ must select same field

Since yy0 is YYSTYPE and so is my stmtMerge, the former 'case' is more 
appealing to
me than the latter. (The returned YYSTYPE may contain more info in it than just 
one field.)
For variants, the second example might be better(?)

>>> You did not provide the implementation of your merger, I'd like to see it.
>> 
>> The implementation is completely irrelevant.
> 
> Well, it is irrelevant for the piece of code above, granted.  But it
> is to me who just discovered this feature and would like to know how
> people are using it, so see if we should fix this typing issue of the
> incoming arguments.

Ah, in that case I can explain what I was doing there! I was wondering
about putting in a request for a feature but I feel I am a getting to be
a bit of an annoyance right now and did not want to push the envelope.
This information could easily benefit you or me :)
(me as in "Am I doing something dumb again?")

There is a problem with this merge prototype:
static YYSTYPE stmtMerge(YYSTYPE x0, YYSTYPE x1)

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-parm { something }
just like your %parse-param and %lex-param
And, since that is an optional extension, it would be backwards compatible.

This way I can access my tree I am building without resorting to some
global variable. Otherwise this defeats the purpose of %define api.pure full

Currently, the only other way I can see doing this (which I have done)
is to pass parameters in thru the %union. That way a copy is passed around
and no global variable is used.

So, confusingly (it seems) that my union is merely the normal union you are
expecting plus an additional structure in that union. The first part of that
structure is large enough to contain the "real guts" of the union (that you
expect to see) and then past that point, the parameters that I was hoping to
pass into my merge function.

Expanded:

%union {
    gen::index_t index;
    struct merge_t {
        gen::index_t index; // Largest part of the union excluding this struct
        // Following member(s) never get written over by "normal union variable 
use"
        class MyTree * pResult;
    } merge;       // A struct to hold my merge info
}

Naturally, in my yylex routine I do this:
%code
{
    int yylex(YYSTYPE *plval, YYLTYPE *plloc, Driver * pDriver) {
        plval->merge.pResult = pDriver->getResult();
        return pDriver->getScanner()->get_next_token(*plval, *plloc, pDriver);
}

Now, doing this for every token is a waste of cpu time and also a waste of
space in the union but I couldn't see any other way besides a global.
And that is why I made mention that I'd prefer that a 'merge' to be the
entire union, not just a field:
  case 1: *yy0 = stmtMerge (*yy0, *yy1); break;  // assign union to union

> You are merging two branches that computed an "index", and your merger
> is expected to merge these two indexes into another "index".

Yes. That is what I am doing. I didn't mention the above as I was just
trying to get everything to compile and this had no bearing on it.

> No, the difference is that you have typed your symbols individually.
> The examples in the documentation don't use %token <index>, %type <index>
> etc.

Yes. Kind of what I meant. In order to do a '$$ = ' you have to give the
rule a %type. By using %type, does this make my merge a "typed merger"
as you mentioned??? You said that is currently broken - thus anything
I attempt will fail (in this form) until this gets fixed(?)

In a previous email
> I'd like to fix that, but I'm not sure how to do that in a backward 
> compatible way.

If my above assumption is true, then I can't see where there is any backwards 
compatibility
issue at all. Since "typed mergers" is broken, no one can use it. There is
nothing to break for a "typed merger" case.

FWIW: Currently I am "getting around" this problem by running win_bison which
gets me the offending merge line we spoke about:
  case 1: yy0->index = stmtMerge (*yy0, *yy1); break;
and I manually change it to:
  case 1: *yy0 = stmtMerge (*yy0, *yy1); break;

To me, this seems logical. At least it compiles now and I can test and debug 
everything else.


Due to this long email, let me recap my questions:

Since I'm using %type and %token, am I in this "typed merger" path that is 
being worked on?
You can see what I am attempting. Is there a workable method today that I'm 
just not doing quite right?

Like I said, I don't mind waiting if something is coming in the near future.
I'd even be happy with a future tarball, help test out a fix, or even redo
things to test your upcoming variant stuff.

It wouldn't really be a wait since I have more code to write
and plenty of bugs of my very own to keep me busy :)

Please advise, and thanks for your help




reply via email to

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