refactor(tidy): Use C++20 contains method

[aureleoules]

C++20 introduced the contains method on containers to check if an element is in the container. It can be used instead of the count method now.
I believe it is easier to understand, as contains directly returns a bool indicating whether the element exists, while count returns the number of occurrences of the element, which then often needs to be compared against 0.
Also, it is slightly more efficient than count for this use-case.

This pull request introduces the clang-tidy check readability-container-contains and fixes the instances where count is being used where contains could be used instead. I used the clang-tidy -fix option to do it automatically.

[DrahtBot]

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	theuni, TheCharlatan, theStack, BrandonOdiwuor

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:

• #29236 (log: Nuke error(...) by maflcko)
• #28948 (v3 transaction policy for anti-pinning by glozow)
• #28886 (refactor: Replace sets of txiter with CTxMemPoolEntryRefs by TheCharlatan)
• #28806 (rpc: Add script verification flags to getdeploymentinfo by ajtowns)
• #28560 (wallet, rpc: FundTransaction refactor by josibake)
• #28201 (Silent Payments: sending by josibake)
• #28170 (p2p: adaptive connections services flags by furszy)
• #28031 (Package Relay 1/3: Introduce TxDownloadManager and improve orphan-handling by glozow)
• #27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
• #27286 (wallet: Keep track of the wallet's own transaction outputs in memory by achow101)
• #27101 (Support JSON-RPC 2.0 when requested by client by pinheadmz)
• #26840 (refactor: importpubkey, importprivkey, importaddress, importmulti, and importdescriptors rpc by KolbyML)
• #26114 (net: Make AddrFetch connections to fixed seeds by mzumsande)
• #25390 (sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es by vasild)

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.

[theuni]

Nice use of tidy!

Concept ACK, assuming ::contains() exists for common stdlibs by now. Waiting to see what c-i thinks.

[sedited]

Concept ACK

[theStack]

Nice! Concept ACK

[BrandonOdiwuor]

Concept ACK using ::contains(Key& key) (C++20) to check for elements

[sedited]

Can the full diff be reproduced with run-clang-tidy -fix? If so, can you post the full commands you are using? I am getting a slightly different diff if I run it myself.

[aureleoules]

Can the full diff be reproduced with run-clang-tidy -fix? If so, can you post the full commands you are using? I am getting a slightly different diff if I run it myself.

Sorry, my initial push did not pass the tidy check because it seems the -fix option does not fix code in macros (all the asserts, Assume, etc.) so I manually fixed these in a second push. This may be the reason why you are not getting the same diff as me.

I don't have the specific command I used on hand but I used the find command to retrieve all cpp and h files, piped to clang-tidy-17 -fix if I remember correctly.

[sedited]

Re #29191 (comment)

Sorry, my initial push did not pass the tidy check because it seems the -fix option does not fix code in macros (all the asserts, Assume, etc.) so I manually fixed these in a second push. This may be the reason why you are not getting the same diff as me.

I see, that is a bit unfortunate. I also found some more things to change: sedited@63fcfbe, but not sure if non-tidy changes should be made here tbh.

[aureleoules]

I see, that is a bit unfortunate. I also found some more things to change: sedited@63fcfbe, but not sure if non-tidy changes should be made here tbh.

Thanks for the commit, I added it. Did you find them manually or with clang-tidy?
I'm not sure why the CI didn't catch them...

[stickies-v]

I support using contains going forward, but I'm not sure for this PR the benefits outweigh the costs?

1. I prefer the contains interface but imo using count is also pretty straightforward, so the benefits seem limited?
2. It conflicts with a large number of PRs
3. It's not a scripted diff

I think I'd prefer removing the clang-tidy check for now and only changing it in places where it doesn't conflict with other PRs, e.g. maybe in tests? And then organically have more contains adoption over time?

[sedited]

Re #29191 (comment)

but I'm not sure for this PR the benefits outweigh the costs?

FWIW I would approve this PR if it would only contain the changes that are automatically picked up by tidy and applied by running with -fix, making resolving potential conflicts purely manual.

[aureleoules]

FWIW I would approve this PR if it would only contain the changes that are automatically picked up by tidy and applied by running with -fix, making resolving potential conflicts purely manual.

Sure, I agree. Closing this pull.