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.5k stars 142 forks source link

The return type of functions that use hints should be consistent with std #138

Closed acd1034 closed 2 years ago

acd1034 commented 2 years ago

The return types of the member functions that take a hint to insert an element are different between robin_hood::unordered_map and std::unordered_map. As a result, to use robin_hood::unordered_map, we need to do more than just replace std::unordered_map with robin_hood::unordered_map. I feel that this is preventing users from using robot_hood::unordered_map.

What I would like to do with this pull request

Make the return type of member functions of robin_hood::unordered_map that take a hint to insert an element consistent with std::unordered_map.

Specific proposals

The member functions of robin_hood::detail::Table that take a hint to insert an element are currently declared as follows.

// robin_hood::detail::Table

// emplace_hint
template <typename... Args>
iterator emplace_hint(Args&&... args);

// insert
std::pair<iterator, bool> insert(const_iterator hint, const value_type& keyval);
std::pair<iterator, bool> insert(const_iterator hint, value_type&& keyval);

// try_emplace
template <typename... Args>
std::pair<iterator, bool> try_emplace(const_iterator hint, const key_type& key, Args&&... args);
template <typename... Args>
std::pair<iterator, bool> try_emplace(const_iterator hint, key_type&& key, Args&&... args);

// insert_or_assign
template <typename Mapped>
std::pair<iterator, bool> insert_or_assign(const_iterator hint, const key_type& key, Mapped&& obj);
template <typename Mapped>
std::pair<iterator, bool> insert_or_assign(const_iterator hint, key_type&& key, Mapped&& obj);

On the other hand, the corresponding functions of std::unordered_map are declared as follows.

Reference: Working Draft, Standard for Programming Language C++ - ISO C++ standards committee

// std::unordered_map

// emplace_hint
template<typename... Args>
iterator emplace_hint(const_iterator position, Args&&... args);

// insert
iterator insert(const_iterator hint, const value_type& keyval);
iterator insert(const_iterator hint, value_type&& keyval);

// try_emplace
template<typename... Args>
iterator try_emplace(const_iterator hint, const key_type& key, Args&&... args);
template<typename... Args>
iterator try_emplace(const_iterator hint, key_type&& key, Args&&... args);

// insert_or_assign
template<typename Mapped>
iterator insert_or_assign(const_iterator hint, const key_type& key, Mapped&& obj);
template<typename Mapped>
iterator insert_or_assign(const_iterator hint, key_type&& key, Mapped&& obj);

I would like to replace the above declaration with the one below. Some may argue that making the above change makes it impossible to know whether an element was actually inserted or not. If this slight difference between robin_hood::unordered_map and std::unordered_map is intentional, please abandon this proposal.

Best regards.

martinus commented 2 years ago

Thanks @acd1034 for the contribution! This was a copy & paste error on my side, and not intentional. No idea why the standard just returns an iterator instead of the pair, but it's better to stick with that.