martinus / robin-hood-hashing

Fast & memory efficient hashtable based on robin hood hashing for C++11/14/17/20
https://gitter.im/martinus/robin-hood-hashing
MIT License
1.52k stars 146 forks source link

support remove/erase idiom #18

Closed jcageman closed 5 years ago

jcageman commented 5 years ago

I noticed when applying the remove/erase idiom that the following method is not implemented:

iterator erase ( const_iterator first, const_iterator last );

Reference: https://en.cppreference.com/w/cpp/container/unordered_map/erase Example: cache.erase( std::remove_if( cache.begin(), cache.end(), [&] ( const pair<int,int>& i ) { return i.second == 1 ); } ), cache.end() );

martinus commented 5 years ago

This hashmap implementation can't support this use case unfortunately. it = erase(it) works though, so you can at least use the idom that's shown here: https://en.cppreference.com/w/cpp/container/unordered_map/erase

ed-lam commented 5 years ago

I'm using the idiom you mentioned above. On that page, it says: "The order of the elements that are not erased is preserved. (This makes it possible to erase individual elements while iterating through the container.)".

In other words, I want to erase while iterating the container but never see the same element twice. Is this supported by unordered_flat_map? I'm getting the same element twice.

martinus commented 5 years ago

Hi ed-lam, you shouldn't get the same elements twice. I actually have a test for that in unit_iterators_erase.cpp. Can you submit a short reproducer here? This should work

auto it = map.begin();
while (it != map.end()) {
    it = map.erase(it);
}
martinus commented 5 years ago

Closing until I get a reproducer

ed-lam commented 5 years ago

Here is a minimally working example. It appears the first element is seen again at the end if I delete some elements. This doesn't occur if you change it to std::unordered_map in the example.

robinhood.txt

martinus commented 5 years ago

Thank you for your reproducer! You really stumbled uppon a bug. I could already narrow that bug down to a test case with just two elements. Working on a fix.

martinus commented 5 years ago

Please see #42