Fix hash overloads for integral types
authorOgnjen Dragoljevic <plamenko@fb.com>
Mon, 16 Oct 2017 14:24:15 +0000 (07:24 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Mon, 16 Oct 2017 14:44:38 +0000 (07:44 -0700)
Summary:
`folly/Hash.h:379:12: error: implicit instantiation of undefined template 'folly::hasher<unsigned long, void>'`
So, folly is unable to hash the very type it returns: `size_t`. Namely, folly has overloads for `uint32_t` and `uint64_t` which on my Mac map to `int` and `long long` respectively. `size_t` on the other hand maps to `long` which is neither.
Rather than overloading library types (which are just typedefs), we should overload all the built-in types: `char`, `short`, `int`, `long`, `long long`, `signed` and `unsigned` variants (with special treatment of `char`).

Reviewed By: yfeldblum, luciang

Differential Revision: D6051600

fbshipit-source-id: d59569dab963cbe0329aa589ff321cfb22308193

folly/Hash.h
folly/test/HashTest.cpp

index 211dc44ed37838896036fca50a5f4e08cb98079d..6d277c16ad4222eff89731071d006bd0fa23c756 100644 (file)
@@ -370,6 +370,21 @@ inline uint32_t hsieh_hash32_str(const std::string& str) {
 
 } // namespace hash
 
+namespace detail {
+template <typename I>
+size_t integral_hash(I const& i) {
+  static_assert(sizeof(I) <= 8, "input type is too wide");
+  if (sizeof(I) <= 4) { // the branch taken is known at compile time
+    auto const i32 = static_cast<int32_t>(i); // as impl accident, sign-extends
+    auto const u32 = static_cast<uint32_t>(i32);
+    return static_cast<size_t>(hash::jenkins_rev_mix32(u32));
+  } else {
+    auto const u64 = static_cast<uint64_t>(i);
+    return static_cast<size_t>(hash::twang_mix64(u64));
+  }
+}
+} // namespace detail
+
 template <class Key, class Enable = void>
 struct hasher;
 
@@ -393,59 +408,85 @@ struct hasher<bool> {
   }
 };
 
-template <> struct hasher<int32_t> {
-  size_t operator()(int32_t key) const {
-    return hash::jenkins_rev_mix32(uint32_t(key));
+template <>
+struct hasher<unsigned long long> {
+  size_t operator()(unsigned long long key) const {
+    return detail::integral_hash(key);
   }
 };
 
-template <> struct hasher<uint32_t> {
-  size_t operator()(uint32_t key) const {
-    return hash::jenkins_rev_mix32(key);
+template <>
+struct hasher<signed long long> {
+  size_t operator()(signed long long key) const {
+    return detail::integral_hash(key);
   }
 };
 
-template <> struct hasher<int16_t> {
-  size_t operator()(int16_t key) const {
-    return hasher<int32_t>()(key); // as impl accident, sign-extends
+template <>
+struct hasher<unsigned long> {
+  size_t operator()(unsigned long key) const {
+    return detail::integral_hash(key);
   }
 };
 
-template <> struct hasher<uint16_t> {
-  size_t operator()(uint16_t key) const {
-    return hasher<uint32_t>()(key);
+template <>
+struct hasher<signed long> {
+  size_t operator()(signed long key) const {
+    return detail::integral_hash(key);
   }
 };
 
-template <> struct hasher<int8_t> {
-  size_t operator()(int8_t key) const {
-    return hasher<int32_t>()(key); // as impl accident, sign-extends
+template <>
+struct hasher<unsigned int> {
+  size_t operator()(unsigned int key) const {
+    return detail::integral_hash(key);
   }
 };
 
-template <> struct hasher<uint8_t> {
-  size_t operator()(uint8_t key) const {
-    return hasher<uint32_t>()(key);
+template <>
+struct hasher<signed int> {
+  size_t operator()(signed int key) const {
+    return detail::integral_hash(key);
   }
 };
 
-template <> struct hasher<char> {
-  using explicit_type =
-      std::conditional<std::is_signed<char>::value, int8_t, uint8_t>::type;
-  size_t operator()(char key) const {
-    return hasher<explicit_type>()(key); // as impl accident, sign-extends
+template <>
+struct hasher<unsigned short> {
+  size_t operator()(unsigned short key) const {
+    return detail::integral_hash(key);
   }
 };
 
-template <> struct hasher<int64_t> {
-  size_t operator()(int64_t key) const {
-    return static_cast<size_t>(hash::twang_mix64(uint64_t(key)));
+template <>
+struct hasher<signed short> {
+  size_t operator()(signed short key) const {
+    return detail::integral_hash(key);
+  }
+};
+
+template <>
+struct hasher<unsigned char> {
+  size_t operator()(unsigned char key) const {
+    return detail::integral_hash(key);
   }
 };
 
-template <> struct hasher<uint64_t> {
-  size_t operator()(uint64_t key) const {
-    return static_cast<size_t>(hash::twang_mix64(key));
+template <>
+struct hasher<signed char> {
+  size_t operator()(signed char key) const {
+    return detail::integral_hash(key);
+  }
+};
+
+// char is different type from both signed char and unsigned char.
+template <>
+struct hasher<char> {
+  using explicit_type = std::conditional<
+      std::is_signed<char>::value,
+      signed char,
+      unsigned char>::type;
+  size_t operator()(char key) const {
+    return detail::integral_hash(explicit_type(key));
   }
 };
 
index dd03242bed071a7ffd7acc1330027f3c97692175..759f9975984cc9bf6c01504253683a491d93f6fc 100644 (file)
@@ -19,6 +19,7 @@
 #include <folly/portability/GTest.h>
 #include <stdint.h>
 #include <unordered_map>
+#include <unordered_set>
 #include <utility>
 
 using namespace folly::hash;
@@ -185,6 +186,37 @@ TEST(Hash, hasher) {
   EXPECT_EQ(get_default(m, 4), 5);
 }
 
+TEST(Hash, integral_types) {
+  // Basically just confirms that things compile ok.
+  std::unordered_set<size_t> hashes;
+  folly::Hash hasher;
+  hashes.insert(hasher((char)1));
+  hashes.insert(hasher((signed char)2));
+  hashes.insert(hasher((unsigned char)3));
+  hashes.insert(hasher((short)4));
+  hashes.insert(hasher((signed short)5));
+  hashes.insert(hasher((unsigned short)6));
+  hashes.insert(hasher((int)7));
+  hashes.insert(hasher((signed int)8));
+  hashes.insert(hasher((unsigned int)9));
+  hashes.insert(hasher((long)10));
+  hashes.insert(hasher((signed long)11));
+  hashes.insert(hasher((unsigned long)12));
+  hashes.insert(hasher((long long)13));
+  hashes.insert(hasher((signed long long)14));
+  hashes.insert(hasher((unsigned long long)15));
+  hashes.insert(hasher((int8_t)16));
+  hashes.insert(hasher((uint8_t)17));
+  hashes.insert(hasher((int16_t)18));
+  hashes.insert(hasher((uint16_t)19));
+  hashes.insert(hasher((int32_t)20));
+  hashes.insert(hasher((uint32_t)21));
+  hashes.insert(hasher((int64_t)22));
+  hashes.insert(hasher((uint64_t)23));
+  hashes.insert(hasher((size_t)24));
+  EXPECT_EQ(24, hashes.size());
+}
+
 // Not a full hasher since only handles one type
 class TestHasher {
  public: