Richard Smith via llvm-dev
2018-Sep-16 17:40 UTC
[llvm-dev] [cfe-dev] New warnings when building trunk with GCC 9
Yes, we should produce this warning in C++11 mode too. (I could be misrecalling, but I think the rationale for the current behaviour is based on historical GCC behaviour.) On Sun, 16 Sep 2018, 10:04 David Blaikie via cfe-dev, < cfe-dev at lists.llvm.org> wrote:> Fair point made on that thread - that this is a DR, so technically the > std::move is pessimizing even in C++11 mode. Richard: Any thoughts on > generalizing the warning to cover these cases even in C++11 mode? > > On Sat, Sep 15, 2018 at 2:37 AM Dávid Bolvanský <david.bolvansky at gmail.com> > wrote: > >> There is a new discussion related to -Wredundant-move warning on GCC >> bugzilla. >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87300 >> >> pi 14. 9. 2018 o 9:53 David Blaikie <dblaikie at gmail.com> napísal(a): >> >>> >>> >>> On Fri, Sep 14, 2018 at 12:48 AM Stephan Bergmann <sbergman at redhat.com> >>> wrote: >>> >>>> On 13/09/2018 18:22, David Blaikie via llvm-dev wrote: >>>> > On Thu, Sep 13, 2018 at 12:13 AM Dávid Bolvanský via llvm-dev >>>> > <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >>>> > >>>> /home/davidbolvansky/trunk/llvm/unittests/ExecutionEngine/Orc/CompileOnDemandLayerTest.cpp:79:40: >>>> >>>> > required from here >>>> > >>>> /home/davidbolvansky/trunk/llvm/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h:314:29: >>>> > warning: redundant move in return statement [-Wredundant-move] >>>> > 314 | return std::move(Err); >>>> >>>> Note that the move (into the implicit JITSymbol(Error) ctor) is only >>>> redundant if the compiler implements a fix for >>>> <http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579> >>>> "Return by converting move constructor". (But not sure whether the >>>> LLVM >>>> compiler baselines imply that, anyway. >>> >>> >>> Looks like that was accepted for C++14, not 11. So I don't believe it's >>> valid to remove the std::move in C++11 code. (& I believe Clang's warning >>> correctly diagnoses this only in the right language versions) - so we >>> probably want to disable the buggy GCC warning here, then. >>> >>> >>>> In LibreOffice it forced me to >>>> introduce ugly #ifs, to not have to disable that warning outright, >>>> < >>>> https://cgit.freedesktop.org/libreoffice/core/commit/?id=dc06c8f4989fc28d0c31ebd333e53dfe0e0f5f66> >>>> >>>> "-Werror=redundant-move (GCC 9), take two".) >>>> >>> _______________________________________________ > cfe-dev mailing list > cfe-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180916/bf4d4fcd/attachment.html>
Richard Trieu via llvm-dev
2018-Sep-18 23:07 UTC
[llvm-dev] [cfe-dev] New warnings when building trunk with GCC 9
I'll look into the pessimizing move warning, then after that, the deprecated copy warning. On Sun, Sep 16, 2018 at 10:41 AM Richard Smith <richard at metafoo.co.uk> wrote:> Yes, we should produce this warning in C++11 mode too. (I could be > misrecalling, but I think the rationale for the current behaviour is based > on historical GCC behaviour.) > > On Sun, 16 Sep 2018, 10:04 David Blaikie via cfe-dev, < > cfe-dev at lists.llvm.org> wrote: > >> Fair point made on that thread - that this is a DR, so technically the >> std::move is pessimizing even in C++11 mode. Richard: Any thoughts on >> generalizing the warning to cover these cases even in C++11 mode? >> >> On Sat, Sep 15, 2018 at 2:37 AM Dávid Bolvanský < >> david.bolvansky at gmail.com> wrote: >> >>> There is a new discussion related to -Wredundant-move warning on GCC >>> bugzilla. >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87300 >>> >>> pi 14. 9. 2018 o 9:53 David Blaikie <dblaikie at gmail.com> napísal(a): >>> >>>> >>>> >>>> On Fri, Sep 14, 2018 at 12:48 AM Stephan Bergmann <sbergman at redhat.com> >>>> wrote: >>>> >>>>> On 13/09/2018 18:22, David Blaikie via llvm-dev wrote: >>>>> > On Thu, Sep 13, 2018 at 12:13 AM Dávid Bolvanský via llvm-dev >>>>> > <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >>>>> > >>>>> /home/davidbolvansky/trunk/llvm/unittests/ExecutionEngine/Orc/CompileOnDemandLayerTest.cpp:79:40: >>>>> >>>>> > required from here >>>>> > >>>>> /home/davidbolvansky/trunk/llvm/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h:314:29: >>>>> > warning: redundant move in return statement [-Wredundant-move] >>>>> > 314 | return std::move(Err); >>>>> >>>>> Note that the move (into the implicit JITSymbol(Error) ctor) is only >>>>> redundant if the compiler implements a fix for >>>>> <http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579> >>>>> "Return by converting move constructor". (But not sure whether the >>>>> LLVM >>>>> compiler baselines imply that, anyway. >>>> >>>> >>>> Looks like that was accepted for C++14, not 11. So I don't believe it's >>>> valid to remove the std::move in C++11 code. (& I believe Clang's warning >>>> correctly diagnoses this only in the right language versions) - so we >>>> probably want to disable the buggy GCC warning here, then. >>>> >>>> >>>>> In LibreOffice it forced me to >>>>> introduce ugly #ifs, to not have to disable that warning outright, >>>>> < >>>>> https://cgit.freedesktop.org/libreoffice/core/commit/?id=dc06c8f4989fc28d0c31ebd333e53dfe0e0f5f66> >>>>> >>>>> "-Werror=redundant-move (GCC 9), take two".) >>>>> >>>> _______________________________________________ >> cfe-dev mailing list >> cfe-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180918/36b05af0/attachment.html>
Richard Trieu via llvm-dev
2018-Sep-25 03:40 UTC
[llvm-dev] [cfe-dev] New warnings when building trunk with GCC 9
+ Erik, who implemented DR1579 Originally, I had the warning similar to GCC's warning, but took it out due to not having DR1579 implemented in clang (warning changed in r243594) Erik in r274291 implemented DR1579, although PR27785 didn't mention anything about std::move It looks like what's happening is that Clang and GCC handles the return differently. Clang needs the std::move call to use the move constructor while GCC will use the move constructor with or without the std::move call. This means that the warning is currently correct when running on either compiler. This is a reduced example. Compiled with Clang, it will print "move constructor" then "copy constructor". GCC will print "move constructor" twice. #include <iostream> #include <memory> struct A { A(int) {} A() { std::cout << "empty constructor"; } A(const A&) { std::cout << "copy constructor\n"; } A(A&&) { std::cout << "move constructor\n"; } }; struct B { B(A a) {} }; A getA() { return A(1); } B run1() { A a = getA(); return std::move(a); } B run2() { A a = getA(); return a; } int main() { std::cout << "with move:\n"; run1(); std::cout << "no move:\n"; run2(); } On Tue, Sep 18, 2018 at 4:07 PM Richard Trieu <rtrieu at google.com> wrote:> I'll look into the pessimizing move warning, then after that, the > deprecated copy warning. > > On Sun, Sep 16, 2018 at 10:41 AM Richard Smith <richard at metafoo.co.uk> > wrote: > >> Yes, we should produce this warning in C++11 mode too. (I could be >> misrecalling, but I think the rationale for the current behaviour is based >> on historical GCC behaviour.) >> >> On Sun, 16 Sep 2018, 10:04 David Blaikie via cfe-dev, < >> cfe-dev at lists.llvm.org> wrote: >> >>> Fair point made on that thread - that this is a DR, so technically the >>> std::move is pessimizing even in C++11 mode. Richard: Any thoughts on >>> generalizing the warning to cover these cases even in C++11 mode? >>> >>> On Sat, Sep 15, 2018 at 2:37 AM Dávid Bolvanský < >>> david.bolvansky at gmail.com> wrote: >>> >>>> There is a new discussion related to -Wredundant-move warning on GCC >>>> bugzilla. >>>> >>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87300 >>>> >>>> pi 14. 9. 2018 o 9:53 David Blaikie <dblaikie at gmail.com> napísal(a): >>>> >>>>> >>>>> >>>>> On Fri, Sep 14, 2018 at 12:48 AM Stephan Bergmann <sbergman at redhat.com> >>>>> wrote: >>>>> >>>>>> On 13/09/2018 18:22, David Blaikie via llvm-dev wrote: >>>>>> > On Thu, Sep 13, 2018 at 12:13 AM Dávid Bolvanský via llvm-dev >>>>>> > <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >>>>>> > >>>>>> /home/davidbolvansky/trunk/llvm/unittests/ExecutionEngine/Orc/CompileOnDemandLayerTest.cpp:79:40: >>>>>> >>>>>> > required from here >>>>>> > >>>>>> /home/davidbolvansky/trunk/llvm/include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h:314:29: >>>>>> > warning: redundant move in return statement [-Wredundant-move] >>>>>> > 314 | return std::move(Err); >>>>>> >>>>>> Note that the move (into the implicit JITSymbol(Error) ctor) is only >>>>>> redundant if the compiler implements a fix for >>>>>> <http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579> >>>>>> "Return by converting move constructor". (But not sure whether the >>>>>> LLVM >>>>>> compiler baselines imply that, anyway. >>>>> >>>>> >>>>> Looks like that was accepted for C++14, not 11. So I don't believe >>>>> it's valid to remove the std::move in C++11 code. (& I believe Clang's >>>>> warning correctly diagnoses this only in the right language versions) - so >>>>> we probably want to disable the buggy GCC warning here, then. >>>>> >>>>> >>>>>> In LibreOffice it forced me to >>>>>> introduce ugly #ifs, to not have to disable that warning outright, >>>>>> < >>>>>> https://cgit.freedesktop.org/libreoffice/core/commit/?id=dc06c8f4989fc28d0c31ebd333e53dfe0e0f5f66> >>>>>> >>>>>> "-Werror=redundant-move (GCC 9), take two".) >>>>>> >>>>> _______________________________________________ >>> cfe-dev mailing list >>> cfe-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >>> >>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180924/5cc05048/attachment.html>