Re: %merge confusion

[Akim Demaille]

Jot,

> Le 30 déc. 2020 à 10:18, Jot Dot <jotdot@shaw.ca> a écrit :
> 
>>> 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.

No, it is not what it expects.

> 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(?)

Jot, as I have already stated several times, your merger is *typed*,
which is an undocumented feature.  The signature of your merger,
as mentioned several times in my messages, should not be

YYSTYPE stmtMerge(YYSTYPE x0, YYSTYPE x1)

but

gen::index_t stmtMerge(YYSTYPE x0, YYSTYPE x1)

The generated code is right, your merger is wrong.


> 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

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.


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

I don't understand how this could work.  All the other actions will be
dealing with 'index' only, and you will lose all the other members.
If you really want to carry all this information around, the easiest
would be to use merge_t only, not index_t.

You seem to believe that semantic values that come from a merge have
a special treatment in the parser, and can carry "more information"
that regular symbol.  They do not.  You _must_ be consistent and
use a unique typing scheme for "regular" symbols and "merged" symbols.
That's why I don't think you really want

> %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
> }

but simply

> %union {
>    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
> }



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

No, it is not.  You are not returning an index, but a merge_t.


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

No, I did not.  I said it is broken *with api.value.type=union*.  Not
with %union.


Cheers!