operator= did not set mInfoInc correctly when it already had the corr…

…ect data size (fixes #34)
martinus · Jun 26, 2019 · 3c7d005 · 3c7d005
1 parent 3650895
commit 3c7d005
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 5 deletions.
diff --git a/src/include/robin_hood.h b/src/include/robin_hood.h
@@ -6,7 +6,7 @@
 //                                      _/_____/
 //
 // robin_hood::unordered_map for C++11
-// version 3.3.0
+// version 3.3.1
 // https://github.com/martinus/robin-hood-hashing
 //
 // Licensed under the MIT License <http://opensource.org/licenses/MIT>.
@@ -37,7 +37,7 @@
 // see https://semver.org/
 #define ROBIN_HOOD_VERSION_MAJOR 3 // for incompatible API changes
 #define ROBIN_HOOD_VERSION_MINOR 3 // for adding functionality in a backwards-compatible manner
-#define ROBIN_HOOD_VERSION_PATCH 0 // for backwards-compatible bug fixes
+#define ROBIN_HOOD_VERSION_PATCH 1 // for backwards-compatible bug fixes
 
 #include <algorithm>
 #include <cstdlib>
@@ -573,7 +573,7 @@ struct pair {
 
 // Hash an arbitrary amount of bytes. This is basically Murmur2 hash without caring about big
 // endianness. TODO(martinus) add a fallback for very large strings?
-inline size_t hash_bytes(void const* ptr, size_t const len) noexcept {
+static size_t hash_bytes(void const* ptr, size_t const len) noexcept {
     static constexpr uint64_t m = UINT64_C(0xc6a4a7935bd1e995);
     static constexpr uint64_t seed = UINT64_C(0xe17a1465);
     static constexpr unsigned int r = 47;
@@ -1381,8 +1381,6 @@ class unordered_map
 
             // no need for calloc here because cloneData performs a memcpy.
             mInfo = reinterpret_cast<uint8_t*>(mKeyVals + o.mMask + 1);
-            mInfoInc = o.mInfoInc;
-            mInfoHashShift = o.mInfoHashShift;
             // sentinel is set in cloneData
         }
         Hash::operator=(static_cast<const Hash&>(o));
@@ -1391,6 +1389,8 @@ class unordered_map
         mNumElements = o.mNumElements;
         mMask = o.mMask;
         mMaxNumElementsAllowed = o.mMaxNumElementsAllowed;
+        mInfoInc = o.mInfoInc;
+        mInfoHashShift = o.mInfoHashShift;
         cloneData(o);
 
         return *this;

diff --git a/src/test/app/checksum.h b/src/test/app/checksum.h
@@ -17,6 +17,18 @@ inline uint64_t mix(uint64_t k) {
     return k;
 }
 
+inline uint64_t mix(std::string const& data) {
+    static constexpr uint64_t FNV_offset_basis = UINT64_C(14695981039346656037);
+    static constexpr uint64_t FNV_prime = UINT64_C(1099511628211);
+
+    uint64_t val = FNV_offset_basis;
+    for (size_t i = 0; i < data.size(); ++i) {
+        val ^= static_cast<uint64_t>(data[i]);
+        val *= FNV_prime;
+    }
+    return val;
+}
+
 inline uint64_t mix(CtorDtorVerifier const& cdv) {
     return mix(cdv.val());
 }

diff --git a/src/test/unit/unit_reserve.cpp b/src/test/unit/unit_reserve.cpp
@@ -1,9 +1,12 @@
 #include <robin_hood.h>
 
+#include <app/checksum.h>
 #include <app/doctest.h>
 
 TYPE_TO_STRING(robin_hood::unordered_flat_map<uint64_t, uint64_t>);
 TYPE_TO_STRING(robin_hood::unordered_node_map<uint64_t, uint64_t>);
+TYPE_TO_STRING(robin_hood::unordered_flat_map<std::string, uint64_t>);
+TYPE_TO_STRING(robin_hood::unordered_node_map<std::string, uint64_t>);
 
 TEST_CASE_TEMPLATE("reserve", Map, robin_hood::unordered_flat_map<uint64_t, uint64_t>,
                    robin_hood::unordered_node_map<uint64_t, uint64_t>) {
@@ -14,3 +17,79 @@ TEST_CASE_TEMPLATE("reserve", Map, robin_hood::unordered_flat_map<uint64_t, uint
     map.reserve(820);
     REQUIRE(2047 == map.mask());
 }
+
+TEST_CASE_TEMPLATE("reserve_and_assign", Map, robin_hood::unordered_flat_map<std::string, uint64_t>,
+                   robin_hood::unordered_node_map<std::string, uint64_t>) {
+    Map a = {
+        {"button", {}},
+        {"selectbox-tr", {}},
+        {"slidertrack-t", {}},
+        {"sliderarrowinc-hover", {}},
+        {"text-l", {}},
+        {"title-bar-l", {}},
+        {"checkbox-checked-hover", {}},
+        {"datagridexpand-active", {}},
+        {"icon-waves", {}},
+        {"sliderarrowdec-hover", {}},
+        {"datagridexpand-collapsed", {}},
+        {"sliderarrowinc-active", {}},
+        {"radio-active", {}},
+        {"radio-checked", {}},
+        {"selectvalue-hover", {}},
+        {"huditem-l", {}},
+        {"datagridexpand-collapsed-active", {}},
+        {"slidertrack-b", {}},
+        {"selectarrow-hover", {}},
+        {"window-r", {}},
+        {"selectbox-tl", {}},
+        {"icon-score", {}},
+        {"datagridheader-r", {}},
+        {"icon-game", {}},
+        {"sliderbar-c", {}},
+        {"window-c", {}},
+        {"datagridexpand-hover", {}},
+        {"button-hover", {}},
+        {"icon-hiscore", {}},
+        {"sliderbar-hover-t", {}},
+        {"sliderbar-hover-c", {}},
+        {"selectarrow-active", {}},
+        {"window-tl", {}},
+        {"checkbox-active", {}},
+        {"sliderarrowdec-active", {}},
+        {"sliderbar-active-b", {}},
+        {"sliderarrowdec", {}},
+        {"window-bl", {}},
+        {"datagridheader-l", {}},
+        {"sliderbar-t", {}},
+        {"sliderbar-active-t", {}},
+        {"text-c", {}},
+        {"window-br", {}},
+        {"huditem-c", {}},
+        {"selectbox-l", {}},
+        {"icon-flag", {}},
+        {"sliderbar-hover-b", {}},
+        {"icon-help", {}},
+        {"selectvalue", {}},
+        {"title-bar-r", {}},
+        {"sliderbar-active-c", {}},
+        {"huditem-r", {}},
+        {"radio-checked-active", {}},
+        {"selectbox-c", {}},
+        {"selectbox-bl", {}},
+        {"icon-invader", {}},
+        {"checkbox-checked-active", {}},
+        {"slidertrack-c", {}},
+        {"sliderarrowinc", {}},
+        {"checkbox", {}},
+    };
+
+    Map b;
+    b = a;
+    REQUIRE(b.find("button") != b.end()); // Passes OK.
+
+    Map c;
+    c.reserve(51);
+    c = a;
+    REQUIRE(checksum::map(a) == checksum::map(c));
+    REQUIRE(c.find("button") != c.end()); // Fails.
+}