Page MenuHomePhabricator

scripted-diff: Avoid temporary copies when looping over std::map

Authored by Fabien on Aug 2 2019, 08:14.


The ::value_type of the std::map/std::multimap/std::unordered_map
containers is std::pair<const Key, T>. Dropping the const results in an
unnecessary copy, for example in C++11 range-based loops.
For this I started with a more general scripted diff, then narrowed it
down based on the inspection showing that all actual
map/multimap/unordered_map variables used in loops start with m or have
map in the name.

sed -i -E 's/for \(([^<]*)std::pair<([^c])(.+) : m/for
(\1std::pair<const \2\3 : m/' src/*.cpp src/**/*.cpp
sed -i -E 's/for \(([^<]*)std::pair<([^c])(.+) : (.*)map/for
(\1std::pair<const \2\3 : \4map/' src/*.cpp src/**/*.cpp

Backport of core PR13241

Note to reviewers:
There is a couple of changes not backported from the PR, due to missing
dependencies regarding the wallet label API. A follow-up diff will
backport PR13480 and introduce a clang warning to detect copies in
loops, so I think this is safe to leave it for now.

Test Plan
make check

Diff Detail

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

This kind of stuff should definitively have a linter. clang-tidy seems like good candidate.

This revision is now accepted and ready to land.Aug 3 2019, 07:21

@deadalnix D3796 introduces a new warning that catch these cases (only with clang).