Annotate unused parameters with [[maybe_unused]] #14194

1. 
   practicalswift commented at 9:22 pm on September 10, 2018: contributor

   Annotate unused parameters with [[maybe_unused]].
2. practicalswift force-pushed on Sep 10, 2018
3. fanquake added the label Build system on Sep 10, 2018
4. 
   DrahtBot commented at 11:50 pm on September 10, 2018: 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:

   • #14605 (Return of the Banman by dongcarl)
   • #14224 (Document intentional and unintentional unsigned integer overflows (wraparounds) using annotations by practicalswift)
   • #13864 (validation: Document where we are intentionally ignoring bool return values from validation related functions by practicalswift)
   • #13815 (util: Add [[nodiscard]] to all {Decode,Parse}… functions returning bool by practicalswift)

   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.

5. 
   Empact commented at 5:09 am on September 11, 2018: member

   Concept ACK maybe_unused is a C++17 introduction, I assume it’s ok to use it though based on preprocessor activation.
6. 
   practicalswift commented at 7:55 am on September 11, 2018: contributor

   @Empact Yes, it is worth noting that both clang++ and g++ support [[maybe_unused]] also under -std=c++11. Luckily no need for wait for C++17 :-)
7. 
   Empact commented at 0:10 am on September 12, 2018: member

   BlockNotifyGenesisWait uses unnamed args to mark them as unreferenced. Should we name there or apply that here?
8. in src/attributes.h:11 in a363c5d1e6 outdated

    6+#ifndef BITCOIN_ATTRIBUTES_H
    7+#define BITCOIN_ATTRIBUTES_H
    8+
    9+#if defined(__has_cpp_attribute) && __has_cpp_attribute(maybe_unused)
   10+#  define MAYBE_UNUSED [[maybe_unused]]
   11+#elif defined(__GNUC__) || defined(__clang__)
   

   ---

   ---

   

   ken2812221 commented at 3:26 am on September 12, 2018:

   __GNUC__ is also defined by clang. Don’t have to check __clang__
9. Introduce MAYBE_UNUSED ([maybe_unused]]) b7ca9568c3
10. practicalswift force-pushed on Sep 12, 2018
11. 
    practicalswift commented at 3:13 pm on September 12, 2018: contributor

    @ken2812221 Updated to address feedback. Please re-review :-)
12. in src/rpc/blockchain.h:29 in 54b3d00e24 outdated

    25@@ -24,7 +26,7 @@ static constexpr int NUM_GETBLOCKSTATS_PERCENTILES = 5;
    26 double GetDifficulty(const CBlockIndex* blockindex);
    27 
    28 /** Callback for when block tip changed. */
    29-void RPCNotifyBlockChange(bool ibd, const CBlockIndex *);
    30+void RPCNotifyBlockChange(MAYBE_UNUSED bool ibd, const CBlockIndex *);
    

    ---

    ---

    

    arvidn commented at 2:11 pm on September 17, 2018:

    it seems odd that this would be necessary in a declaration. instead of adding the attribute here, you could just remove the name of the parameter (just like for the second parameter). neither are used in the declaration anyway.
13. in src/rpc/blockchain.cpp:194 in 54b3d00e24 outdated

    190@@ -191,7 +191,7 @@ static UniValue getbestblockhash(const JSONRPCRequest& request)
    191     return chainActive.Tip()->GetBlockHash().GetHex();
    192 }
    193 
    194-void RPCNotifyBlockChange(bool ibd, const CBlockIndex * pindex)
    195+void RPCNotifyBlockChange(MAYBE_UNUSED bool ibd, const CBlockIndex * pindex)
    

    ---

    ---

    

    arvidn commented at 2:12 pm on September 17, 2018:

    ibd seems to always be unused here. I think a better approach is to just remove the argument name
14. Annotate unused parameters with [[maybe_unused]] 9cd951cc77
15. in src/qt/transactiontablemodel.cpp:725 in 54b3d00e24 outdated

    721@@ -722,7 +722,7 @@ static void NotifyTransactionChanged(TransactionTableModel *ttm, const uint256 &
    722     notification.invoke(ttm);
    723 }
    724 
    725-static void ShowProgress(TransactionTableModel *ttm, const std::string &title, int nProgress)
    726+static void ShowProgress(TransactionTableModel *ttm, MAYBE_UNUSED const std::string &title, int nProgress)
    

    ---

    ---

    

    arvidn commented at 2:13 pm on September 17, 2018:

    title appears to be unused as well. I think a better approach is to remove the parameter name.
16. practicalswift force-pushed on Sep 21, 2018
17. doc: Add a developer note about MAYBE_UNUSED 8d63c12279
18. 
    practicalswift commented at 2:51 pm on September 21, 2018: contributor

    @arvidn Good points. Now updated. Also added a developer note. Please re-review :-)
19. 
    DrahtBot commented at 11:41 am on September 28, 2018: member

    Coverage	Change (pull 14194)	Reference (master)
    Lines	+0.0375 %	87.0361 %
    Functions	+0.1235 %	84.1130 %
    Branches	+0.0095 %	51.5451 %

20. practicalswift closed this on Nov 12, 2018

    ---

21. practicalswift deleted the branch on Apr 10, 2021
22. DrahtBot locked this on Aug 16, 2022

Contributors
practicalswift DrahtBot Empact ken2812221 arvidn

Labels
Build system