<div dir="ltr"><div dir="ltr">On Sat, Feb 15, 2020 at 3:42 PM Mehdi AMINI via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:</div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Feb 15, 2020 at 12:13 PM Whisperity via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>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:</div><div><br></div><div>  struct Something {};<br></div><div>  <T> struct CreateFromMoveable {</div><div>    CreateFromMoveable(T&&);</div><div>  };</div><div><br></div><div>  CreateFromMoveable<Something> f() {</div><div>    Something S;</div><div>    // ...</div><div>    return S;</div><div>  }</div><div><br></div><div>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.</div></div></blockquote><div><br></div><div>Seems like this was fixed in gcc5?</div><div><br></div><div><a href="https://godbolt.org/z/zaDyt6" target="_blank">https://godbolt.org/z/zaDyt6</a></div></div></div></div></blockquote><div><br></div><div>Hi Mehdi, Nicolai,</div><div><br></div><div>As the author of P1155, I probably know the most about the tangle that compilers (and WG21) have made of the situation. :)</div><div><br></div><div>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 <i><b>not</b></i> appropriate for codebases that need to compile with other present-day compilers (e.g. Clang).</div><div><br></div><div>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.</div><div><a href="https://godbolt.org/z/Ye9Lnt">https://godbolt.org/z/Ye9Lnt</a><br></div><div><br></div><div>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:</div><div><a href="https://godbolt.org/z/sgrBkc">https://godbolt.org/z/sgrBkc</a><br></div><div><br></div><div>So for the foreseeable future, I would strongly recommend that Nicolai just pass `-Wno-redundant-move` when building with GCC.</div><div><br></div><div>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`.</div><div><br></div><div>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 <i><b>add</b></i> 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 <a href="https://wg21.link/p1155">https://wg21.link/p1155</a> for the field experience report.</div><div><br></div><div>HTH,</div><div>–Arthur</div></div></div>