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

maflcko · 2021-06-16T11:05:22Z

Fixes #18967

Alternative to #22240

maflcko · 2021-06-16T11:57:40Z

Tested warnings-free on Focal with:

clang (./configure --with-incompatible-bdb --enable-suppress-external-warnings CC=clang CXX=clang++ && make clean && make -j $(nproc))
gcc (./configure --with-incompatible-bdb --enable-suppress-external-warnings && make clean && make -j $(nproc))
depends (CONFIG_SITE="$PWD/depends/x86_64-pc-linux-gnu/share/config.site" ./configure --enable-suppress-external-warnings && make clean && make -j $(nproc))

Also tested on Fedora 34 with clang and gcc.

maflcko · 2021-06-16T11:59:30Z

@fanquake (or someone else) Can you test this one please: #18738 (comment)

practicalswift · 2021-06-16T12:00:09Z

Concept ACK

configure.ac

…e enabled

DrahtBot · 2021-06-16T15:14:55Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

build: Add -Werror=implicit-fallthrough compile flag #21430 (build: Add -Werror=implicit-fallthrough compile flag by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake

tACK 1111457

… external warnings are enabled 1111457 build: Disable deprecated-copy warning only when external warnings are enabled (MarcoFalke) Pull request description: Fixes bitcoin#18967 Alternative to bitcoin#22240 ACKs for top commit: fanquake: tACK 1111457 Tree-SHA512: 0fc826f26ebbeab662fa7eed2a5cc1630c6c4e612deb91734885fc8bae0352be657ec48ae94ff55a984ac36d27b95cea8d947cc5cf408231d56addecf79db83f

vasild · 2021-06-21T15:29:46Z

According to Clang 13.0.0 we have deprecated-copy warnings in our code too:

clang++-devel -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config  -DDEBUG -DDEBUG_LOCKORDER -DABORT_ON_FAILED_ASSUME  -I. -I./secp256k1/include -isystem /usr/local/include/db5 -DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC -isystem /usr/local/include -I./leveldb/include -I./leveldb/helpers/memenv -I./univalue/include   -isystem /usr/local/include -I/usr/local/include -D_THREAD_SAFE -pthread  -I/usr/local/include -Wno-deprecated-declarations -Werror -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DPROVIDE_FUZZ_MAIN_FUNCTION -O0 -g3 -ftrapv -Wstack-protector -fstack-protector-all -fcf-protection=full -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wswitch -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunused-variable -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Wsign-compare -Woverloaded-virtual -Wsuggest-override -Wunreachable-code-loop-increment -Wdocumentation -Wno-unused-parameter -Wno-self-assign -Wno-unused-local-typedef -Wno-implicit-fallthrough    -fPIE  -MT policy/libbitcoin_server_a-rbf.o -MD -MP -MF policy/.deps/libbitcoin_server_a-rbf.Tpo -c -o policy/libbitcoin_server_a-rbf.o `test -f 'policy/rbf.cpp' || echo './'`policy/rbf.cpp

In file included from policy/rbf.cpp:5:
In file included from ./policy/rbf.h:8:
In file included from ./txmempool.h:24:
./util/epochguard.h:53:17: error: definition of implicit copy constructor for 'Marker' is deprecated because it has a user-declared copy assignment operator [-Werror,-Wdeprecated-copy]
        Marker& operator=(const Marker&) = delete;
                ^
./txmempool.h:81:7: note: in implicit copy constructor for 'Epoch::Marker' first required here
class CTxMemPoolEntry
      ^
policy/rbf.cpp:29:29: note: in implicit copy constructor for 'CTxMemPoolEntry' first required here
    CTxMemPoolEntry entry = *pool.mapTx.find(tx.GetHash());
                            ^
1 error generated.




In file included from test/fuzz/policy_estimator.cpp:9:
In file included from ./test/fuzz/util.h:27:
In file included from ./txmempool.h:24:
./util/epochguard.h:53:17: error: definition of implicit copy constructor for 'Marker' is deprecated because it has a user-declared copy assignment operator [-Werror,-Wdeprecated-copy]
        Marker& operator=(const Marker&) = delete;
                ^
./txmempool.h:81:7: note: in implicit copy constructor for 'Epoch::Marker' first required here
class CTxMemPoolEntry
      ^
