Compilation warnings with g++ 9.1.1 #16992

1. 
   laanwj commented at 10:05 am on September 30, 2019: member

   I just compiled with a new g++ version for the first time, and noticed some warnings. I don’t know how serious these are. I don’t think they’re very bad (concerning deprecations mostly):

   • “specified bound 12 equals destination size”

   0In file included from /usr/include/string.h:494,
   1                 from ./serialize.h:20,
   2                 from ./netaddress.h:13,
   3                 from ./protocol.h:13,
   4                 from protocol.cpp:6:
   5In function ‘char* strncpy(char*, const char*, size_t)’,
   6    inlined from ‘CMessageHeader::CMessageHeader(const unsigned char (&)[4], const char*, unsigned int)’ at protocol.cpp:91:12:
   7/usr/include/bits/string_fortified.h:106:34: warning: ‘char* __builtin_strncpy(char*, const char*, long unsigned int)’ specified bound 12 equals destination size [-Wstringop-truncation]
   8  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
   9      |          ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   

   • implicitly-declared ‘PartiallySignedTransaction& PartiallySignedTransaction::operator=(const PartiallySignedTransaction&)’ is deprecated [-Wdeprecated-copy]

   0psbt.cpp: In function ‘TransactionError CombinePSBTs(PartiallySignedTransaction&, const std::vector<PartiallySignedTransaction>&)’:
   1psbt.cpp:335:19: warning: implicitly-declared ‘PartiallySignedTransaction& PartiallySignedTransaction::operator=(const PartiallySignedTransaction&)’ is deprecated [-Wdeprecated-copy]
   2  335 |     out = psbtxs[0]; // Copy the first one
   3      |                   ^
   4In file included from psbt.cpp:5:
   5./psbt.h:398:5: note: because ‘PartiallySignedTransaction’ has user-provided ‘PartiallySignedTransaction::PartiallySignedTransaction(const PartiallySignedTransaction&)’
   6  398 |     PartiallySignedTransaction(const PartiallySignedTransaction& psbt_in) : tx(psbt_in.tx), inputs(psbt_in.inputs), outputs(psbt_in.outputs), unknown(psbt_in.unknown) {}
   7      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~
   

   • warning: redundant move in return statement

   0interfaces/chain.cpp: In member function ‘virtual std::unique_ptr<interfaces::Chain::Lock> interfaces::{anonymous}::ChainImpl::lock(bool)’:
   1interfaces/chain.cpp:249:25: warning: redundant move in return statement [-Wredundant-move]
   2  249 |         return std::move(result);  
   3      |                ~~~~~~~~~^~~~~~~~                   
   4interfaces/chain.cpp:249:25: note: remove ‘std::move’ call
   

2. laanwj added the label Build system on Sep 30, 2019
3. 
   hebasto commented at 1:21 pm on September 30, 2019: member

   Also: #15822
4. 
   ryanofsky commented at 4:10 pm on September 30, 2019: member

   0interfaces/chain.cpp:249:25: warning: redundant move in return statement [-Wredundant-move]
   1  249 |         return std::move(result);  
   2      |                ~~~~~~~~~^~~~~~~~                   
   3interfaces/chain.cpp:249:25: note: remove ‘std::move’ call
   

   std::move here was needed to support older versions of clang, I think including the version used for travis macos builds (https://stackoverflow.com/questions/36752678/clang-returning-stdunique-ptr-with-type-conversion)

   Could maybe use some macro magic to avoid both the clang error without std::move and the gcc warning with std::move if the clang bug is still a concern.

5. 
   laanwj commented at 10:00 am on October 1, 2019: member

   @ryanofsky Thanks for the info! Will try to figure out if this is still necessary, and if so, if we can find an alternative work-around that doesn’t make gcc complain.

   Edit: it is still necessary. And not just for clang. Without the std::move it fails on trusty, xenial and MacOSX.

6. 
   laanwj commented at 11:39 am on October 1, 2019: member

   Libreoffice stumbled on the same problem: https://cgit.freedesktop.org/libreoffice/core/commit/?id=dc06c8f4989fc28d0c31ebd333e53dfe0e0f5f66

   What they did was detect it in the build system:

    0HAVE_CXX_CWG1579_FIX=
    1AC_MSG_CHECKING([whether $CXX has a fix for CWG1579])
    2AC_LANG_PUSH([C++])
    3save_CXXFLAGS=$CXXFLAGS
    4CXXFLAGS="$CXXFLAGS $CXXFLAGS_CXX11"
    5AC_COMPILE_IFELSE([AC_LANG_SOURCE([
    6        #include <memory>
    7        struct S1 {};
    8        struct S2: S1 {};
    9        std::unique_ptr<S1> f() {
   10            std::unique_ptr<S2> s2(new S2);
   11            return s2;
   12        }
   13    ])], [
   14        AC_DEFINE([HAVE_CXX_CWG1579_FIX],[1])
   15        AC_MSG_RESULT([yes])
   16    ], [AC_MSG_RESULT([no])])
   17CXXFLAGS=$save_CXXFLAGS
   18AC_LANG_POP([C++])
   19AC_SUBST([HAVE_CXX_CWG1579_FIX])```
   

   Then use this to determine whether to use std::move for the upcast

   0#if HAVE_CXX_CWG1579_FIX
   1            return pNew;
   2#else
   3            return std::move(pNew);
   4#endif
   

   … not sure if this is worth it for a warning, but FWIW

7. 
   hebasto commented at 11:51 am on October 1, 2019: member

   http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579
8. 
   laanwj commented at 9:46 am on October 3, 2019: member

   Not worth fixing, it bounces from one C++ compiler issue to another (see discussion in #16995).
9. laanwj closed this on Oct 3, 2019

   ---

10. laanwj referenced this in commit f2c416bcf5 on Mar 27, 2020
11. sidhujag referenced this in commit cb17fbc666 on Mar 28, 2020
12. MarcoFalke locked this on Dec 16, 2021

Contributors
laanwj hebasto ryanofsky

Labels
Build system