[libc++][NFC] Add a static assertion to document an assumption in std::hash #114440
Conversation
…::hash The implementation of std::hash for unsigned long makes the (correct) assumption that size_t is at least as large as unsigned long. If that were not the case on a platform, the implementation of std::hash for unsigned long would be absolutely terrible. Add a static assertion to document that assumption.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesThe implementation of std::hash for unsigned long makes the (correct) assumption that size_t is at least as large as unsigned long. If that were not the case on a platform, the implementation of std::hash for unsigned long would be absolutely terrible. Add a static assertion to document that assumption. Full diff: https://github.com/llvm/llvm-project/pull/114440.diff 1 Files Affected:
diff --git a/libcxx/include/__functional/hash.h b/libcxx/include/__functional/hash.h
index f7b89f759b5f5c..87009dfa62ef59 100644
--- a/libcxx/include/__functional/hash.h
+++ b/libcxx/include/__functional/hash.h
@@ -406,7 +406,11 @@ struct _LIBCPP_TEMPLATE_VIS hash<long> : public __unary_function<long, size_t> {
template <>
struct _LIBCPP_TEMPLATE_VIS hash<unsigned long> : public __unary_function<unsigned long, size_t> {
- _LIBCPP_HIDE_FROM_ABI size_t operator()(unsigned long __v) const _NOEXCEPT { return static_cast<size_t>(__v); }
+ _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 <>
|
…::hash (llvm#114440) The implementation of std::hash for unsigned long makes the (correct) assumption that size_t is at least as large as unsigned long. If that were not the case on a platform, the implementation of std::hash for unsigned long would be absolutely terrible. Add a static assertion to document that assumption.
…::hash (llvm#114440) The implementation of std::hash for unsigned long makes the (correct) assumption that size_t is at least as large as unsigned long. If that were not the case on a platform, the implementation of std::hash for unsigned long would be absolutely terrible. Add a static assertion to document that assumption.
|
I happen to have such a target (32-bit |
|
@s-barannikov What platform do you have? If possible it'd be great to get official support for that platform and implement a good hashing algorithm for that case. I don't think we'll break any 16 bit platforms, since we don't have a general hashing algorithm for them either. |
|
The platform has 32-bit bytes, it is not currently possible to upstream it. I understand that static_assert is better than untested code, so I don't insist on any changes upstream. I'm merely asking for advice on how I could fix it downstream. |
|
Yeah, I think you should be able to use |
The implementation of std::hash for unsigned long makes the (correct) assumption that size_t is at least as large as unsigned long. If that were not the case on a platform, the implementation of std::hash for unsigned long would be absolutely terrible. Add a static assertion to document that assumption.