Merge pull request #21 from tinloaf/fix_emplace_hint

Fix emplace_hint for dense hash sets

sparsehash/internal/densehashtable.h

@@ -1021,18 +1021,36 @@ class dense_hashtable {
     return insert_noresize(std::forward<K>(key), std::forward<K>(key), std::forward<Args>(args)...);
   }
 
-  template <typename K, typename... Args>
-  std::pair<iterator, bool> emplace_hint(const_iterator hint, K&& key, Args&&... args) {
+  /* Overload for maps: Here, K != V, and we need to pass hint->first to the equal() function. */
+  template <typename K, typename... Args, typename KeyCopy = Key>
+  typename std::enable_if<!std::is_same<KeyCopy, Value>::value,
+                          std::pair<iterator, bool>>::type
+  emplace_hint(const_iterator hint, K&& key, Args&&... args) {
     resize_delta(1);
 
-    if (equals(key, hint->first)) {
+    if ((hint != this->end()) && (equals(key, hint->first))) {
         return {iterator(this, const_cast<pointer>(hint.pos), const_cast<pointer>(hint.end), false), false};
     }
 
     // here we push key twice as we need it once for the indexing, and the rest of the params are for the emplace itself
     return insert_noresize(std::forward<K>(key), std::forward<K>(key), std::forward<Args>(args)...);
   }
 
+  /* Overload for sets: Here, K == V, and we need to pass *hint to the equal() function. */
+  template <typename K, typename... Args, typename KeyCopy = Key>
+  typename std::enable_if<std::is_same<KeyCopy, Value>::value,
+                          std::pair<iterator, bool>>::type
+  emplace_hint(const_iterator hint, K&& key, Args&&... args) {
+    resize_delta(1);
+
+    if ((hint != this->end()) && (equals(key, *hint))) {
+      return {iterator(this, const_cast<pointer>(hint.pos), const_cast<pointer>(hint.end), false), false};
+    }
+
+    // here we push key twice as we need it once for the indexing, and the rest of the params are for the emplace itself
+    return insert_noresize(std::forward<K>(key), std::forward<K>(key), std::forward<Args>(args)...);
+  }
+
   // When inserting a lot at a time, we specialize on the type of iterator
   template <class InputIterator>
   void insert(InputIterator f, InputIterator l) {

tests/CMakeLists.txt

@@ -9,7 +9,9 @@ add_executable(sparsehash_unittests
     hashtable_unittests.cc
     hashtable_c11_unittests.cc
     fixture_unittests.cc
-    allocator_unittests.cc)
+    allocator_unittests.cc
+    dense_hash_set_unittests.cc
+    dense_hash_map_unittests.cc)
 
 add_executable(bench bench.cc)
 

tests/dense_hash_map_unittests.cc

@@ -0,0 +1,21 @@
+//
+// Created by Lukas Barth on 17.04.18.
+//
+
+#include "gtest/gtest.h"
+#include "sparsehash/dense_hash_map"
+
+using google::dense_hash_map;
+
+TEST(DenseHashMap, TestEmplaceHint) {
+	dense_hash_map<int, const char *> map;
+	map.set_empty_key(0);
+
+	const char * str1 = "Hello";
+
+	map.insert({42, str1});
+	auto it = map.begin();
+	map.emplace_hint(it, 1701, "World");
+
+	ASSERT_EQ(map.size(), 2);
+}

tests/dense_hash_set_unittests.cc

@@ -0,0 +1,38 @@
+//
+// Created by Lukas Barth on 17.04.18.
+//
+
+#include "gtest/gtest.h"
+#include "sparsehash/dense_hash_set"
+
+using google::dense_hash_set;
+
+TEST(DenseHashSet, TestEmplaceHint) {
+	dense_hash_set<const char *> set;
+	set.set_empty_key(nullptr);
+
+	const char * str1 = "Hello";
+	const char * str2 = "World";
+
+	set.insert(str1);
+	auto it = set.begin();
+	set.emplace_hint(it, str2);
+
+	ASSERT_EQ(set.size(), 2ul);
+}
+
+TEST(DenseHashSet, TestEmplaceHintAfterDelete) {
+	dense_hash_set<const char *> set;
+
+	const char * deleted_ptr = "";
+	const char * str1 = "Hello";
+
+	set.set_empty_key(nullptr);
+	set.set_deleted_key(deleted_ptr);
+
+	auto insertion_result = set.insert(str1);
+	auto str1_inserted_it = insertion_result.first;
+
+	auto deleted_iterator = set.erase(str1_inserted_it);
+	set.emplace_hint(deleted_iterator, str1);
+}