2

Here's a bit of code which is supposed to filter out the elements of a map which satisfy a predicate, into a new map (MCVE-fied):

#include <algorithm>
#include <unordered_map>
#include <iostream>

using namespace std;

int main() {
    unordered_map<string, int> m = { { "hello", 1 }, { "world", 2 } };
    auto p = [](const decltype(m)::value_type& e) { return e.second == 2; };
    const auto& m2(m);
    auto m3(m2);
    auto it = remove_if(m3.begin(), m3.end(), p);
    m3.erase(it, m3.end());
    cout << "m3.size() = " << m3.size() << endl;
    return 0;
}

Compilation fails on the remove_if() line, and I get:

In file included from /usr/include/c++/4.9/utility:70:0,
                 from /usr/include/c++/4.9/algorithm:60,
                 from /tmp/b.cpp:1:
/usr/include/c++/4.9/bits/stl_pair.h: In instantiation of ‘std::pair<_T1, _T2>& std::pair<_T1, _T2>::operator=(std::pair<_T1, _T2>&&) [with _T1 = const std::basic_string<char>; _T2 = int]’:
/usr/include/c++/4.9/bits/stl_algo.h:868:23:   required from ‘_ForwardIterator std::__remove_if(_ForwardIterator, _ForwardIterator, _Predicate) [with _ForwardIterator = std::__detail::_Node_iterator<std::pair<const std::basic_string<char>, int>, false, true>; _Predicate = __gnu_cxx::__ops::_Iter_pred<main()::<lambda(const value_type&)> >]’
/usr/include/c++/4.9/bits/stl_algo.h:937:47:   required from ‘_FIter std::remove_if(_FIter, _FIter, _Predicate) [with _FIter = std::__detail::_Node_iterator<std::pair<const std::basic_string<char>, int>, false, true>; _Predicate = main()::<lambda(const value_type&)>]’
/tmp/b.cpp:12:48:   required from here
/usr/include/c++/4.9/bits/stl_pair.h:170:8: error: passing ‘const std::basic_string<char>’ as ‘this’ argument of ‘std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’ discards qualifiers [-fpermissive]
  first = std::forward<first_type>(__p.first);
        ^

Why is this happening? remove_if should not need non-const map keys (strings in this case) - if I am not mistaken. Perhaps the autos assume somehow I want non-const iterators? If so, what do I do other tha spelling out the type (I want to avoid that since this code needs to be templated).

einpoklum
  • 118,144
  • 57
  • 340
  • 684

3 Answers3

8

Your example fails even without all those intermediate variables.

unordered_map<string, int> m = { { "hello", 1 }, { "world", 2 } };
auto p = [](const decltype(m)::value_type& e) { return e.second == 2; };  
auto it = remove_if(m.begin(), m.end(), p);

The code above will fail with the same errors. You can't use remove_if with associative containers because the algorithm works by moving elements that satisfy your predicate to the end of the container. But how would you reorder an unordered_map?

Write a loop for erasing elements

for(auto it = m.begin(); it != m.end();)
{
  if(p(*it)) it = m.erase(it);
  else ++it;
}

Or you could package that into an algorithm

template<typename Map, typename Predicate>
void map_erase_if(Map& m, Predicate const& p)
{
    for(auto it = m.begin(); it != m.end();)
    {
      if(p(*it)) it = m.erase(it);
      else ++it;
    }
}

If you have a standard library implementation that implements the uniform container erasure library fundamentals extensions, then you have an algorithm similar to the one above in the std::experimental namespace.

Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • Unfortunately I'm stuck with C++11 with GCC 4.9.3 right now since I'm working on CUDA code. But - thanks. – einpoklum Mar 09 '16 at 19:24
2

Don't use std::remove_if for node-based containers. The algorithm attempts to permute the collection, which you either cannot do (for associative containers) or which is inefficient (for lists).

For associative containers, you'll need a normal loop:

for (auto it = m.begin(); it != m.end(); )
{
    if (it->second == 2) { m.erase(it++); }
    else                 { ++it;          }
}

If you're removing from a list, use the remove member function instead, which takes a predicate.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
0

From cppreference:

Removing is done by shifting (by means of move assignment) the elements in the range in such a way that the elements that are not to be removed appear in the beginning of the range.

You can't reorder the elements in an associative container, for the unordered_map this doesn't make sense because moving the elements to the end doesn't mean anything, they are looked up by keys anyway.

SirGuy
  • 10,660
  • 2
  • 36
  • 66
  • Can you mention the source of this quote? Also, if this is the case, why isn't remove_if complaining about a lack of some appropriate type trait on the map, rather than confusing me with string constness complaints? – einpoklum Mar 09 '16 at 18:56
  • @einpoklum Compiler error messages are a whole other beast, remove_if doesn't know what the error is from a higher level (telling you the container is associative), just that it can't perform that assignment to a const string. – SirGuy Mar 09 '16 at 19:10