[Trivial] Do not shadow local variable, cleanup #8449

pull paveljanik wants to merge 2 commits into bitcoin:master from paveljanik:20160803_shadow_blockencodings changing 2 files +4 −5

paveljanik commented at 5:50 PM on August 3, 2016: contributor
I was not available when #8068 was merged, sorry for late fix.

~~This limits the scope of local variable, unifies for cycles used and as a side effect/bonus, fixes three instances of -Wshadow warnings ;-)~~

For the reference, fixed warning is:
```
./blockencodings.h:56:25: warning: declaration shadows a local variable [-Wshadow]
            for (size_t i = 0; i < indexes.size(); i++) {
                        ^
./blockencodings.h:43:20: note: previous declaration is here
            size_t i = 0;
                   ^
```
For more details, see #8105.

in src/blockencodings.h:None in 329680e6d4 outdated

  39 | @@ -40,10 +40,9 @@ class BlockTransactionsRequest {
  40 |          uint64_t indexes_size = (uint64_t)indexes.size();
  41 |          READWRITE(COMPACTSIZE(indexes_size));
  42 |          if (ser_action.ForRead()) {
  43 | -            size_t i = 0;
  44 |              while (indexes.size() < indexes_size) {
  45 |                  indexes.resize(std::min((uint64_t)(1000 + indexes.size()), indexes_size));
  46 | -                for (; i < indexes.size(); i++) {
  47 | +                for (size_t i = 0; i < indexes.size(); i++) {

MarcoFalke commented at 6:05 PM on August 3, 2016:

I think the initial design is on purpose. You are changing it such that the performance drops.

paveljanik commented at 6:24 PM on August 3, 2016:

If it is the case (I doubt it), why the exactly same code is a few lines below, i is not moved up and shared?

MarcoFalke commented at 8:00 PM on August 3, 2016:

Change this occurrence to j? Maybe it would help to combine such changes in a single pull?

paveljanik commented at 6:24 PM on August 3, 2016: contributor

@TheBlueMatt Can you please review this?
Do not shadow local variable, cleanup cce3024c23
paveljanik force-pushed on Aug 3, 2016
jonasschnelli added the label Refactoring on Aug 4, 2016
Remove redundand (and shadowing) declaration a159f25e19
paveljanik commented at 6:05 PM on August 5, 2016: contributor

Added one related commit - redundant declaration.
MarcoFalke commented at 6:15 PM on August 5, 2016: member

Instead of renaming two vars in small scopes you could just rename the one in the large scope?
paveljanik commented at 6:31 PM on August 5, 2016: contributor

Yes, I thought about it too. But then the question would be "Why j before i" ;-) I found this a bit more readable.
sipa commented at 11:46 AM on August 23, 2016: member

utACK a159f25e19b8d921e30742f5ba87ef12803d991d
paveljanik commented at 12:38 PM on August 24, 2016: contributor

more ACKs please, this is trivial PR.
jonasschnelli commented at 12:39 PM on August 24, 2016: contributor

utACK https://github.com/bitcoin/bitcoin/pull/8449/commits/a159f25e19b8d921e30742f5ba87ef12803d991d
MarcoFalke commented at 12:47 PM on August 24, 2016: member

utACK a159f25
laanwj commented at 10:23 AM on September 2, 2016: member

bitcoind for 5c7a5e1 and a159f25 matches, ACK a159f25
laanwj merged this on Sep 2, 2016
laanwj closed this on Sep 2, 2016
laanwj referenced this in commit 381d0ddc8a on Sep 2, 2016
gladcow referenced this in commit 99cea6454c on Mar 2, 2018
gladcow referenced this in commit 7e826d7e8f on Mar 13, 2018
gladcow referenced this in commit ffa39052d9 on Mar 14, 2018
gladcow referenced this in commit f838e15fcc on Mar 15, 2018
gladcow referenced this in commit e17895eb5c on Mar 15, 2018
gladcow referenced this in commit b8c3aa7810 on Mar 24, 2018
gladcow referenced this in commit 63c33e210d on Apr 4, 2018
UdjinM6 referenced this in commit bc45a2f87a on Apr 11, 2018
andvgal referenced this in commit fd5c50bc2b on Jan 6, 2019
CryptoCentric referenced this in commit dd3fd51204 on Feb 28, 2019
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#8068 Compact Blocks #8105 avoid shadowing vars, and enable -Wshadow