Re: RFC: lalr1.cc: support move semantics

[Akim Demaille]

Hi Frank!

> Le 10 sept. 2018 à 23:32, Frank Heckenbach <address@hidden> a écrit :
> 
> Akim Demaille wrote:
> 
>> This is my proposal to add support for move semantics to Bison.
>> The commit message should be clear enough.  I'd be happy to receive
>> comments/reviews.
>> 
>> I have added an example/variant-11.yy that _requires_ move semantics
>> for some of its semantics values.  Please, check it.
> 
> Just a quick reading (sorry, don’t have much time ATM):

Thanks _a lot_ for your answer!

> 
>>    Symbols (terminal/non terminal) are handled by several functions that used
>>    to take const-refs, which resulted eventually in a copy pushed on the 
>> stack.
>>    With modern C++ (post C++11),
> 
> "Post" could be understood as ">" rather than ">=" as intended.

Wow, I was unaware of this, thanks!

>>    move semantics allows to avoid some of these
>>    copies.
> 
> More precisely, all of them (otherwise move-only types wouldn't
> work).

Right.

>>    That's easy for inner types, when the parser's functions pass arguments to
>>    each other.  Funtions facing the user (make_NUMBER, make_STRING, etc.)
>>    should support both rvalue-refs (for instance to support move-only types:
>>    make_INT (std::make_unique<int> (1))), and lvalue-refs (so that we can 
>> pass
>>    a variable: make_INT (my_int)).  To avoid the multiplication of the
>>    signatures (there is also the location), let's take the argument by copy.
> 
> By copy? Do you mean by value? (Maybe these terms were used
> synonymously pre-C++11, but in the presence of move semantics I
> think it's better to distinguish them clearly; same for the naming
> of YY_COPY.)

I agree with using ‘by value’ (which is more traditional, but is
less clear than ‘by copy’ precisely now that we have ‘by move’, which
is rather called by rvalue-ref, indeed), but really YY_VALUE instead of
YY_COPY looks confusing: it’s not about a semantical value.  And
it matches nicely with YY_MOVE_OR_COPY, imho.  But maybe I’m too
cautious.

>> +  // Koening look up will look into std, since that's an std::vector.
> 
> Koenig (or ADL).

Damn.  Was copied from the origin variant.yy.  But anyway, this is
bad practice, I’ll get rid of it (well, make it a regular function
outside of std).


>> +  template <typename... Args>
>> +  ustring
>> +  make_ustring (Args&&... args)
>> +  {
>> +    // std::make_unique is C++14.
>> +    return std::unique_ptr<std::string>(new 
>> std::string(std::forward<Args>(args)...));
>> +  }
> 
> From that template it's not a far way to a full make_unique<T>
> implementation you could define conditional on C++<14.

I’m running this example with -std=c++11, make_unique is not
an option.

> (And FWIW, not sure if "ustring" is an ideal name for
> unique_ptr<string>. My own code uses "ustring" for Unicode strings,
> as does glib, AFAICS.)

Ok, I’ll using something else.


>> --- a/tests/c++.at
>> +++ b/tests/c++.at
>> @@ -151,8 +151,12 @@ int main()
>> 
>>   // stack_symbol_type: construction, accessor.
>>   {
>> +#if defined __cplusplus && 201103L <= __cplusplus
>> +    auto ss = parser::stack_symbol_type(1, parser::make_INT(123));
>> +#else
>>     parser::symbol_type s = parser::make_INT(123);
>>     parser::stack_symbol_type ss(1, s);
>> +#endif
>>     std::cerr << ss.value.as<int>() << '\n';
>>   }
>> 
>> @@ -161,8 +165,12 @@ int main()
>>     parser::stack_type st;
>>     for (int i = 0; i < 100; ++i)
>>       {
>> +#if defined __cplusplus && 201103L <= __cplusplus
>> +        auto ss = parser::stack_symbol_type(1, parser::make_INT(123));
>> +#else
>>         parser::symbol_type s = parser::make_INT(123);
>>         parser::stack_symbol_type ss(1, s);
>> +#endif
>>         st.push(ss);
>>       }
>>   }
> 
> Wouldn't
> 
>  parser::stack_symbol_type ss(1, parser::make_INT(123));
> 
> work in pre-C++11 too? If it takes a const-ref, a function result
> should be OK, IIRC (haven’t used it for a long time).

No, because I’m using a non-const ref in pre-C++11, so that I
can move the content of the RHS into the LHS.  So I really need
the symbol_type to be an lvalue.


> And then also in C++>11?

No, I have use ‘symbol&’ in pre 11, and ’symbol&&’ in 11+.
These guys are not exposed to the user, it’s private plumbing 
of the parser.  This test actually requires ‘#define private public’
to work…



> I personally don't think "auto x = T (v);"
> adds much compared to "T x = v;" anyway,

I’m addict to auto almost everywhere.  It’s way more consistent.
Likewise, I hate ‘typedef' with a passion, so I’m very happy to
use ‘using' everywhere.  But not an option in lalr1.cc. :)

> and here it would avoid the
> conditionals.

I’ll update my submission in the near future.  Cheers!