refactor: Use std::erase and std::erase_if #36682

pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2610-erase-if changing 13 files +34 −69

maflcko commented at 9:12 AM on October 28, 2026: member
Convert remove and remove_if to the C++20 std::erase and std::erase_if overloads.

The legacy ones can be confusing, because:
- Array-based containers (like vector) must use the erase-remove idiom. Forgetting the erase, or forgetting to pass the end iterator (bugprone-inaccurate-erase) can be a bug, see also #36588 (review). So a call only to "remove" will not erase.
- Node-based containers (like list or set) must not use the idiom. Using it will be a pessimization or won't compile. And correctly not using the idiom reads confusing, because a call to "remove" will actually erase.
- They are more verbose.
Also convert simple iterator-erase (which is verbose, but fine) to the std overload.

refactor: Use std::erase and std::erase_if over list removal or vector erase-remove

The std::list removal or std::vector erase-remove idiom is fine, but the
std overloads can be less confusing and more consistent.

fa93a02563

refactor: Use std::erase and std::erase_if over iterator-based erase

The erase with an iterator is perfectly fine, but the std overloads can
be used as well.

fa87bfe31d

DrahtBot added the label Refactoring on Oct 28, 2026
DrahtBot commented at 9:12 AM on October 28, 2026: contributor


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



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36682
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36682.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36682#author-ralph


Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK hodlinator, jpk68, w0xlt

Concept ACK l0rinc

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
fanquake requested review from stickies-v on Oct 28, 2026
in src/common/args.cpp:166 in fa87bfe31d
```
 162 | @@ -162,7 +163,7 @@ std::list<SectionInfo> ArgsManager::GetUnrecognizedSections() const
 163 |  
 164 |      LOCK(cs_args);
 165 |      std::list<SectionInfo> unrecognized = m_config_sections;
 166 | -    unrecognized.remove_if([](const SectionInfo& appeared){ return available_sections.contains(appeared.m_name); });
 167 | +    std::erase_if(unrecognized, [](const SectionInfo& appeared) { return available_sections.contains(appeared.m_name); });
```
hodlinator commented at 10:05 AM on October 28, 2026:

nit: Have a slight preference for pre-existing approach when it's shorter. Although consistency is also nice.

maflcko commented at 10:17 AM on October 28, 2026:

Yeah, in C++-speak "remove" has two meanings: It means erase for std::list (unless you incorrectly use the iterator-based or range-based remove on the list, in which case it means move) and it means just move for array-based containers. I think consistently using "erase" to mean erase seems better than having to manually decipher what "remove" means in this context.
hodlinator approved
hodlinator commented at 10:10 AM on October 28, 2026: contributor

ACK fa87bfe31dfda2d102c4e0cb5d39813cf84c7d1e

The old erase()+remove() pattern is indeed error-prone, and translating the raw loops into named and shorter constructs is good.
l0rinc commented at 11:44 AM on October 28, 2026: contributor

Concept ACK
jpk68 commented at 2:58 PM on October 28, 2026: contributor

ACK fa87bfe31dfda2d102c4e0cb5d39813cf84c7d1e
DrahtBot requested review from l0rinc on Oct 28, 2026
w0xlt commented at 5:54 PM on October 28, 2026: contributor

ACK fa87bfe31dfda2d102c4e0cb5d39813cf84c7d1e
fanquake merged this on Oct 29, 2026
fanquake closed this on Oct 29, 2026
maflcko deleted the branch on Oct 29, 2026