bison-patches
[Top][All Lists]
Advanced

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

Re: RFC: lalr1.cc: support move semantics


From: Akim Demaille
Subject: Re: RFC: lalr1.cc: support move semantics
Date: Wed, 12 Sep 2018 06:30:49 +0200

Hi Frank!

Thanks for taking the time to answer with details.

> Le 11 sept. 2018 à 17:21, Frank Heckenbach <address@hidden> a écrit :
> 
> Hi Akim!
> 
>>>>   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.
> 
> Maybe I misunderstood the context. I assumed you meant to write
> 
>  (1)
>  foo (T bar);
> 
> instead of
> 
>  (2)
>  foo (const T & bar);
>  foo (T && bar);

No, that’s indeed what I meant.

> In fact, (1) is good practice in modern C++ (except, of course, for
> constructors which need to be written as (2); assignment operators
> often are too, but don’t strictly need to be).

Yep, I know :)  That’s why I wrote it this way.  I know my Meyers (rip),
Alexandrescu (rip too), Sutter, STL (not the library, the guy) and other
Turners :)

I wrote this in the commit message because the reader might not
know this.

> If you call (1) "foo (std::move (baz));", baz will be moved to the
> bar parameter, and foo might then move that to some internal field,
> so no copy is ever involved, so I wouldn't call this "by copy", if
> that's what you mean. (Of course, you can also call (1) foo by copy,
> in which case a copy constructor will construct the T argument
> before foo is actually called, so I consider this more of a
> caller-side thing.)

I agree this should have been ‘by value’ :)


>>>> +  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.
> 
> I meant you could implement make_unique (which would look mostly
> like the above function, with another template type argument)
> conditional on C++<14, and then use make_unique in the caller
> unconditionally.

The real make_unique has to deal with nasty things, including
whether T{…} or T(…) should be used, and take care of arrays.
I don’t want to go that way, nor give the impression that this
make_unique would be trustworthy.



>> This test actually requires '#define private public'
>> to work...
> 
> Which might be UB:
> https://stackoverflow.com/questions/27778908/define-private-to-public-in-c

So far, so good.  The test suite passes on all the compilers/libs
I have access to.  If needed, I’ll have to add scaffolding in the
parser to allow the test suite to reach the symbols without this
trick.


>>> 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.
> 
> I'm more like auto almost almost always. ;) I'm fine with
> "auto b = false;", but when I have to spell out the type anyway, I
> think the "auto" form just makes it longer for no benefit. But
> that’s just personal opinion.

Since we’re talking about preferences, I’m definitely in favor
of AAA + westconst.  So my favorite style is definitely

const auto x = T{v};

and I wish I could get rid of auto:

const x = T{v};

>> Likewise, I hate 'typedef' with a passion, so I'm very happy to
>> use 'using' everywhere.  But not an option in lalr1.cc. :)
> 
> I only slightly prefer "using", and use it in new code, but haven't
> found a compelling reason to convert "typedef" to "using" in my
> existing code (though it actually should be quite easy to
> semi-automate, and the code requires C++14 anyway).

I have used clang-tidy with success to migrate from typedef to using.
However, some hand tweaks were sometimes needed when it was too
eager and resolved some type aliases in type aliases (i.e., breaking
abstractions by revealing the actual types behinds).




reply via email to

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