test: Avoid overwriting the NodeContext member of the testing setup [-Wshadow-field]

MarcoFalke commented at 12:14 pm on June 6, 2020: member

Adding this warning will eliminate unexpected test failures and hard to review code. Moreover, there shouldn’t be a use case in Bitcoin Core that relies on fields to be shadowed.

build: Add -Wshadow-field fa16e7816b

MarcoFalke commented at 12:15 pm on June 6, 2020: member

Requested by @ryanofsky in https://github.com/bitcoin/bitcoin/pull/18923/files#r436093574

MarcoFalke renamed this:
~~2006 test no shadow field~~
test: Avoid overwriting the NodeContext member of the testing setup
on Jun 6, 2020

MarcoFalke renamed this:
~~test: Avoid overwriting the NodeContext member of the testing setup~~
test: Avoid overwriting the NodeContext member of the testing setup [-Wshadow-field]
on Jun 6, 2020

MarcoFalke commented at 12:17 pm on June 6, 2020: member

Can be tested by reverting the fix and observing the warning:

0In file included from wallet/test/init_test_fixture.cpp:8:
1./wallet/test/init_test_fixture.h:21:17: warning: non-static data member 'm_node' of 'InitWalletDirTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
2    NodeContext m_node;
3                ^
4./test/util/setup_common.h:76:17: note: declared here
5    NodeContext m_node;
6                ^
71 warning generated.

DrahtBot commented at 12:49 pm on June 6, 2020: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#18857 (build: avoid repetitions when enabling warnings in configure.ac by vasild)

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.

test: Avoid overwriting the NodeContext member of the testing setup fac6b9b938

MarcoFalke force-pushed on Jun 6, 2020

DrahtBot added the label Build system on Jun 6, 2020

DrahtBot added the label Tests on Jun 6, 2020

DrahtBot added the label Wallet on Jun 6, 2020

hebasto commented at 2:23 pm on June 6, 2020: member

Concept ACK.

MarcoFalke removed the label Build system on Jun 6, 2020

MarcoFalke removed the label Wallet on Jun 6, 2020

hebasto approved

hebasto commented at 3:35 pm on June 6, 2020: member

ACK fac6b9b938230d24c13fcb6e9be28515d674c6c8, tested on Linux Mint 19.3 (x86_64):

Clang 9.0.0 accepts -Wshadow-field and works as expected
GCC 7.5.0 does not accept -Wshadow-field

The wider -Wshadow was disabled in #10136.

practicalswift commented at 5:38 pm on June 6, 2020: contributor

ACK fac6b9b938230d24c13fcb6e9be28515d674c6c8 – patch looks correct

MarcoFalke requested review from fanquake on Jun 7, 2020

fanquake approved

fanquake commented at 5:35 am on June 8, 2020: member

ACK fac6b9b938230d24c13fcb6e9be28515d674c6c8 - Warnings compiling fa16e7816b886b82d36457b0d8edc773cba76421 are below. No warnings with fac6b9b938230d24c13fcb6e9be28515d674c6c8. The -Wshadow-field diagnostic has been available in Clang since 5.0.0. It’s not available for GCC.

 0In file included from wallet/test/wallet_test_fixture.cpp:5:
 1./wallet/test/wallet_test_fixture.h:22:17: warning: non-static data member 'm_node' of 'WalletTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
 2    NodeContext m_node;
 3                ^
 4./test/util/setup_common.h:76:17: note: declared here
 5    NodeContext m_node;
 6                ^
 71 warning generated.
 8....
 9In file included from wallet/test/init_test_fixture.cpp:8:
10./wallet/test/init_test_fixture.h:21:17: warning: non-static data member 'm_node' of 'InitWalletDirTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
11    NodeContext m_node;
12                ^
13./test/util/setup_common.h:76:17: note: declared here
14    NodeContext m_node;
15                ^
161 warning generated.
17...
18In file included from wallet/test/wallet_test_fixture.cpp:5:
19./wallet/test/wallet_test_fixture.h:22:17: warning: non-static data member 'm_node' of 'WalletTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
20    NodeContext m_node;
21                ^
22./test/util/setup_common.h:76:17: note: declared here
23    NodeContext m_node;
24                ^
251 warning generated.
26...
27In file included from wallet/test/init_tests.cpp:11:
28./wallet/test/init_test_fixture.h:21:17: warning: non-static data member 'm_node' of 'InitWalletDirTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
29    NodeContext m_node;
30                ^
31./test/util/setup_common.h:76:17: note: declared here
32    NodeContext m_node;
33                ^
341 warning generated.
35  CXX      test/util/libtest_util_a-mining.o
36  CXX      test/util/libtest_util_a-net.o
37In file included from wallet/test/psbt_wallet_tests.cpp:12:
38./wallet/test/wallet_test_fixture.h:22:17: warning: non-static data member 'm_node' of 'WalletTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
39    NodeContext m_node;
40                ^
41./test/util/setup_common.h:76:17: note: declared here
42    NodeContext m_node;
43                ^
441 warning generated.
45  CXX      test/util/libtest_util_a-setup_common.o
46  CXX      test/util/libtest_util_a-str.o
47  CXX      test/util/libtest_util_a-transaction_utils.o
48  CXX      test/util/libtest_util_a-wallet.o
49  CXX      bench/bench_bitcoin-addrman.o
50  CXX      bench/bench_bitcoin-bench_bitcoin.o
51  CXX      bench/bench_bitcoin-bench.o
52  CXX      bench/bench_bitcoin-block_assemble.o
53  CXX      bench/bench_bitcoin-checkblock.o
54In file included from wallet/test/coinselector_tests.cpp:12:
55./wallet/test/wallet_test_fixture.h:22:17: warning: non-static data member 'm_node' of 'WalletTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
56    NodeContext m_node;
57                ^
58./test/util/setup_common.h:76:17: note: declared here
59    NodeContext m_node;
60                ^
611 warning generated.
62  CXX      bench/bench_bitcoin-checkqueue.o
63In file included from wallet/test/wallet_tests.cpp:22:
64./wallet/test/wallet_test_fixture.h:22:17: warning: non-static data member 'm_node' of 'WalletTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
65    NodeContext m_node;
66                ^
67./test/util/setup_common.h:76:17: note: declared here
68    NodeContext m_node;
69                ^
70wallet/test/wallet_tests.cpp:538:17: warning: non-static data member 'm_node' of 'ListCoinsTestingSetup' shadows member inherited from type 'BasicTestingSetup' [-Wshadow-field]
71    NodeContext m_node;
72                ^
73./test/util/setup_common.h:76:17: note: declared here
74    NodeContext m_node;
75                ^
762 warnings generated.

fanquake merged this on Jun 8, 2020

fanquake closed this on Jun 8, 2020

MarcoFalke deleted the branch on Jun 8, 2020

jasonbcox referenced this in commit 3de58c3057 on Jul 8, 2020

DrahtBot locked this on Feb 15, 2022

test: Avoid overwriting the NodeContext member of the testing setup [-Wshadow-field] #19188

Conflicts