build: Suppress -Wdeprecated-copy warnings #18738

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:200422-qt-warn changing 1 files +1 −0
  1. hebasto commented at 3:01 pm on April 22, 2020: member

    Tomorrow, on Apr 23 the Ubuntu 20.04 release is expected. It packaged with Qt 5.12 LTS that has a nasty peculiarity to cause modern compilers, including Clang 10.0 and GCC 9.3, to emit spammy -Wdeprecated-copy warnings (#15822, #18419).

    This PR suppress such warnings temporarily, until the upstream is fixed.

    Here are some affected systems (with system packages):

    • Ubuntu 20.04 LTS + Qt 5.12.8 LTS + { Clang 10.0 | GCC 9.3 }
    • Fedora 32 + Qt 5.13.2 + Clang 10.0

    Reference: QTBUG-75210

    Also see fanquake’s comment.

  2. hebasto commented at 4:17 pm on April 22, 2020: member

    Clang 10.0 Release Notes:

    • -Wextra now enables -Wdeprecated-copy. The warning deprecates move and copy constructors in classes where an explicit destructor is declared. This is for compatibility with GCC 9, and forward looking for a change that’s being considered for C++23. You can disable it with -Wno-deprecated-copy.
  3. DrahtBot added the label Build system on Apr 22, 2020
  4. MarcoFalke commented at 4:31 pm on April 22, 2020: member
    Is there a way to only add this flag when a file is compiled in the src/qt folder?
  5. hebasto commented at 5:29 pm on April 22, 2020: member

    @MarcoFalke

    Is there a way to only add this flag when a file is compiled in the src/qt folder?

    Unfortunately, it does not help. Warnings are emitted from /usr/include/x86_64-linux-gnu/qt5/QtCore/ and /usr/include/x86_64-linux-gnu/qt5/QtWidgets/ dirs.

    It seems @Empact’s 422dcd5b4e2bedefc25f04741ff9718835551dc7 from #14920 or @vasild’s c9376ae34cb9ab9e1634fefb080ecdbe33c2656b from #18149 could help, no?

  6. MarcoFalke commented at 5:34 pm on April 22, 2020: member

    The warnings are emitted when compiling our .cpp files located in src/qt. See for example the full include trace:

     0In file included from qt/rpcconsole.cpp:9:
 1In file included from ./qt/rpcconsole.h:8:
 2In file included from ./qt/guiutil.h:12:
 3In file included from /usr/include/qt5/QtWidgets/QHeaderView:1:
 4In file included from /usr/include/qt5/QtWidgets/qheaderview.h:44:
 5In file included from /usr/include/qt5/QtWidgets/qabstractitemview.h:44:
 6In file included from /usr/include/qt5/QtWidgets/qabstractscrollarea.h:44:
 7In file included from /usr/include/qt5/QtWidgets/qframe.h:44:
 8In file included from /usr/include/qt5/QtWidgets/qwidget.h:45:
 9In file included from /usr/include/qt5/QtCore/qobject.h:47:
10/usr/include/qt5/QtCore/qstring.h:1061:22: warning: definition of implicit copy constructor for 'QCharRef' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy]
11    inline QCharRef &operator=(const QCharRef &c) { return operator=(QChar(c)); }
12                     ^
13/usr/include/qt5/QtCore/qstring.h:1161:28: note: in implicit copy constructor for 'QCharRef' first required here
14{ Q_ASSERT(i >= 0); return QCharRef(*this, i); }
15                           ^
162 warnings generated.

    So compiling (in this example rcpconsole.cpp) with -Wno-deprecated-copy would silence the warning (and any other related warning, but only for the gui). Other Bitcoin Core files can be compiled normally.

  7. hebasto commented at 5:38 pm on April 22, 2020: member
    @MarcoFalke mind looking into https://github.com/hebasto/bitcoin/commits/pr18738.02 ?
  8. MarcoFalke commented at 5:43 pm on April 22, 2020: member

    @MarcoFalke mind looking into https://github.com/hebasto/bitcoin/commits/pr18738.02 ?

    Concept ACK

  9. hebasto commented at 5:52 pm on April 22, 2020: member

    @MarcoFalke mind looking into https://github.com/hebasto/bitcoin/commits/pr18738.02 ?

    Concept ACK

    But it does not work (comment) :confused:

    UPDATE: I found bugs in my solution :)

  10. hebasto force-pushed on Apr 22, 2020
  11. hebasto commented at 6:03 pm on April 22, 2020: member

    Updated 0025d098fc4b0f233423ed3b59dbbcd095b62ff8 -> 710a07207d484f7b6ba1e1dc35222d5964d230ee (pr18738.01 -> pr18738.03, diff):

    Is there a way to only add this flag when a file is compiled in the src/qt folder?

  12. in configure.ac:358 in 710a07207d outdated 
    354@@ -355,6 +355,8 @@ if test "x$CXXFLAGS_overridden" = "xno"; then
