Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

unexpected copies with foreach over a map

I am trying to loop over the entries of a map, and I get unexpected copies. Here is the program:

#include <iostream>
#include <map>
#include <string>

struct X
{
    X()
    {
        std::cout << "default constructor\n";
    }

    X(const X&)
    {
        std::cout << "copy constructor\n";
    }
};

int main()
{
    std::map<int, X> numbers = {{1, X()}, {2, X()}, {3, X()}};
    std::cout << "STARTING LOOP\n";
    for (const std::pair<int, X>& p : numbers)
    {
    }
    std::cout << "ENDING LOOP\n";
}

And here is the output:

default constructor
copy constructor
default constructor
copy constructor
default constructor
copy constructor
copy constructor
copy constructor
copy constructor
STARTING LOOP
copy constructor
copy constructor
copy constructor
ENDING LOOP

Why do I get three copies inside the loop? The copies disappear if I use type inference:

for (auto&& p : numbers)
{
}

What's going on here?

like image 454
fredoverflow Avatar asked Dec 20 '13 17:12

fredoverflow


3 Answers

The value type of map<K,V> is pair<const K,V>; so your loop needs to convert pair<const int,X> to pair<int,X>, copying both the key and the value, to give you a reference to that type.

Using the correct type (specified explicitly, or deduced with auto) will remove the copies.

like image 151
Mike Seymour Avatar answered Nov 15 '22 07:11

Mike Seymour


The value_type in a std::map<K, V> is not std::pair<K, V> but rather std::pair<K const, V>. That is, you can't change the key component of the elements. If the compiler finds a request to get a std::pair<K, V> const& from a std::pair<K const, K> const& it uses the conversion constructor of std::pair<...> and creates a suitable temporary.

I'd recommend using value_type instead to avoid any differences:

for (std::map<int, X>::value_type const& p: numbers)
    ...

Alternatively you can have the type be deduced:

for (auto const& p: number)
    ...
like image 40
Dietmar Kühl Avatar answered Nov 15 '22 05:11

Dietmar Kühl


The copies are due to the fact that you are iterating a map but binding the wrong reference type. The value_type for that map is std::pair<const int, X> (note the const). Because they are not the same type, the compiler is creating a temporary and binding the reference.

You can (probably should) iterator using auto& or const auto& in most cases, which will avoid this kind of issues. If you want spell out the type, you can use the nested value_type or the exact type:

for (const std::map<int,X>::value_type& r : numbers) {
// or
for (const std::pair<const int,X>& r : numbers) {

The preference should be: const auto& r then const std::map<...>::value_type& and then const std::pair<const int, X>&. Note that the further to the right in the list, the more knowledge you are providing and the less you allow the compiler to help you.

like image 4
David Rodríguez - dribeas Avatar answered Nov 15 '22 06:11

David Rodríguez - dribeas