Merge pull request #26 from Exerg/bugfix/identiy_hash

Adding requirement for identity_hash function
Exerg · Dec 16, 2018 · 8983567 · 8983567
2 parents 2b2859a + 9cd69b0
commit 8983567
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 11 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -73,6 +73,10 @@ if (BUILD_TESTS)
     ctulu_create_target(UnorderedMapSingleThreadsTests "WaitFreeCollections" tests/unordered_map/single_thread_test.cpp TEST CXX 17 W_LEVEL 2)
     ctulu_target_warning_from_file(UnorderedMapSingleThreadsTests ${root_dir}/cmake/warnings.txt)
     target_link_libraries(UnorderedMapSingleThreadsTests WaitFreeCollections CONAN_PKG::gtest)
+
+    ctulu_create_target(UtilityTests "WaitFreeCollections" DIRS ${test_dir}/utility/ TEST CXX 17 W_LEVEL 2)
+    ctulu_target_warning_from_file(UtilityTests ${root_dir}/cmake/warnings.txt)
+    target_link_libraries(UtilityTests WaitFreeCollections CONAN_PKG::gtest)
 endif()
 
 #set(CMAKE_VERBOSE_MAKEFILE 1)
diff --git a/appveyor.yml b/appveyor.yml
@@ -29,5 +29,7 @@ build_script:
 
 test_script:
     - ${APPVEYOR_BUILD_FOLDER}/debug_build/bin/UnorderedMapSingleThreadsTests
+    - ${APPVEYOR_BUILD_FOLDER}/debug_build/bin/UtilityTests
     - ${APPVEYOR_BUILD_FOLDER}/release_build/bin/UnorderedMapSingleThreadsTests
+    - ${APPVEYOR_BUILD_FOLDER}/release_build/bin/UtilityTests
 
diff --git a/include/wfc/unordered_map.hpp b/include/wfc/unordered_map.hpp
@@ -36,12 +36,13 @@ namespace wfc
 	}
 
 	template <typename Key>
-	class identity_hash // FIXME: Size doesn't match
+	class identity_hash
 	{
 	public:
-		std::size_t operator()(const Key& k) const noexcept
+		static_assert(std::is_fundamental_v<Key>, "Key should be a fundamental type to use the default hash");
+		Key operator()(const Key& k) const noexcept
 		{
-			return static_cast<std::size_t>(k);
+			return k;
 		}
 	};
 
@@ -173,7 +174,7 @@ namespace wfc
 	template <typename Key, typename Value, typename HashFunction>
 	operation_result unordered_map<Key, Value, HashFunction>::insert(const Key& key, const Value& value)
 	{
-		std::size_t nbr_bits_to_shift = log2_of_power_of_two<hash_size_in_bits>(m_arrayLength);
+		std::size_t nbr_bits_to_shift = log2_of_power_of_two(m_arrayLength);
 
 		std::size_t position;
 		std::size_t failCount;
@@ -258,7 +259,7 @@ namespace wfc
 	template <typename Key, typename Value, typename HashFunction>
 	std::optional<Value> unordered_map<Key, Value, HashFunction>::get(const Key& key)
 	{
-		std::size_t array_pow = log2_of_power_of_two<hash_size_in_bits>(m_arrayLength);
+		std::size_t array_pow = log2_of_power_of_two(m_arrayLength);
 
 		std::size_t position;
 		node_union local{&m_head};
@@ -458,7 +459,7 @@ namespace wfc
 	                                                                                CmpFun&& compare_expected_value,
 	                                                                                AllocFun&& replacing_node)
 	{
-		std::size_t array_pow = log2_of_power_of_two<hash_size_in_bits>(m_arrayLength);
+		std::size_t array_pow = log2_of_power_of_two(m_arrayLength);
 
 		std::size_t position;
 		node_union local{&m_head};

diff --git a/include/wfc/utility/math.hpp b/include/wfc/utility/math.hpp
@@ -1,6 +1,9 @@
 #ifndef WFC_MATH_HPP
 #define WFC_MATH_HPP
 
+#include <limits>
+#include <type_traits>
+
 namespace wfc
 {
 	namespace details
@@ -29,16 +32,19 @@ namespace wfc
 		}
 	} // namespace details
 
-	template <std::size_t HashSizeInBit, typename T>
+	template <typename T>
 	constexpr std::size_t log2_of_power_of_two(T x) noexcept;
 
 	template <typename T>
 	constexpr bool is_power_of_two(T nbr) noexcept;
 
-	template <std::size_t HashSizeInBit, typename T>
+	template <typename T>
 	constexpr std::size_t log2_of_power_of_two(T x) noexcept
 	{
-		return HashSizeInBit - details::clz(x) - 1UL;
+		static_assert(std::is_integral_v<T>, "T should be an integral type");
+		assert(is_power_of_two(x));
+
+		return sizeof(T) * std::numeric_limits<unsigned char>::digits - details::clz(x) - 1UL;
 	}
 
 	template <typename T>

diff --git a/tests/unordered_map/single_thread_test.cpp b/tests/unordered_map/single_thread_test.cpp
@@ -1,5 +1,7 @@
 #include <gtest/gtest.h>
 
+#include <limits>
+
 #include <wfc/unordered_map.hpp>
 
 TEST(WaitFreeHashMap, Construction)
@@ -145,14 +147,15 @@ TEST(WaitFreeHashMap, FullHashMapGet)
 
 	for (std::size_t i = 0; i <= std::numeric_limits<unsigned char>::max(); ++i)
 	{
-		map.insert(static_cast<unsigned char>(i), static_cast<unsigned char>(i));
+		ASSERT_EQ(map.insert(static_cast<unsigned char>(i), static_cast<unsigned char>(i)),
+		          wfc::operation_result::success);
 	}
 
 	ASSERT_EQ(map.size(), std::numeric_limits<unsigned char>::max() + 1);
 
 	for (std::size_t i = 0; i <= std::numeric_limits<unsigned char>::max(); ++i)
 	{
-		unsigned char value = *map.get(static_cast<unsigned char>(i));
+		unsigned char value = map.get(static_cast<unsigned char>(i)).value();
 		ASSERT_EQ(value, i);
 	}
 }

diff --git a/tests/utility/math.cpp b/tests/utility/math.cpp
@@ -0,0 +1,26 @@
+#include <gtest/gtest.h>
+
+#include <wfc/utility/math.hpp>
+
+TEST(Utilty, IsPowerOfTwo)
+{
+	ASSERT_TRUE(wfc::is_power_of_two(2));
+	ASSERT_FALSE(wfc::is_power_of_two(3));
+}
+
+TEST(Utilty, Log2OfPowerOfTwo)
+{
+	ASSERT_EQ(wfc::log2_of_power_of_two(2U), 1U);
+	ASSERT_EQ(wfc::log2_of_power_of_two(4U), 2U);
+	ASSERT_EQ(wfc::log2_of_power_of_two(8U), 3U);
+}
+
+#ifndef NDEBUG
+#define Log2OfPowerOfTwoDeath Log2OfPowerOfTwoDeath
+#else
+#define Log2OfPowerOfTwoDeath DISABLED_Log2OfPowerOfTwoDeath
+#endif
+
+TEST(Utility, Log2OfPowerOfTwoDeath) {
+	EXPECT_DEATH(wfc::log2_of_power_of_two(3U), "");
+}