/usr/include/c++/v1/memory:1876:31: note: in implicit move constructor for 'CTxMemPoolEntry' first required here
            ::new((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
                              ^
/usr/include/c++/v1/memory:1768:18: note: in instantiation of function template specialization 'std::allocator<CTxMemPoolEntry>::construct<CTxMemPoolEntry, CTxMemPoolEntry>' requested here
            {__a.construct(__p, _VSTD::forward<_Args>(__args)...);}
                 ^
/usr/include/c++/v1/memory:1595:14: note: in instantiation of function template specialization 'std::allocator_traits<std::allocator<CTxMemPoolEntry>>::__construct<CTxMemPoolEntry, CTxMemPoolEntry>' requested here
            {__construct(__has_construct<allocator_type, _Tp*, _Args...>(),
             ^
/usr/include/c++/v1/vector:924:21: note: in instantiation of function template specialization 'std::allocator_traits<std::allocator<CTxMemPoolEntry>>::construct<CTxMemPoolEntry, CTxMemPoolEntry>' requested here
    __alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__tx.__pos_),
                    ^
/usr/include/c++/v1/vector:1652:9: note: in instantiation of function template specialization 'std::vector<CTxMemPoolEntry>::__construct_one_at_end<CTxMemPoolEntry>' requested here
        __construct_one_at_end(_VSTD::move(__x));
        ^
test/fuzz/policy_estimator.cpp:49:37: note: in instantiation of member function 'std::vector<CTxMemPoolEntry>::push_back' requested here
                    mempool_entries.push_back(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx));
                                    ^
1 error generated.

cc @ajtowns the below fixes it:

diff --git i/src/policy/rbf.cpp w/src/policy/rbf.cpp
index 8125b41c41..10859d256d 100644
--- i/src/policy/rbf.cpp
+++ w/src/policy/rbf.cpp
@@ -23,13 +23,13 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool)
     }
 
     // If all the inputs have nSequence >= maxint-1, it still might be
     // signaled for RBF if any unconfirmed parents have signaled.
     uint64_t noLimit = std::numeric_limits<uint64_t>::max();
     std::string dummy;
-    CTxMemPoolEntry entry = *pool.mapTx.find(tx.GetHash());
+    const CTxMemPoolEntry& entry = *pool.mapTx.find(tx.GetHash());
     pool.CalculateMemPoolAncestors(entry, setAncestors, noLimit, noLimit, noLimit, noLimit, dummy, false);
 
     for (CTxMemPool::txiter it : setAncestors) {
         if (SignalsOptInRBF(it->GetTx())) {
             return RBFTransactionState::REPLACEABLE_BIP125;
         }
diff --git i/src/util/epochguard.h w/src/util/epochguard.h
index 1570ec4eb4..79c0abb4eb 100644
--- i/src/util/epochguard.h
+++ w/src/util/epochguard.h
@@ -42,12 +42,16 @@ public:
     Epoch& operator=(const Epoch&) = delete;
 
     bool guarded() const { return m_guarded; }
 
     class Marker
     {
+    public:
+        Marker() = default;
+        Marker(const Marker& other) = default;
+
     private:
         uint64_t m_marker = 0;
 
         // only allow modification via Epoch member functions
         friend class Epoch;
         Marker& operator=(const Marker&) = delete;

… external warnings are enabled 1111457 build: Disable deprecated-copy warning only when external warnings are enabled (MarcoFalke) Pull request description: Fixes bitcoin#18967 Alternative to bitcoin#22240 ACKs for top commit: fanquake: tACK 1111457 Tree-SHA512: 0fc826f26ebbeab662fa7eed2a5cc1630c6c4e612deb91734885fc8bae0352be657ec48ae94ff55a984ac36d27b95cea8d947cc5cf408231d56addecf79db83f

fanquake reviewed Jun 16, 2021

View reviewed changes

configure.ac Show resolved Hide resolved

DrahtBot added the Build system label Jun 16, 2021

build: Disable deprecated-copy warning only when external warnings ar…

1111457

…e enabled

maflcko force-pushed the 2106-buildEnableWarnDeprecatedCopy branch from fa9824c to 1111457 Compare June 16, 2021 13:44

DrahtBot mentioned this pull request Jun 16, 2021

build: Add -Werror=implicit-fallthrough compile flag #21430

Merged

Subawit approved these changes Jun 17, 2021

View reviewed changes

fanquake approved these changes Jun 17, 2021

View reviewed changes

fanquake merged commit 65c4a36 into bitcoin:master Jun 17, 2021

maflcko deleted the 2106-buildEnableWarnDeprecatedCopy branch June 17, 2021 07:12

gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022

Merge 65c4a36 into merged_master (Bitcoin PR bitcoin/bitcoin#22258)

98800c6

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

maflcko commented Jun 16, 2021

Uh oh!

maflcko commented Jun 16, 2021

Uh oh!

maflcko commented Jun 16, 2021

Uh oh!

practicalswift commented Jun 16, 2021

Uh oh!

Uh oh!

DrahtBot commented Jun 16, 2021

Uh oh!

fanquake left a comment

Uh oh!

vasild commented Jun 21, 2021

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

maflcko commented Jun 16, 2021

Uh oh!

maflcko commented Jun 16, 2021

Uh oh!

maflcko commented Jun 16, 2021

Uh oh!

practicalswift commented Jun 16, 2021

Uh oh!

Uh oh!

DrahtBot commented Jun 16, 2021

Conflicts

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

vasild commented Jun 21, 2021

Uh oh!

Uh oh!