-
Notifications
You must be signed in to change notification settings - Fork 37.7k
test: Move variable state
down where it is used
#10739
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: Move variable state
down where it is used
#10739
Conversation
utACK 46d53a65f8627f3d65baab00abf26bba3b6259b8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not remove the upper declaration as it isn't used in that scope and state
is not shared across those blocks?
@promag no particular reason other than loosing 3- diff status... |
utACK 46d53a6 |
I'd prefer the +1-1 diff suggested by promag. This way, the state does not leak into the outer scope. diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index a74f402..3d8a4fd 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -198,4 +198,4 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
// spend_tx is invalid according to DERSIG
- CValidationState state;
{
+ CValidationState state;
PrecomputedTransactionData ptd_spend_tx(spend_tx); |
46d53a6
to
855adc6
Compare
Combined/correct fix used instead. |
src/test/txvalidationcache_tests.cpp
Outdated
@@ -293,7 +292,6 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) | |||
|
|||
// Make it valid, and check again | |||
invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100; | |||
CValidationState state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep.
src/test/txvalidationcache_tests.cpp
Outdated
@@ -354,7 +352,6 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) | |||
// Invalidate vin[1] | |||
tx.vin[1].scriptWitness.SetNull(); | |||
|
|||
CValidationState state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep.
{ | ||
CValidationState state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave the old line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean with unchanged indentation? Or?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore, it's good.
855adc6
to
5618b7d
Compare
Obvious ACK 5618b7d. |
Please rename PR and commit before merge. |
state
, reuse it as in other cases.state
state
state
down where it is used
utACK 5618b7d |
1 similar comment
utACK 5618b7d |
state
down where it is usedstate
down where it is used
utACK 5618b7d |
5618b7d Do not shadow upper local variable `state`. (Pavel Janík) Pull request description: Tests added in #10192 emit few shadowing warnings: ``` test/txvalidationcache_tests.cpp:268:26: warning: declaration shadows a local variable [-Wshadow] test/txvalidationcache_tests.cpp:296:26: warning: declaration shadows a local variable [-Wshadow] test/txvalidationcache_tests.cpp:357:26: warning: declaration shadows a local variable [-Wshadow] ``` Remove shadowing declarations and reuse the upper local declaration as in other already present test cases. Tree-SHA512: 1e3c52cf963f8f33e729900c8ecdcd5cc6fe28caa441ba53c4636df9cc3d1a351ca231966d36384589f1340ae8ddd447424c2ee3e8527d334d0412f0d1a10c8f
Tests added in #10192 emit few shadowing warnings:
Remove shadowing declarations and reuse the upper local declaration as in other already present test cases.