build: Suppress -Wdeprecated-copy warnings #18738

hebasto · 2020-04-22T15:01:36Z

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.

hebasto · 2020-04-22T16:17:01Z

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.

maflcko · 2020-04-22T16:31:05Z

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

hebasto · 2020-04-22T17:29:43Z

@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 422dcd5 from #14920 or @vasild's c9376ae from #18149 could help, no?

maflcko · 2020-04-22T17:34:52Z

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

In file included from qt/rpcconsole.cpp:9:
In file included from ./qt/rpcconsole.h:8:
In file included from ./qt/guiutil.h:12:
In file included from /usr/include/qt5/QtWidgets/QHeaderView:1:
In file included from /usr/include/qt5/QtWidgets/qheaderview.h:44:
In file included from /usr/include/qt5/QtWidgets/qabstractitemview.h:44:
In file included from /usr/include/qt5/QtWidgets/qabstractscrollarea.h:44:
In file included from /usr/include/qt5/QtWidgets/qframe.h:44:
In file included from /usr/include/qt5/QtWidgets/qwidget.h:45:
In file included from /usr/include/qt5/QtCore/qobject.h:47:
/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]
    inline QCharRef &operator=(const QCharRef &c) { return operator=(QChar(c)); }
                     ^
/usr/include/qt5/QtCore/qstring.h:1161:28: note: in implicit copy constructor for 'QCharRef' first required here
{ Q_ASSERT(i >= 0); return QCharRef(*this, i); }
                           ^
2 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.

hebasto · 2020-04-22T17:38:57Z

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

maflcko · 2020-04-22T17:43:10Z

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

Concept ACK

hebasto · 2020-04-22T17:52:25Z

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

Concept ACK

But it does not work (comment) 😕

UPDATE: I found bugs in my solution :)

hebasto · 2020-04-22T18:03:46Z

Updated 0025d09 -> 710a072 (pr18738.01 -> pr18738.03, diff):

addressed @MarcoFalke's comment:

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

configure.ac

DrahtBot · 2020-04-22T19:40:40Z

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

Conflicts

No conflicts as of last run.

hebasto · 2020-04-22T22:30:46Z

Updated 710a072 -> d69174c (pr18738.03 -> pr18738.04, diff):

removed too narrow mention of Qt version; addressed @MarcoFalke's comment:

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

vasild · 2020-04-23T07:58:39Z

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.

vasild · 2020-04-23T20:18:55Z

I will submit it as a separate PR

Here it is: #18750

fanquake · 2020-05-02T13:45:28Z

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

make src/bitcoind -j8
...
In file included from addrman.cpp:6:
In file included from ./addrman.h:15:
In file included from ./util/system.h:20:
In file included from ./fs.h:14:
In file included from /usr/local/include/boost/filesystem.hpp:16:
In file included from /usr/local/include/boost/filesystem/path.hpp:32:
/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]
        quoted_proxy& operator=(const quoted_proxy&);  // = deleted
                      ^
/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
      return detail::quoted_proxy<std::basic_string<Char, Traits, Alloc> const &, Char>
             ^
/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
      << boost::io::quoted(p.template string<std::basic_string<Char> >(), static_cast<Char>('&'));
                    ^
./tinyformat.h:358:13: note: in instantiation of function template specialization 'boost::filesystem::operator<<<char, std::__1::char_traits<char> >' requested here
        out << value;
            ^
./tinyformat.h:543:13: note: in instantiation of function template specialization 'tinyformat::formatValue<boost::filesystem::path>' requested here
            formatValue(out, fmtBegin, fmtEnd, ntrunc, *static_cast<const T*>(value));
            ^
./tinyformat.h:519:27: note: in instantiation of function template specialization 'tinyformat::detail::FormatArg::formatImpl<boost::filesystem::path>' requested here
            m_formatImpl(&formatImpl<T>),
                          ^
./tinyformat.h:975:32: note: (skipping 1 context in backtrace; use -ftemplate-backtrace-limit=0 to see all)
            m_formatterStore { FormatArg(args)... }
                               ^
./tinyformat.h:1028:12: note: in instantiation of function template specialization 'tinyformat::detail::FormatListN<2>::FormatListN<boost::filesystem::path, int>' requested here
    return detail::FormatListN<sizeof...(args)>(args...);
           ^
./tinyformat.h:1064:23: note: in instantiation of function template specialization 'tinyformat::makeFormatList<boost::filesystem::path, int>' requested here
    vformat(out, fmt, makeFormatList(args...));
                      ^
./tinyformat.h:1073:5: note: in instantiation of function template specialization 'tinyformat::format<boost::filesystem::path, int>' requested here
    format(oss, fmt, args...);
    ^
./logging.h:169:28: note: in instantiation of function template specialization 'tinyformat::format<boost::filesystem::path, int>' requested here
            log_msg = tfm::format(fmt, args...);
                           ^
