bug-bison
[Top][All Lists]
Advanced

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

Re: C++11 move semantics


From: Frank Heckenbach
Subject: Re: C++11 move semantics
Date: Mon, 05 Mar 2018 18:15:19 +0100

Hans Åberg wrote:

> >>> That's what I thought too, but it doesn't seem to (and it wouldn't
> >>> work with move-only types without std::move). It just seems to leave
> >>> $$ unset (i.e. default-constructed) without an explicit action.
> >> 
> >> If you don't have an {...}, one should get $$ = $1.
> > 
> > Should, but apparently doesn't (in C++).
> 
> So then a bug, perhaps not worth to support, so a change in the manual.

Let's see if I can add "$$ = std::move ($1)" as a default action in
my patches. (Not as a pre-action as in C, that would be wrong with
move, only as a default action when no user action is supplied.)

Indeed, it doesn't seem too complicated (patch on top of my previous
patch). Actually the changes in variant.hh are only to make its
move() and destroy() functions a bit more tolerant. With
std::variant instead, this would not be necessary, and the change
would become basically a one-liner.

So I see no reason why it shouldn't be done by default, especially
since in C that's not a Bison quirk, but actually a Yacc feature.

--- variant.hh
+++ ./variant.hh
@@ -179,7 +179,8 @@
     void
     move (self_type& other)
     {
-      build<T> ();]b4_parse_assert_if([
+      if (!yytypeid_)
+        build<T> ();]b4_parse_assert_if([
       YYASSERT (yytypeid_);
       YYASSERT (*yytypeid_ == *other.yytypeid_);])[
       as<T> () = std::move (other.as<T> ());
@@ -201,7 +202,8 @@
     void
     destroy ()
     {
-      as<T> ().~T ();]b4_parse_assert_if([
+      if (yytypeid_)
+        as<T> ().~T ();]b4_parse_assert_if([
       yytypeid_ = YY_NULLPTR;])[
     }
--- lalr1.cc
+++ ./lalr1.cc
@@ -857,7 +857,10 @@
           switch (yyn)
             {
 ]b4_user_actions[
-            default:
+            default:]b4_variant_if([
+              if (yylen)
+                b4_symbol_variant([yylhs.type_get ()], [yylhs.value], [move],
+                                  address@hidden - address@hidden)])[
               break;
             }
         }

> > I don't follow you. I was talking about the make_FOO functions which
> > are usually called from the lexer, no $$ and $k involved.
> 
> I am only concerned with the parser.

The make_FOO functions are generated in the parser (and called from
the lexer, basically must be called, so one cannot ignore them).
But there are other places in the parser that also need std::move.
Actually, you did both of them in your patch in 2015.

> > For parser
> > actions, one needs to write std::move manually, that's understood
> > and it works. There are also places in the parser generated code
> > that don't support moving, but if already the first step (make_FOO)
> > doesn't, how should move-only types work?
> 
> If one ends up writing std::move in user code, that is a poor design anyway. 
> Simplest for to be use std::deque and avoid copying.

That's unrelated. std::vector does all the std::move's it needs
internally.

In user actions, one needs std::move regardless, e.g.
"$$ = std::move ($1)" (like the default action) or something like
"$$ = build_something (std::move ($1), std::move ($2))" with some
user-defined build_something function. That's not poor design; such
code is expected when using move-only types. But again, the user
actions are not the issue, bison can't do anything there anyway
(except for the default action, see above). The required moves
within the parser are the problem.

> > Note, this was still in reply to your initial statement that Bison
> > works with unique_ptr; meanwhile you said it doesn't support move
> > semantics, so the point may be moot now. (I think the
> > misunderstanding stems from you thinking unique_ptr doesn't require
> > move semantics; in fact it's the prototypical move-only type in the
> > standard library.)
> 
> If one adds copying to unique_ptr by the use of a GC, then this
> what I use,

If one adds copying to unique_ptr, it's not unique anymore. Might as
well use a raw pointer then.

GC doesn't call destructors, does it?

> and shared_ptr might be an alternative.

Not for me (in many cases), as I explained.

> I was thinking on the deprecated auto_ptr.

Oh, that! I think it's widely agreed that auto_ptr was a design fail
(just a hack to try to support moving pre-C++11). Too dangerous to
accidentally move, and it must not be used with standard containers:
https://stackoverflow.com/questions/111478/why-is-it-wrong-to-use-stdauto-ptr-with-standard-containers

> It is not difficult to hack the M4 file, but if one writes the
> parser, not actively develops for awhile, and Bison changes it,
> then that may turn out be a hurdle say in a few years time.

Exactly. That's why I hoped to see a maintainer here.

> I do not remember, so you would have to search the list archives. I am just 
> saying there might a risk there.
> 
> > Well, search for what?
> 
> Maybe "variant(s)".

That's a rather general term. I did some searching, found no such
thing. Instead found several other requests for moveable types
(including yours from 2015), so apparently I'm not the only one with
this problem.

> > So the only problems with it are (a) it requires C++11 and
> > (b) there's no maintainer who could include such a patch, even with
> > conditionals for backward-compatibility.
> 
> GCC7 supports C++17, but I do not know what the developers, would they be 
> active, might think.

Neither me. I guess gcc7 (needed for std::variant) is too early now,
but std::move only requires C++11 which has been fully supported by
gcc for a while.

Regards,
Frank



reply via email to

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