Page MenuHomePhabricator

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

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

Details

Summary
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.

-BEGIN VERIFY SCRIPT-
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
-END VERIFY SCRIPT-

Backport of core PR13241
https://github.com/bitcoin/bitcoin/pull/13241/files

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

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

Event Timeline

Fabien created this revision.Aug 2 2019, 08:14
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 2 2019, 08:14
deadalnix accepted this revision.Aug 3 2019, 07:21

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
Fabien added a comment.Aug 3 2019, 19:44

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