addrman.cpp:638:5: note: in instantiation of function template specialization 'LogPrintf<boost::filesystem::path, int>' requested here
    LogPrintf("Opened asmap file %s (%d bytes) from disk\n", path, length);
    ^
1 warning generated.

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

-Wdeprecated-copy warnings.

hebasto · 2020-05-03T06:39:43Z

@fanquake I can see these boost-1.72.0-specific warnings too.

maflcko · 2020-05-04T15:44:30Z

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.

hebasto · 2020-05-05T03:19:25Z

Reverted back d69174c -> 0025d09 (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.

fanquake · 2020-05-05T03:20:53Z

The commit message needs updating as well.

hebasto · 2020-05-05T03:26:09Z

Updated 0025d09 -> 0c63f80 (pr18738.01 -> pr18738.05, diff):

addressed @fanquake's comment:

The commit message needs updating as well.

maflcko · 2020-05-05T11:35:39Z

ACK 0c63f80 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.

hebasto · 2020-05-13T06:46:16Z

@MarcoFalke Is it worth backporting to 0.20 branch before rc2 for neat user experience on Ubuntu Focal?

maflcko · 2020-05-13T10:50:14Z

cc @fanquake ^

fanquake

ACK 0c63f80 - 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.

hebasto · 2020-05-13T13:19:43Z

Backport to 0.20?

0c63f80 build: Suppress -Wdeprecated-copy warnings (Hennadii Stepanov) Pull request description: 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 (bitcoin#15822, bitcoin#18419). This PR suppress such warnings _temporarily_, until the [upstream is fixed](https://codereview.qt-project.org/c/qt/qtbase/+/272258). 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](https://bugreports.qt.io/browse/QTBUG-75210) Also see **fanquake**'s [comment](bitcoin#18738 (comment)). ACKs for top commit: MarcoFalke: ACK 0c63f80 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. fanquake: ACK 0c63f80 - 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. Tree-SHA512: 7064a3272bc9eae00b73a16c421ac58be148f374cbef87320e8f092f52761f6e98166eff60346b70867f8a69a9698a79455dc16b42d92f8fbe7c56519571ac08

ae41ebe Remove redundant explicitly defined copy ctors (Dan Raviv) 454aa37 Trivial: explicit (default) copy-constructor for CTransaction (random-zebra) 2af2306 Cleanup: Remove redundant copy-constructor for CSignedMessage (random-zebra) e28259d Masternode: proper copy-assignment for CMasternode/CmasternodePing (random-zebra) Pull request description: Fix the `-Wdeprecated-copy` warnings listed in #2076 **Background** While perfectly valid in C++98, implicit generation of the copy constructor is declared as deprecated in C++11, when there is already a user-defined copy assignment operator, or vice-versa. The rationale behind this is the "rule of three" (https://en.cppreference.com/w/cpp/language/rule_of_three) > If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three. **Issue - Fix** We have several places that define redundant copy assignment operators (and then use the implicit copy constructor), or that do need non-default ctors/dtors but don't define copy assignment. (See also bitcoin#11161, which is also cherry-picked here) Specifically: - `CSignedMessage`/`CMasternodePing`/`CMasternode` use a weird copy-assignment that *swaps* with the argument (which, of course, cannot be "const" then). Here we remove all ::swap implementations, and provide actual copy assignment operators. - `CTransaction` has an explicit copy-assignment operator, but uses the implicit copy-ctor in `CMerkleTx` constructor. - `CFeeRate` and `CTxMemPoolEntry` have explicitly defined copy ctor, which can (and should, for the rule-of-three) be replaced by the default one (bitcoin#11161). **Note** Reason why these warnings pop up just now. Since Clang 10.0 and GCC 9.3, `-Wextra` enables `-Wdeprecated-copy` by default, and that is very spammy, especially when compiling with the QT GUI (bitcoin#15822 / bitcoin#18419). It is so spammy (also with `boost-1.72.0`) that Bitcoin decided to temporarily suppress this warning (bitcoin#18738) until QT fixes it upstream, and boost is definitely removed in 0.22 (bitcoin#18967) ACKs for top commit: furszy: utACK ae41ebe Tree-SHA512: 1f59f9f7426abbf4b7d4bb0d1ac50491eb52f776e2a018de3acc8ab381af3b69db6b9a5b60c04b48f0d495e560eaa1e3d19be03d133f230daca66ba66e766289

0c63f80 build: Suppress -Wdeprecated-copy warnings (Hennadii Stepanov) Pull request description: 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 (bitcoin#15822, bitcoin#18419). This PR suppress such warnings _temporarily_, until the [upstream is fixed](https://codereview.qt-project.org/c/qt/qtbase/+/272258). 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](https://bugreports.qt.io/browse/QTBUG-75210) Also see **fanquake**'s [comment](bitcoin#18738 (comment)). ACKs for top commit: MarcoFalke: ACK 0c63f80 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. fanquake: ACK 0c63f80 - 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. Tree-SHA512: 7064a3272bc9eae00b73a16c421ac58be148f374cbef87320e8f092f52761f6e98166eff60346b70867f8a69a9698a79455dc16b42d92f8fbe7c56519571ac08

Should have been fixed with gridcoin-community#1702 Ref: bitcoin/bitcoin#18738

0c63f80 build: Suppress -Wdeprecated-copy warnings (Hennadii Stepanov) Pull request description: 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 (bitcoin#15822, bitcoin#18419). This PR suppress such warnings _temporarily_, until the [upstream is fixed](https://codereview.qt-project.org/c/qt/qtbase/+/272258). 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](https://bugreports.qt.io/browse/QTBUG-75210) Also see **fanquake**'s [comment](bitcoin#18738 (comment)). ACKs for top commit: MarcoFalke: ACK 0c63f80 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. fanquake: ACK 0c63f80 - 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. Tree-SHA512: 7064a3272bc9eae00b73a16c421ac58be148f374cbef87320e8f092f52761f6e98166eff60346b70867f8a69a9698a79455dc16b42d92f8fbe7c56519571ac08

DrahtBot added the Build system label Apr 22, 2020

hebasto force-pushed the 200422-qt-warn branch from 0025d09 to 710a072 Compare April 22, 2020 18:02

maflcko reviewed Apr 22, 2020

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

DrahtBot mentioned this pull request Apr 22, 2020

Multiprocess build support #18677

Merged

hebasto force-pushed the 200422-qt-warn branch from 710a072 to d69174c Compare April 22, 2020 22:27

DrahtBot mentioned this pull request Apr 23, 2020

Multiprocess bitcoin #10102

Draft

hebasto force-pushed the 200422-qt-warn branch from d69174c to 0025d09 Compare May 5, 2020 03:13

hebasto changed the title ~~build: Suppress Qt 5.12 warnings on Ubuntu Focal~~ build: Suppress -Wdeprecated-copy warnings May 5, 2020

build: Suppress -Wdeprecated-copy warnings

0c63f80

hebasto force-pushed the 200422-qt-warn branch from 0025d09 to 0c63f80 Compare May 5, 2020 03:23

fanquake approved these changes May 13, 2020

View reviewed changes

fanquake merged commit 6c647c8 into bitcoin:master May 13, 2020

hebasto deleted the 200422-qt-warn branch May 13, 2020 13:21

maflcko mentioned this pull request May 13, 2020

build: [0.22] Enable -Wdeprecated-copy warnings #18967

Closed

hebasto mentioned this pull request Jun 6, 2020

Ubuntu 20.04 LTS #18419

Closed

random-zebra mentioned this pull request Dec 20, 2020

[Refactor] Fix some -Wdeprecated-copy warnings PIVX-Project/PIVX#2089

Merged

jeroz1 mentioned this pull request May 15, 2021

build: Suppress -Wdeprecated-copy warnings RavenProject/Ravencoin#1007

Merged

hebasto mentioned this pull request Jun 14, 2021

build: Re-enable -Wdeprecated-copy #22240

Closed

maflcko mentioned this pull request Jun 16, 2021

build: Disable deprecated-copy warning only when external warnings are enabled #22258

Merged

barton2526 added a commit to barton2526/Gridcoin-Research that referenced this pull request Jul 13, 2021

build: Suppress -Wdeprecated-copy warnings

193cf03

Should have been fixed with gridcoin-community#1702 Ref: bitcoin/bitcoin#18738

barton2526 mentioned this pull request Jul 13, 2021

build: Suppress -Wdeprecated-copy warnings gridcoin-community/Gridcoin-Research#2229

Closed

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

build: Suppress -Wdeprecated-copy warnings #18738

build: Suppress -Wdeprecated-copy warnings #18738

Uh oh!

Conversation

hebasto commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Apr 22, 2020

Uh oh!

maflcko commented Apr 22, 2020

Uh oh!

hebasto commented Apr 22, 2020

Uh oh!

maflcko commented Apr 22, 2020

Uh oh!

hebasto commented Apr 22, 2020

Uh oh!

maflcko commented Apr 22, 2020

Uh oh!

hebasto commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Apr 22, 2020

Uh oh!

Uh oh!

DrahtBot commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Apr 22, 2020

Uh oh!

vasild commented Apr 23, 2020

Uh oh!

vasild commented Apr 23, 2020

Uh oh!

fanquake commented May 2, 2020

Uh oh!

hebasto commented May 3, 2020

Uh oh!

maflcko commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented May 5, 2020

Uh oh!

fanquake commented May 5, 2020

Uh oh!

hebasto commented May 5, 2020

Uh oh!

maflcko commented May 5, 2020

Uh oh!

hebasto commented May 13, 2020

Uh oh!

maflcko commented May 13, 2020

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto commented May 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hebasto commented Apr 22, 2020 •

edited

Loading

hebasto commented Apr 22, 2020 •

edited

Loading

DrahtBot commented Apr 22, 2020 •

edited

Loading

maflcko commented May 4, 2020 •

edited

Loading