[cfe-dev] Coding style and warning spam: redundant std::move?

[Arthur O'Dwyer via cfe-dev]

On Sat, Feb 15, 2020 at 3:42 PM Mehdi AMINI via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> On Sat, Feb 15, 2020 at 12:13 PM Whisperity via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> You cannot really *fix* the issue until the minimum required GCC version
>> for the project is bumped because changing "std::move(Err)" to just "Err"
>> may (and most likely will) result in compile errors. The following case:
>>
>>   struct Something {};
>>   <T> struct CreateFromMoveable {
>>     CreateFromMoveable(T&&);
>>   };
>>
>>   CreateFromMoveable<Something> f() {
>>     Something S;
>>     // ...
>>     return S;
>>   }
>>
>> is a hard failure with old GCC, that's why "std::move" is written there.
>> Once the "std::move" is there, old GCC properly picks up the fact that it
>> could bind the move ctor in the return.
>>
>
> Seems like this was fixed in gcc5?
>
> https://godbolt.org/z/zaDyt6
>

Hi Mehdi, Nicolai,

As the author of P1155, I probably know the most about the tangle that
compilers (and WG21) have made of the situation. :)

GCC's `-Wredundant-move` is a super useful warning for people who build
only with GCC trunk and want to migrate off of std::move as quickly as
possible. It is *not* appropriate for codebases that need to compile with
other present-day compilers (e.g. Clang).

For example, here is a case (isomorphic to some of the JSON-related cases
in the above patch, AIUI) where GCC's new warning recommends to remove
`std::move`, but if you do that, you'll get silently worse codegen on Clang
— that is, if you remove `std::move`, the code no longer moves, it does a
copy instead.
https://godbolt.org/z/Ye9Lnt

And here is a case where GCC's new warning recommends to remove
`std::move`, but if you do that, the code stops building on Clang entirely:
https://godbolt.org/z/sgrBkc

So for the foreseeable future, I would strongly recommend that Nicolai just
pass `-Wno-redundant-move` when building with GCC.

What would be super useful, IMO, would be if Nicolai (or anyone) would look
into how to bring Clang into line with GCC (or even all the way into line
with the brand-new C++20 standard, which makes several massive changes in
this area), so that `return x` would hardly ever make a copy of `x`.

I did some work in that area a year or two ago, but my work on
`-Wreturn-std-move` was actually aimed at telling people to *add* std::move
to their returns, in cases such as the above snippets where they were
silently getting copies and probably didn't expect copies. See
https://wg21.link/p1155 for the field experience report.

HTH,
–Arthur
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200215/a23b0c5e/attachment.html>