Remove unnecessary and broken hash specializations

Summary:
No need to specialize std::hash for std::basic_string; the STL already does
this. Plus, the specializations were broken in multiple ways:

1. std::hash does not treat const char* specially, so it just hashes the
pointer value (contrasting the old __gnu_cxx::hash, which hashed a C-string)

2. Either way, using __gnu_cxx::hash<const char*> for std::string is broken,
as it won't hash anything past the first null byte.

Also, make sure fbstring gets the same (full) specializations as std::string
does in the standard.

Test Plan: fbconfig -r folly && fbmake runtests_opt, also tests in tupperware/agent which is still using __gnu_cxx::hash_map with string keys.

Reviewed By: pgriess@fb.com, andrei.alexandrescu@fb.com

Subscribers: njormrod, jhj, kma, lesha

FB internal diff: D1426479

diff --git a/folly/FBString.h b/folly/FBString.h
index 78c0409064e0179c0f8a463e9d61846c271336b5..e80eba5914434885ffb28a82f37ca0d8baa339ff 100644 (file)
--- a/folly/FBString.h
+++ b/folly/FBString.h
@@ -2417,47 +2417,40 @@ _GLIBCXX_END_NAMESPACE_VERSION
 //
 // Handle interaction with different C++ standard libraries, which
 // expect these types to be in different namespaces.
-namespace std {
 
-template <class C>
-struct hash<folly::basic_fbstring<C> > : private hash<const C*> {
-  size_t operator()(const folly::basic_fbstring<C> & s) const {
-    return hash<const C*>::operator()(s.c_str());
-  }
-};
+#define FOLLY_FBSTRING_HASH1(T) \
+  template <> \
+  struct hash< ::folly::basic_fbstring<T> > { \
+    size_t operator()(const ::folly::fbstring& s) const { \
+      return ::folly::hash::fnv32_buf(s.data(), s.size()); \
+    } \
+  };
 
-template <>
-struct hash< ::folly::fbstring> {
-  size_t operator()(const ::folly::fbstring& s) const {
-    return ::folly::hash::fnv32_buf(s.data(), s.size());
-  }
-};
+// The C++11 standard says that these four are defined
+#define FOLLY_FBSTRING_HASH \
+  FOLLY_FBSTRING_HASH1(char) \
+  FOLLY_FBSTRING_HASH1(char16_t) \
+  FOLLY_FBSTRING_HASH1(char32_t) \
+  FOLLY_FBSTRING_HASH1(wchar_t)
 
-}
+namespace std {
+
+FOLLY_FBSTRING_HASH
+
+}  // namespace std
 
-#ifndef _LIBSTDCXX_FBSTRING
 #if FOLLY_HAVE_DEPRECATED_ASSOC
 #if defined(_GLIBCXX_SYMVER) && !defined(__BIONIC__)
 namespace __gnu_cxx {
 
-template <class C>
-struct hash<folly::basic_fbstring<C> > : private hash<const C*> {
-  size_t operator()(const folly::basic_fbstring<C> & s) const {
-    return hash<const C*>::operator()(s.c_str());
-  }
-};
+FOLLY_FBSTRING_HASH
 
-template <>
-struct hash< ::folly::fbstring> {
-  size_t operator()(const ::folly::fbstring& s) const {
-    return ::folly::hash::fnv32_buf(s.data(), s.size());
-  }
-};
-
-}
+}  // namespace __gnu_cxx
 #endif // _GLIBCXX_SYMVER && !__BIONIC__
 #endif // FOLLY_HAVE_DEPRECATED_ASSOC
-#endif // _LIBSTDCXX_FBSTRING
+
+#undef FOLLY_FBSTRING_HASH
+#undef FOLLY_FBSTRING_HASH1
 
 #endif // _LIBSTDCXX_FBSTRING
 

diff --git a/folly/String.h b/folly/String.h
index 563f5313ecc04a63c4fb739763a819a93026c1c6..3df3f9c41a15758a90bed49cb9e244c718f65752 100644 (file)
--- a/folly/String.h
+++ b/folly/String.h
@@ -524,36 +524,6 @@ inline void toLowerAscii(MutableStringPiece str) {
 
 } // namespace folly
 
-// Hash functions to make std::string usable with e.g. hash_map
-//
-// Handle interaction with different C++ standard libraries, which
-// expect these types to be in different namespaces.
-namespace std {
-
-template <class C>
-struct hash<std::basic_string<C> > : private hash<const C*> {
-  size_t operator()(const std::basic_string<C> & s) const {
-    return hash<const C*>::operator()(s.c_str());
-  }
-};
-
-}
-
-#if FOLLY_HAVE_DEPRECATED_ASSOC
-#if defined(_GLIBCXX_SYMVER) && !defined(__BIONIC__)
-namespace __gnu_cxx {
-
-template <class C>
-struct hash<std::basic_string<C> > : private hash<const C*> {
-  size_t operator()(const std::basic_string<C> & s) const {
-    return hash<const C*>::operator()(s.c_str());
-  }
-};
-
-}
-#endif
-#endif
-
 // Hook into boost's type traits
 namespace boost {
 template <class T>