[libc++] Simplify the implementation of std::hash #140407
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philnik777
force-pushed
the
simplify_hash
branch
2 times, most recently
from
f545ed4 to
3a1f7ef
Compare
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesInstead of providing full specializations of Full diff: https://github.com/llvm/llvm-project/pull/140407.diff 1 Files Affected:
diff --git a/libcxx/include/__functional/hash.h b/libcxx/include/__functional/hash.h
index f9f7d2c767caa..35820455ca820 100644
--- a/libcxx/include/__functional/hash.h
+++ b/libcxx/include/__functional/hash.h
@@ -19,6 +19,8 @@
#include <__type_traits/invoke.h>
#include <__type_traits/is_constructible.h>
#include <__type_traits/is_enum.h>
+#include <__type_traits/is_floating_point.h>
+#include <__type_traits/is_integral.h>
#include <__type_traits/underlying_type.h>
#include <__utility/pair.h>
#include <__utility/swap.h>
@@ -345,122 +347,43 @@ struct hash<_Tp*> : public __unary_function<_Tp*, size_t> {
}
};
-template <>
-struct hash<bool> : public __unary_function<bool, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(bool __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-template <>
-struct hash<char> : public __unary_function<char, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(char __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-template <>
-struct hash<signed char> : public __unary_function<signed char, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(signed char __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-template <>
-struct hash<unsigned char> : public __unary_function<unsigned char, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(unsigned char __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-#if _LIBCPP_HAS_CHAR8_T
-template <>
-struct hash<char8_t> : public __unary_function<char8_t, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(char8_t __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-#endif // _LIBCPP_HAS_CHAR8_T
-
-template <>
-struct hash<char16_t> : public __unary_function<char16_t, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(char16_t __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-template <>
-struct hash<char32_t> : public __unary_function<char32_t, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(char32_t __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-#if _LIBCPP_HAS_WIDE_CHARACTERS
-template <>
-struct hash<wchar_t> : public __unary_function<wchar_t, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(wchar_t __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-#endif // _LIBCPP_HAS_WIDE_CHARACTERS
-
-template <>
-struct hash<short> : public __unary_function<short, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(short __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-template <>
-struct hash<unsigned short> : public __unary_function<unsigned short, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(unsigned short __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
+template <class _Tp, class = void>
+struct __hash_impl {
+ __hash_impl() = delete;
+ __hash_impl(__hash_impl const&) = delete;
+ __hash_impl& operator=(__hash_impl const&) = delete;
};
-template <>
-struct hash<int> : public __unary_function<int, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(int __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-template <>
-struct hash<unsigned int> : public __unary_function<unsigned int, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(unsigned int __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-template <>
-struct hash<long> : public __unary_function<long, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(long __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
-};
-
-template <>
-struct hash<unsigned long> : public __unary_function<unsigned long, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(unsigned long __v) const _NOEXCEPT {
- static_assert(sizeof(size_t) >= sizeof(unsigned long),
- "This would be a terrible hash function on a platform where size_t is smaller than unsigned long");
- return static_cast<size_t>(__v);
+template <class _Tp>
+struct __hash_impl<_Tp, __enable_if_t<is_enum<_Tp>::value> > : __unary_function<_Tp, size_t> {
+ _LIBCPP_HIDE_FROM_ABI size_t operator()(_Tp __v) const _NOEXCEPT {
+ using type = __underlying_type_t<_Tp>;
+ return hash<type>()(static_cast<type>(__v));
}
};
-template <>
-struct hash<long long> : public __scalar_hash<long long> {};
-
-template <>
-struct hash<unsigned long long> : public __scalar_hash<unsigned long long> {};
-
-#if _LIBCPP_HAS_INT128
-
-template <>
-struct hash<__int128_t> : public __scalar_hash<__int128_t> {};
-
-template <>
-struct hash<__uint128_t> : public __scalar_hash<__uint128_t> {};
+template <class _Tp>
+struct __hash_impl<_Tp, __enable_if_t<is_integral<_Tp>::value && (sizeof(_Tp) <= sizeof(size_t))> >
+ : __unary_function<_Tp, size_t> {
+ _LIBCPP_HIDE_FROM_ABI size_t operator()(_Tp __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
+};
-#endif
+template <class _Tp>
+struct __hash_impl<_Tp, __enable_if_t<is_integral<_Tp>::value && (sizeof(_Tp) > sizeof(size_t))> >
+ : __scalar_hash<_Tp> {};
-template <>
-struct hash<float> : public __scalar_hash<float> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(float __v) const _NOEXCEPT {
+template <class _Tp>
+struct __hash_impl<_Tp, __enable_if_t<is_floating_point<_Tp>::value> > : __scalar_hash<_Tp> {
+ _LIBCPP_HIDE_FROM_ABI size_t operator()(_Tp __v) const _NOEXCEPT {
// -0.0 and 0.0 should return same hash
if (__v == 0.0f)
return 0;
- return __scalar_hash<float>::operator()(__v);
- }
-};
-
-template <>
-struct hash<double> : public __scalar_hash<double> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(double __v) const _NOEXCEPT {
- // -0.0 and 0.0 should return same hash
- if (__v == 0.0)
- return 0;
- return __scalar_hash<double>::operator()(__v);
+ return __scalar_hash<_Tp>::operator()(__v);
}
};
template <>
-struct hash<long double> : public __scalar_hash<long double> {
+struct __hash_impl<long double, void> : __scalar_hash<long double> {
_LIBCPP_HIDE_FROM_ABI size_t operator()(long double __v) const _NOEXCEPT {
// -0.0 and 0.0 should return same hash
if (__v == 0.0L)
@@ -501,22 +424,8 @@ struct hash<long double> : public __scalar_hash<long double> {
}
};
-template <class _Tp, bool = is_enum<_Tp>::value>
-struct __enum_hash : public __unary_function<_Tp, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(_Tp __v) const _NOEXCEPT {
- using type = __underlying_type_t<_Tp>;
- return hash<type>()(static_cast<type>(__v));
- }
-};
-template <class _Tp>
-struct __enum_hash<_Tp, false> {
- __enum_hash() = delete;
- __enum_hash(__enum_hash const&) = delete;
- __enum_hash& operator=(__enum_hash const&) = delete;
-};
-
template <class _Tp>
-struct hash : public __enum_hash<_Tp> {};
+struct hash : public __hash_impl<_Tp> {};
#if _LIBCPP_STD_VER >= 17
|
ldionne
approved these changes
Jun 6, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/15162 Here is the relevant piece of the build log for the reference |
rorth
pushed a commit
to rorth/llvm-project
that referenced
this pull request
Jun 11, 2025
Instead of providing full specializations of `hash` for every arithmetic type, this moves the implementation to a base class, which is specialized via `enable_if`s instead.
DhruvSrivastavaX
pushed a commit
to DhruvSrivastavaX/lldb-for-aix
that referenced
this pull request
Jun 12, 2025
Instead of providing full specializations of `hash` for every arithmetic type, this moves the implementation to a base class, which is specialized via `enable_if`s instead.
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
Instead of providing full specializations of `hash` for every arithmetic type, this moves the implementation to a base class, which is specialized via `enable_if`s instead.
|
Since this commit we've been seeing weird behavior when trying to build chromium -- sometimes the linker chooses a different version of std::hash that's defined elsewhere in our codebase. I'm not sure exactly what's going on, but I suspect it might be because the hash definitions are no longer explicit specializations, which affects template overload resolution. It's not clear if we should consider this a problem, but it certainly seems like a behavior change that probably wasn't intended, so it's worth discussing. Should we consider the explicit specializations to be better because they'll be chosen first during overload resolution? Should we just say that defining your own std::hash isn't supported? I've been trying to get a standalone repro but sadly it's going poorly. I'll post it if I manage to find one. |
|
This is a Chromium bug, as it defines a std::hash partial specialization that applies to int (amongst many other similarly-invalid-to-specialize types!), violating https://eel.is/c++draft/namespace.std#2 "the added declaration depends on at least one program-defined type". It's unfortunate that Clang doesn't have any diagnostics which would tell users that they've created an invalid specialization like that. I just hope the issue isn't more widespread than just in Chromium V8, because in both libstdc++ and previous versions of libc++, the UB would've gone undetected, since an explicit specialization of std::hash takes precedence over a user-defined partial specialization. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of providing full specializations of
hashfor every arithmetic type, this moves the implementation to a base class, which is specialized viaenable_ifs instead.