Re: Bison C++ mid-rule value lost with variants

[Frank Heckenbach]

Akim Demaille wrote:

> > Le 19 juin 2018 à 00:27, Frank Heckenbach <address@hidden> a écrit :
> > 
> > Akim Demaille wrote:
> > 
> >> Well, you can use $<> wherever you want, in regular actions
> >> too.
> > 
> > And that's just as unsafe and hiding things from Bison. From the
> > rest of your mail I now see you want to get rid of $<> completely in
> > C++. I hadn't gathered this from your previous mail.
> 
> Yes, indeed.  $<> is really too low level a feature when
> it is comes to 'active types' such as we have in C++ compared
> to C.  So we should not strive to have it work for dubious
> cases (a type mismatch between the declared type and the one
> passed to $<>), and allow to not use it (typed mid-rule actions).

I agree with dropping $<>, though not quite for the same reasons.
IMHO, "active types" aren't really the problem (as std::variant or
an equivalent implementation can handle them), but indeed it's too
low-level and error-prone, though that would apply to all skeletons
(but of course, dropping it from C is impossible because of
backward-compatibility).

Declared types on mid-rule actions are safer in the normal case
(and if exotic uses of $<> will stop working, I wouldn't mind -- one
should just declare a more suitable type then, possibly itself a
variant if one does such complex things in the actions).

> I'm willing to discourage the use of $<> in all the outputs,
> but forbid them when there's really no way to support them.

Depends on your definition of "no way" (see above). ;)

> > Though it does help when moving from C to C++ which I did recently
> > (though my code was actually C++ the whole time, I had used the C
> > skeleton before). If I had used $<>, and it was not available in the
> > C++ skeleton, it would have been another hurdle at this point. In my
> > case, I'm talking doubly hypothetically; for other users (as the
> > original report indicates, there are people who use it) it may
> > become relevant, but some kind of porting guide may help.
> 
> That's a good idea.  But I'm not sure what it should cover :)

I saw your discussion with Hans about the calc++ examples. Cleaning
that up as suggested certainly helps here.

Most of my porting work, apart from writing the new skeletons, was
general grammar cleanup and conversion of semantic types from raw
pointers and containers to smart pointers and other RAII classes
(which was my main goal of the port, of course), and changes in the
lexer (dropping flex, but that's another story).

Otherwise, AFAIR, the biggest changes were the different "%token"
and "%type" declarations with types instead of union members (which
made things easier), rewriting the interface to the parser, largely
changing (and in some cases, implementing :) the "%define"s in the
grammar file, and some changes to my Makefiles due to the additional
generated files. -- Actually, all of these changes were large in a
relative sense, but small in an absolute sense, as all those parts
of the code (parser interface, defines, Makefile, etc.) are rather
short (at least in my projects). So they may be more of a
psychological hurdle, since at first glance it all looks completely
different, and since one needs that before one can actually get
started with the new skeleton.

I fear existing projects using the C skeleton differ too widely to
offer a detailed porting guide for all (or most) cases, but some
easy to use examples may help here (see above), if they show the
important features (not too many to avoid confusing users, but not
too few so they're actually useful to many; it's probably a delicate
balance).

> > When I did the coding, std::variant actually simplified things for
> > me (e.g., I could avoid adding move support in Bison's variant
> > implementation), so if I were you, I'd probably use it even if I
> > dopped $<>, but if you want to avoid its small runtime overhead,
> > that seems possible.
> 
> I don't think we can afford to simply drop all C++s pre 17.

As I wrote before, there are std::variant implementations for C++14
(which I'm actually using mostly so far) and I think also C++11
(I haven't tested this).

> You did wonders with your C++17 skeleton, and it would be
> great to port your effort in the current framework.  Would
> you contribute to that?

For the rest of this year I'll be quite busy (as you can see from
this late reply):, but next year I might have some time to work on
Bison.

However, I've ported my bigger parsers to my new skeletons and use
them actively. So I have a solution that works for me, and the
slight overhead of storing both the static (by the skeleton) and
dynamic (by std::variant) types is no issue to me.

But I've come to rely on some of the features I implemented there.
AFAICS, you haven't commented on them yet, so I don't know how you
think about them. If you really object to them (or equivalent
features), I might prefer to keep using my skeletons.

These are especially the following features (described in more
detail in my original announcement):

- "%define extra_header_prefix", to avoid file name conflicts

- std::move internally, to support move-only types

- automatic std::move in actions -- this can be dangerous, so cannot
  be on by default; I do it with "%define api.rhs.access {move}"

- default action (when there's no user action):
  "$$ = std::move ($1);" (easy with std::variant; I think with
  Bison's variant, it requires a generated switch)

- pre-action for empty rules with a user action: $$ is initialized
  to the default value of the correct type. With completely static
  variants, this might even be natural (or may need to call the
  default constructor in a switch), and should be officially
  documented.

  (In contrast, for non-empty rules with a user action, I
  pre-initialize $$ to an invalid variant, so if the user action
  forgets to set $$, a bad_variant_access will happen on access
  which may catch some errors in user actions. This won't be
  possible with static variants, but I can live without that.)

- Not sure if there will be issues WRT "%printer" and "%destructor".
  Since they don't work with dynamic variants, I've forbidden them
  in my skeletons.

  I don't (want to have to) ever use "%destructor" (C++ destructors
  should do the work in all cases), and for printing I use a single
  overloaded function (yy_print_value). This works with dynamic
  variants using std::visit, but I think it should also work with
  "%printer <*>", right? So I hope no problems here (apart from
  adding this one directive in my grammars which I can do then :).

Regards,
Frank