355   AX_CHECK_COMPILE_FLAG([-Wunused-local-typedef],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-local-typedef"],,[[$CXXFLAG_WERROR]])
356   AX_CHECK_COMPILE_FLAG([-Wdeprecated-register],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-deprecated-register"],,[[$CXXFLAG_WERROR]])
357   AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-implicit-fallthrough"],,[[$CXXFLAG_WERROR]])
358+  dnl Temporary workaround for QTBUG-75210 on Qt 5.12 until it is fixed.
    MarcoFalke commented at 6:24 pm on April 22, 2020:
    This bug is also in qt 5.13.2 (not sure if it is tracked)
    hebasto commented at 6:30 pm on April 22, 2020:
    The patch is merged to Qt 5.13 tree though.
    hebasto commented at 6:32 pm on April 22, 2020:
    Will removing of mentioning of “Qt 5.12” be ok?
    MarcoFalke commented at 6:37 pm on April 22, 2020:
    I was testing on Fedora 32, which comes with qt 5.13 and the issue is still there
    hebasto commented at 10:21 pm on April 22, 2020:

    I was testing on Fedora 32, which comes with qt 5.13 and the issue is still there

    Confirm. OP has been updated.

  13. DrahtBot commented at 7:40 pm on April 22, 2020: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    No conflicts as of last run.

  14. hebasto force-pushed on Apr 22, 2020
  15. hebasto commented at 10:30 pm on April 22, 2020: member

    Updated 710a07207d484f7b6ba1e1dc35222d5964d230ee -> d69174ca43e0bc8028ec2012b8787f0f6ba2f970 (pr18738.03 -> pr18738.04, diff):

    This bug is also in qt 5.13.2 (not sure if it is tracked)

  16. vasild commented at 7:58 am on April 23, 2020: member

    Is there a way to only add this flag when a file is compiled in the src/qt folder?

    Yes, we can compile src/qt/foo.cpp with less strict warnings. However this would be imperfect and we can do better.

    Why imperfect? Because it would also suppress warnings from our code that is in src/qt/foo.cpp. Ideally we want to always see all warnings from our code. We may also want to see warnings from external headers, so that we are at least aware of some problem (in e.g. boost or qt), or maybe even try to fix it upstream. However we may also want to suppress warnings from external headers if they clutter the compilation output and make our warnings harder to spot, or if we want to enable -Werror[=...] on our code, but don’t want the build to break due to warnings from external headers.

    How can we do better? We can tell the compiler to suppress warnings from headers located in a particular directory:

    It seems @Empact’s 422dcd5 from #14920 or @vasild’s c9376ae from #18149 could help, no?

    Those two commits are actually the same thing. I will submit it as a separate PR, isolated from other changes contained in the above PRs.

  17. vasild commented at 8:18 pm on April 23, 2020: member

    I will submit it as a separate PR

    Here it is: https://github.com/bitcoin/bitcoin/pull/18750

  18. fanquake commented at 1:45 pm on May 2, 2020: member

    I see -Wdeprecated-copy warnings building just bitcoind with LLVM Clang 10.0.0 on macOS i.e:

     0make src/bitcoind -j8
 1...
 2In file included from addrman.cpp:6:
 3In file included from ./addrman.h:15:
 4In file included from ./util/system.h:20:
 5In file included from ./fs.h:14:
 6In file included from /usr/local/include/boost/filesystem.hpp:16:
 7In file included from /usr/local/include/boost/filesystem/path.hpp:32:
 8/usr/local/include/boost/io/detail/quoted_manip.hpp:62:23: warning: definition of implicit copy constructor for 'quoted_proxy<const std::__1::basic_string<char> &, char>' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy]
 9        quoted_proxy& operator=(const quoted_proxy&);  // = deleted
10                      ^
11/usr/local/include/boost/io/detail/quoted_manip.hpp:166:14: note: in implicit copy constructor for 'boost::io::detail::quoted_proxy<const std::__1::basic_string<char> &, char>' first required here
12      return detail::quoted_proxy<std::basic_string<Char, Traits, Alloc> const &, Char>
13             ^
14/usr/local/include/boost/filesystem/path.hpp:827:21: note: in instantiation of function template specialization 'boost::io::quoted<char, std::__1::char_traits<char>, std::__1::allocator<char> >' requested here
15      << boost::io::quoted(p.template string<std::basic_string<Char> >(), static_cast<Char>('&'));
16                    ^
17./tinyformat.h:358:13: note: in instantiation of function template specialization 'boost::filesystem::operator<<<char, std::__1::char_traits<char> >' requested here
18        out << value;
19            ^
20./tinyformat.h:543:13: note: in instantiation of function template specialization 'tinyformat::formatValue<boost::filesystem::path>' requested here
21            formatValue(out, fmtBegin, fmtEnd, ntrunc, *static_cast<const T*>(value));
22            ^
23./tinyformat.h:519:27: note: in instantiation of function template specialization 'tinyformat::detail::FormatArg::formatImpl<boost::filesystem::path>' requested here
24            m_formatImpl(&formatImpl<T>),
25                          ^
26./tinyformat.h:975:32: note: (skipping 1 context in backtrace; use -ftemplate-backtrace-limit=0 to see all)
27            m_formatterStore { FormatArg(args)... }
28                               ^
29./tinyformat.h:1028:12: note: in instantiation of function template specialization 'tinyformat::detail::FormatListN<2>::FormatListN<boost::filesystem::path, int>' requested here
30    return detail::FormatListN<sizeof...(args)>(args...);
31           ^
32./tinyformat.h:1064:23: note: in instantiation of function template specialization 'tinyformat::makeFormatList<boost::filesystem::path, int>' requested here
33    vformat(out, fmt, makeFormatList(args...));
34                      ^
35./tinyformat.h:1073:5: note: in instantiation of function template specialization 'tinyformat::format<boost::filesystem::path, int>' requested here
36    format(oss, fmt, args...);
37    ^
38./logging.h:169:28: note: in instantiation of function template specialization 'tinyformat::format<boost::filesystem::path, int>' requested here
39            log_msg = tfm::format(fmt, args...);
40                           ^
41addrman.cpp:638:5: note: in instantiation of function template specialization 'LogPrintf<boost::filesystem::path, int>' requested here
42    LogPrintf("Opened asmap file %s (%d bytes) from disk\n", path, length);
43    ^
441 warning generated.

    So if we are going to suppress these I don’t think it should be qt specific.

    -Wdeprecated-copy warnings.

  19. hebasto commented at 6:39 am on May 3, 2020: member
    @fanquake I can see these boost-1.72.0-specific warnings too.
  20. MarcoFalke commented at 3:44 pm on May 4, 2020: member

    So it seems this warning is not yet ready for general use. I guess we can disable it for 0.21.0.

    Hopefully removing boost in 0.22.0 will also make the warnings go away and we can turn it back on.

  21. hebasto force-pushed on May 5, 2020
  22. hebasto commented at 3:19 am on May 5, 2020: member

    Reverted back d69174ca43e0bc8028ec2012b8787f0f6ba2f970 -> 0025d098fc4b0f233423ed3b59dbbcd095b62ff8 (pr18738.04 -> pr18738.01):

    • addressed:

      • @fanquake’s comment:

        So if we are going to suppress these I don’t think it should be qt specific.

      • @MarcoFalke’s comment:

        So it seems this warning is not yet ready for general use. I guess we can disable it for 0.21.0.

  23. hebasto renamed this:
    build: Suppress Qt 5.12 warnings on Ubuntu Focal
    build: Suppress -Wdeprecated-copy warnings
    on May 5, 2020
  24. fanquake commented at 3:20 am on May 5, 2020: member
    The commit message needs updating as well.
  25. build: Suppress -Wdeprecated-copy warnings 0c63f80854
  26. hebasto force-pushed on May 5, 2020
  27. hebasto commented at 3:26 am on May 5, 2020: member

    Updated 0025d098fc4b0f233423ed3b59dbbcd095b62ff8 -> 0c63f808542ba02fc41aa90b1d96e9123f16d8ad (pr18738.01 -> pr18738.05, diff):

    The commit message needs updating as well.

  28. MarcoFalke commented at 11:35 am on May 5, 2020: member
    ACK 0c63f808542ba02fc41aa90b1d96e9123f16d8ad seems fine to disable this warning for the 0.21.0 release temporarily and then enable it for 0.22.0, when boost is removed.
  29. hebasto commented at 6:46 am on May 13, 2020: member
    @MarcoFalke Is it worth backporting to 0.20 branch before rc2 for neat user experience on Ubuntu Focal?
  30. MarcoFalke commented at 10:50 am on May 13, 2020: member
    cc @fanquake ^
  31. fanquake approved
  32. fanquake commented at 1:07 pm on May 13, 2020: member
    ACK 0c63f808542ba02fc41aa90b1d96e9123f16d8ad - I think it’s ok to suppress these for now, given that -Wdeprecated-copy is enabled (via -Wextra) in GCC 9 and Clang 10. The Qt output is pretty noisy, and there’s a few warnings from Boost as well.
  33. fanquake merged this on May 13, 2020
  34. fanquake closed this on May 13, 2020
  35. hebasto commented at 1:19 pm on May 13, 2020: member
    Backport to 0.20?
  36. hebasto deleted the branch on May 13, 2020
  37. sidhujag referenced this in commit 700181dce0 on May 14, 2020
  38. ComputerCraftr referenced this in commit f89a3d5ca9 on Jun 10, 2020
  39. furszy referenced this in commit f6b14ca9ef on Jan 21, 2021
  40. ComputerCraftr referenced this in commit 6beb2fce19 on Feb 14, 2021
  41. PastaPastaPasta referenced this in commit cf15c83b5e on Jun 27, 2021
  42. PastaPastaPasta referenced this in commit 1e0ea2bb58 on Jun 28, 2021
  43. PastaPastaPasta referenced this in commit 891a0f9099 on Jun 29, 2021
  44. PastaPastaPasta referenced this in commit 60ea1e2615 on Jul 1, 2021
  45. PastaPastaPasta referenced this in commit b29af0747b on Jul 1, 2021
  46. barton2526 referenced this in commit 193cf03b8d on Jul 13, 2021
  47. PastaPastaPasta referenced this in commit eae83b57e2 on Jul 14, 2021
  48. PastaPastaPasta referenced this in commit ceeea4c0fa on Jul 15, 2021
  49. DrahtBot locked this on Feb 15, 2022


hebasto MarcoFalke DrahtBot vasild fanquake

Labels
Build system

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-01 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me