SkTHash: hash from fnptr to functor type

Passing &SkGoodHash to SkTHashMap and SkTHashSet doesn't guarantee that it's actually instantiated.  Using a functor does.

BUG=skia:

Review URL: https://codereview.chromium.org/1405053002

diff --git a/dm/DM.cpp b/dm/DM.cpp
index 4b32d2f..ccff84c 100644 (file)
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -144,7 +144,11 @@ struct Gold : public SkString {
         this->append(name);
         this->append(md5);
     }
-    static uint32_t Hash(const Gold& g) { return SkGoodHash((const SkString&)g); }
+    struct Hash {
+        uint32_t operator()(const Gold& g) const {
+            return SkGoodHash()((const SkString&)g);
+        }
+    };
 };
 static SkTHashSet<Gold, Gold::Hash> gGold;
 

diff --git a/include/private/SkChecksum.h b/include/private/SkChecksum.h
index 8eb1766..cbc8a73 100644 (file)
--- a/include/private/SkChecksum.h
+++ b/include/private/SkChecksum.h
@@ -182,17 +182,20 @@ public:
 
 // SkGoodHash should usually be your first choice in hashing data.
 // It should be both reasonably fast and high quality.
-
-template <typename K>
-uint32_t SkGoodHash(const K& k) {
-    if (sizeof(K) == 4) {
+struct SkGoodHash {
+    template <typename K>
+    SK_WHEN(sizeof(K) == 4, uint32_t) operator()(const K& k) const {
         return SkChecksum::Mix(*(const uint32_t*)&k);
     }
-    return SkChecksum::Murmur3(&k, sizeof(K));
-}
 
-inline uint32_t SkGoodHash(const SkString& k) {
-    return SkChecksum::Murmur3(k.c_str(), k.size());
-}
+    template <typename K>
+    SK_WHEN(sizeof(K) != 4, uint32_t) operator()(const K& k) const {
+        return SkChecksum::Murmur3(&k, sizeof(K));
+    }
+
+    uint32_t operator()(const SkString& k) const {
+        return SkChecksum::Murmur3(k.c_str(), k.size());
+    }
+};
 
 #endif

diff --git a/include/private/SkTHash.h b/include/private/SkTHash.h
index 53d5275..8a644e3 100644 (file)
--- a/include/private/SkTHash.h
+++ b/include/private/SkTHash.h
@@ -191,7 +191,7 @@ private:
 
 // Maps K->V.  A more user-friendly wrapper around SkTHashTable, suitable for most use cases.
 // K and V are treated as ordinary copyable C++ types, with no assumed relationship between the two.
-template <typename K, typename V, uint32_t(*HashK)(const K&) = &SkGoodHash>
+template <typename K, typename V, typename HashK = SkGoodHash>
 class SkTHashMap : SkNoncopyable {
 public:
     SkTHashMap() {}
@@ -247,14 +247,14 @@ private:
         K key;
         V val;
         static const K& GetKey(const Pair& p) { return p.key; }
-        static uint32_t Hash(const K& key) { return HashK(key); }
+        static uint32_t Hash(const K& key) { return HashK()(key); }
     };
 
     SkTHashTable<Pair, K> fTable;
 };
 
 // A set of T.  T is treated as an ordiary copyable C++ type.
-template <typename T, uint32_t(*HashT)(const T&) = &SkGoodHash>
+template <typename T, typename HashT = SkGoodHash>
 class SkTHashSet : SkNoncopyable {
 public:
     SkTHashSet() {}
@@ -293,7 +293,7 @@ public:
 private:
     struct Traits {
         static const T& GetKey(const T& item) { return item; }
-        static uint32_t Hash(const T& item) { return HashT(item); }
+        static uint32_t Hash(const T& item) { return HashT()(item); }
     };
     SkTHashTable<T, T, Traits> fTable;
 };

diff --git a/src/pdf/SkPDFCanon.h b/src/pdf/SkPDFCanon.h
index 988b285..3d2ba6a 100644 (file)
--- a/src/pdf/SkPDFCanon.h
+++ b/src/pdf/SkPDFCanon.h
@@ -102,10 +102,12 @@ private:
             SkASSERT(rhs.fPtr);
             return *fPtr == *rhs.fPtr;
         }
-        static uint32_t Hash(const WrapGS& w) {
-            SkASSERT(w.fPtr);
-            return w.fPtr->hash();
-        }
+        struct Hash {
+            uint32_t operator()(const WrapGS& w) const {
+                SkASSERT(w.fPtr);
+                return w.fPtr->hash();
+            }
+        };
     };
     SkTHashSet<WrapGS, WrapGS::Hash> fGraphicStateRecords;
 

diff --git a/tests/ChecksumTest.cpp b/tests/ChecksumTest.cpp
index 0302ea9..c095a5a 100644 (file)
--- a/tests/ChecksumTest.cpp
+++ b/tests/ChecksumTest.cpp
@@ -52,14 +52,14 @@ DEF_TEST(Checksum, r) {
 }
 
 DEF_TEST(GoodHash, r) {
-    ASSERT(SkGoodHash(( int32_t)4) ==  614249093);  // 4 bytes.  Hits SkChecksum::Mix fast path.
-    ASSERT(SkGoodHash((uint32_t)4) ==  614249093);  // (Ditto)
+    ASSERT(SkGoodHash()(( int32_t)4) ==  614249093);  // 4 bytes.  Hits SkChecksum::Mix fast path.
+    ASSERT(SkGoodHash()((uint32_t)4) ==  614249093);  // (Ditto)
 
     // None of these are 4 byte sized, so they use SkChecksum::Murmur3, not SkChecksum::Mix.
-    ASSERT(SkGoodHash((uint64_t)4) == 3491892518);
-    ASSERT(SkGoodHash((uint16_t)4) ==  899251846);
-    ASSERT(SkGoodHash( (uint8_t)4) ==  962700458);
+    ASSERT(SkGoodHash()((uint64_t)4) == 3491892518);
+    ASSERT(SkGoodHash()((uint16_t)4) ==  899251846);
+    ASSERT(SkGoodHash()( (uint8_t)4) ==  962700458);
 
     // Tests SkString is correctly specialized.
-    ASSERT(SkGoodHash(SkString("Hi")) == 55667557);
+    ASSERT(SkGoodHash()(SkString("Hi")) == 55667557);
 }

diff --git a/tests/HashTest.cpp b/tests/HashTest.cpp
index c90c377..c9b1bc9 100644 (file)
--- a/tests/HashTest.cpp
+++ b/tests/HashTest.cpp
@@ -120,14 +120,16 @@ private:
     uint32_t* fCounter;
 };
 
-uint32_t hash_copy_counter(const CopyCounter&) {
-    return 0; // let them collide, what do we care?
-}
+struct HashCopyCounter {
+    uint32_t operator()(const CopyCounter&) const {
+        return 0; // let them collide, what do we care?
+    }
+};
 
 }
 
 DEF_TEST(HashSetCopyCounter, r) {
-    SkTHashSet<CopyCounter, hash_copy_counter> set;
+    SkTHashSet<CopyCounter, HashCopyCounter> set;
 
     uint32_t globalCounter = 0;
     CopyCounter copyCounter1(1, &globalCounter);