refactor: Fix clang-tidy readability-const-return-type violations #26935

1. 
   maflcko commented at 3:06 pm on January 20, 2023: member

   This comes up during review, so instead of wasting review cycles on this, just enforce it via CI
2. 
   DrahtBot commented at 3:06 pm on January 20, 2023: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	stickies-v, hebasto

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #26889 (refactor: wallet, remove global ‘ArgsManager’ dependency by furszy)
   • #26642 (clang-tidy: Add more performance-* checks and related fixes by hebasto)
   • #26627 (wallet: Migrate non-HD keys with single combo containing a list of keys by achow101)
   • #26596 (wallet: Migrate legacy wallets to descriptor wallets without requiring BDB by achow101)
   • #26152 (Bump unconfirmed ancestor transactions to target feerate by Xekyo)
   • #26008 (wallet: cache IsMine scriptPubKeys to improve performance of wallets with a lot of non-ranged descriptors by achow101)
   • #22693 (RPC/Wallet: Add “use_txids” to output of getaddressinfo by luke-jr)

   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.

3. DrahtBot added the label Refactoring on Jan 20, 2023
4. 
   hebasto commented at 3:23 pm on January 20, 2023: member

   Concept ACK.
5. maflcko force-pushed on Jan 20, 2023
6. 
   maflcko commented at 4:10 pm on January 20, 2023: member

   For docs: https://clang.llvm.org/extra/clang-tidy/checks/readability/const-return-type.html
7. 
   hebasto commented at 9:05 pm on January 20, 2023: member

   MSVC linker error:

   0libbitcoin_node.lib(validationinterface.obj) : error LNK2019: unresolved external symbol "class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const __cdecl RemovalReasonToString(enum MemPoolRemovalReason const &)" (?RemovalReasonToString@@YA?BV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@AEBW4MemPoolRemovalReason@@@Z) referenced in function "public: __cdecl `public: void __cdecl CMainSignals::TransactionRemovedFromMempool(class std::shared_ptr<class CTransaction const > const &,enum MemPoolRemovalReason,unsigned __int64)'::`4'::<lambda_2>::operator()(void)const " (??R<lambda_2>@?3??TransactionRemovedFromMempool@CMainSignals@@QEAAXAEBV?$shared_ptr@$$CBVCTransaction@@@std@@W4MemPoolRemovalReason@@_K@Z@QEBA@XZ) [C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\build_msvc\bitcoind\bitcoind.vcxproj]
   

8. maflcko force-pushed on Jan 21, 2023
9. 
   maflcko commented at 11:38 am on January 21, 2023: member

   Thanks, force pushed to remove const from validationinterface.cpp as well.
10. hebasto approved
11. 
    hebasto commented at 12:51 pm on January 21, 2023: member

    ACK fa7767ae278baa7074735c9eec070d992d2884ff
12. maflcko force-pushed on Feb 1, 2023
13. Fix clang-tidy readability-const-return-type violations fa451d4b60
14. maflcko force-pushed on Feb 1, 2023
15. fanquake requested review from aureleoules on Feb 1, 2023
16. fanquake requested review from stickies-v on Feb 1, 2023
17. fanquake requested review from hebasto on Feb 1, 2023
18. stickies-v approved
19. 
    stickies-v commented at 12:36 pm on February 1, 2023: contributor

    utACK fa451d4b6

    I verified that all the removed const qualifiers were top-level, and as such can be safely removed. Checking this with clang-tidy makes the review process more efficient.

20. in src/test/fuzz/util.h:50 in fa451d4b60

    46@@ -47,7 +47,7 @@ size_t CallOneOf(FuzzedDataProvider& fuzzed_data_provider, Callables... callable
    47 template <typename Collection>
    48 auto& PickValue(FuzzedDataProvider& fuzzed_data_provider, Collection& col)
    49 {
    50-    const auto sz = col.size();
    51+    auto sz{col.size()};
    

    ---

    ---

    

    hebasto commented at 12:54 pm on February 1, 2023:

    A note: it looks not related to the readability-const-return-type check, but it does a few lines below in ConsumeIntegralInRange<decltype(sz)>(0, sz - 1).

    ---

    

    maflcko commented at 1:16 pm on February 1, 2023:

    Correct, decltype(sz) is const ..., which will be the return value of ConsumeIntegralInRange, and thus cause the readability-const-return-type fail
21. hebasto approved
22. 
    hebasto commented at 1:03 pm on February 1, 2023: member

    ACK fa451d4b60ee0538b3ea6b946740a64734b35b6d.

    Maybe add a commit to address #26705 (review) as well?

23. fanquake merged this on Feb 1, 2023
24. fanquake closed this on Feb 1, 2023

    ---

25. sidhujag referenced this in commit bd399601bd on Feb 1, 2023
26. maflcko deleted the branch on Feb 2, 2023
27. bitcoin locked this on Feb 2, 2024

Contributors
maflcko DrahtBot hebasto stickies-v

Review Requested
aureleoules

Labels
Refactoring