Do not shadow in src/qt #8793

1. 
   paveljanik commented at 2:15 pm on September 22, 2016: contributor

   As requested by @laanwj in #8105, Qt shadowing changes all-in-one.

   Maintainers allowed to edit.

2. 
   laanwj commented at 2:16 pm on September 22, 2016: member

   Awesome, will check.
3. 
   paveljanik commented at 2:20 pm on September 22, 2016: contributor

   Please note that many places could be rewritten to use member initializers. And on many places, the diff can be shortened by rewriting initialization process, simplifying this-> stuff and co. But this was not the goal here…
4. 
   laanwj commented at 2:25 pm on September 22, 2016: member

   Right, the goal is easy review and checking, as well as being able to enable those warnings by default after this.
5. laanwj added the label GUI on Sep 22, 2016
6. laanwj added the label Refactoring on Sep 22, 2016
7. laanwj approved
8. 
   laanwj commented at 2:29 pm on September 22, 2016: member

   Binaries match

   0f1916a25418f7a2dac48f142fb56466e6c2d68197f8f1787b68317c1ec62d281  /tmp/compare/bitcoind.26b370a
   1dff7a8d163c1543fe4044b1bc2e70af1d4fa3d3bc2b8b9ded0635b312c7d1534  /tmp/compare/bitcoind.26b370a.stripped
   2f1916a25418f7a2dac48f142fb56466e6c2d68197f8f1787b68317c1ec62d281  /tmp/compare/bitcoind.eec927e
   3dff7a8d163c1543fe4044b1bc2e70af1d4fa3d3bc2b8b9ded0635b312c7d1534  /tmp/compare/bitcoind.eec927e.stripped
   

9. 
   laanwj commented at 2:43 pm on September 22, 2016: member

   Eh, oops I checked the bitcoind binary while this relates to bitcoin-qt, need to recheck. Ignore the above approval for now (I don’t think I can delete it).
10. 
    laanwj commented at 3:21 pm on September 22, 2016: member

    My binary comparator finds differences in the following functions:

     00000000000000000 <WalletModel::WalletModel(PlatformStyle const*, CWallet*, OptionsModel*, QObject*)>:
     10000000000000000 <WalletView::WalletView(PlatformStyle const*, QWidget*)>:
     20000000000000000 <BitcoinGUI::BitcoinGUI(PlatformStyle const*, NetworkStyle const*, QWidget*)>:
     30000000000000000 <EditAddressDialog::EditAddressDialog(EditAddressDialog::Mode, QWidget*)>:
     40000000000000000 <AskPassphraseDialog::AskPassphraseDialog(AskPassphraseDialog::Mode, QWidget*)>:
     50000000000000000 <AddressTableModel::AddressTableModel(CWallet*, WalletModel*)>:
     60000000000000000 <SignVerifyMessageDialog::SignVerifyMessageDialog(PlatformStyle const*, QWidget*)>:
     70000000000000000 <AddressBookPage::AddressBookPage(PlatformStyle const*, AddressBookPage::Mode, AddressBookPage::Tabs, QWidget*)>:
     80000000000000000 <PlatformStyle::PlatformStyle(QString const&, bool, bool, bool)>:
     90000000000000000 <SendCoinsEntry::SendCoinsEntry(PlatformStyle const*, QWidget*)>:
    100000000000000000 <RPCConsole::RPCConsole(PlatformStyle const*, QWidget*)>:
    

    These are just the constructors, even empty ones. I don’t see why, it may have something to do with Qt’s moc generating different metadata?

    ut/code review ACK https://github.com/bitcoin/bitcoin/pull/8793/commits/46b15bb509ac9b4212ed40b12098d1842c878424

    Edit: new commit removes two functions from the list

11. 
    MarcoFalke commented at 6:28 pm on September 22, 2016: member

    Would be interesting to know why the binaries don’t match for qt. Encountered the same here: #8658 (comment)
12. 
    paveljanik commented at 6:40 pm on September 22, 2016: contributor

    The difference cause by this PR is probably different from the difference caused by #8658.

    In this PR, the change in https://github.com/bitcoin/bitcoin/pull/8793/commits/46b15bb509ac9b4212ed40b12098d1842c878424 illustrates the reason. Technically it does the same, but compiler has two places where to get the value used (member variable initialized from the argument or the argument itself).

13. 
    laanwj commented at 7:07 pm on September 22, 2016: member

    @MarcoFalke yes we figured it out on IRC: it has to do with name binding, using the argument versus member variable it was assigned to. For example this seems to make the differences in AddressBookPage::AddressBookPage go away: http://www.hastebin.com/jixoyufuxu.patch .

    But that’s kind of ugly. So in this case I prefer slightly different executables to larger source code patch that makes the code uglier. If it was consensus code it’d be different.

14. Do not shadow in src/qt f839350420
15. laanwj force-pushed on Sep 23, 2016
16. 
    laanwj commented at 10:43 am on September 23, 2016: member

    Squashed the squashme commit.
17. laanwj merged this on Sep 23, 2016
18. laanwj closed this on Sep 23, 2016

    ---

19. laanwj referenced this in commit 5d0219d983 on Sep 23, 2016
20. codablock referenced this in commit f157015eb7 on Sep 19, 2017
21. codablock referenced this in commit c7a0b91d31 on Jan 11, 2018
22. andvgal referenced this in commit 80f3c051b8 on Jan 6, 2019
23. DrahtBot locked this on Sep 8, 2021

Contributors
paveljanik laanwj MarcoFalke

Labels
GUI Refactoring