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.
Concept ACK, this is clearly an improvement.
However, I feel that this issue is also an indication against using explicit types in this situation. Usually the intent with explicit types (in places where auto would be sufficient) is making sure relevant code fails to compile when a type is changed. This however is an example where an incorrect type results in degraded but otherwise undetectable degradation.
@sipa Fair enough, an alternative would be to switch to auto or ::value_type for the associated collections.
Main argument for explicit types in this case is that they are locally self-documenting. I find first and second pretty weak on that front. So I’d prefer value_type over auto.
More arguments re auto in #12120.
This may improve current code but the problem can come again in future code. I’m +1 in following a simple convention that prevents this problem. Personally for range based loops I prefer auto& or const auto& for any kind of container.
utACK a631e95.
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-
utACK 9b72c988a0050d8932275c74c60928918ee7ef71
Some of these look like they might have a performance impact too.
The discussion on auto/::value_type/explicit type is probably one to hold elsewhere; this is a clear improvement.
