Recover moved element after failed move insertion/emplace in std::unordered_map

user_1131907 0
Viewed 319

I fill a std::unordered_map using the insert or emplace methods and the move semantics. When a key clash occurs the element is not inserted in the map but the moved element is erased anyway:

#include <unordered_map>
#include <iostream>

int main(){
    std::unordered_map<int, std::string> m;
    m.insert(std::make_pair<int, std::string>(0, "test"));
    std::string s = "test";
    
    // try insert
    auto val = std::make_pair<int, std::string>(0, std::move(s));
    if(m.insert(std::move(val)).second){
        std::cout << "insert successful, ";
    }else{
        std::cout << "insert failed, ";
    }
    std::cout << "s: " << s << ", val.second: " << val.second <<  std::endl;
    
    // try emplace
    s = "test";
    if(m.emplace(0, std::move(s)).second){
        std::cout << "emplace successful, ";
    }else{
        std::cout << "emplace failed, ";
    }
    std::cout << "s: " << s << std::endl;
    
    return 0;
}

Output:

insert failed, s: , val.second:
emplace failed, s:

Thus nothing is inserted but the object (the string in the example) is erased anyway, making it impossible to use it for any other purpose. Possible fixes would be to not use move semantics or to check for the key prior to the insertion/emplacement, both of which have a performance penalty.

To resume, I understand that move semantics implies that a moved object is left in a state which is useful just for destruction, but I don't fully understand why a failed move operation in std::unordered_map should anyway lead to that state, and if there is any code pattern that allows to avoid this without too much performance penalty.

move-semantics unordered-map c++
3 Answers

If using C++17 features is an option, try_emplace will only move the arguments if the key doesn't already exist in the map.

Otherwise, you can have your own version to get (functionally) the same effect, by combining find and emplace (or insert).

Note that this will likely be less efficient than the try_emplace implementation, if it exists (as you have 2 searches through the container if the key isn't in the map).

Hasturkun_202700

As workaround on C++11 you could write proxy function insert, which will look for an element before trying to insert it. For unordered_map the search could take at most O(n) but this is the worst case, normally (or average) take O(1). Something like this:

#include <unordered_map>
#include <iostream>

template <typename T, typename V> 
std::pair<typename T::iterator, bool> insert(T& m, V&& val) 
{ 
    auto res = m.find(val.first);
    if (res == m.end())
        return m.insert(std::forward<V>(val));
    else
        return std::make_pair(res, false);
} 

int main(){
    std::unordered_map<int, std::string> m;
    m.insert(std::make_pair<int, std::string>(0, "test"));
    std::string s = "test";
    
    // try insert
    auto val = std::make_pair<int, std::string>(0, std::move(s));
    if(insert(m, std::move(val)).second)
        std::cout << "insert successful, ";
    else
        std::cout << "insert failed, ";        
    std::cout << "s: " << s << ", val.second: " << val.second <<  std::endl;    
  
    return 0;
}
user_137826690

I think what you're trying to do goes against the true meaning of the move semantic.

With std::move, you declare you're not going to use that object anymore so C++ compiler can optimize various aspects. The function, in this case insert or emplace, is free to do what it wants with the object because you declare it moved, including deleting it. To continue to use the object under certain conditions, e.g. failed insert, the function should give it back to you returning it, possibly also using std::move.

Back to your example, you can't use val or s with std::cout after std::move.

user_34869660
Related