Hi Frank!
> Le 10 sept. 2018 à 23:32, Frank Heckenbach <f.heckenb...@fh-soft.de> 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!
