constexpr for hash functions #686
Conversation
<type_traits> adds constexpr to the FNV-1a hash functions and helpers <xstring> adds constexpr to struct hash<basic_string_view<_Elem, _Traits>> Test and verification: Verification for int, double, std::string_view https://godbolt.org/z/jrqPg8 clang -std=c++2a -O3 clang version 6 or higher, evaluates the function at compile time MSVC /std:c++latest /O3 MSVC 19.24, up MSVC cl 19.26.28720.3 does not evaluate the function at compile time.
| @@ -2177,7 +2177,7 @@ _NODISCARD inline size_t _Fnv1a_append_bytes(size_t _Val, const unsigned char* c | |||
| } | |||
|
|
|||
| template <class _Ty> | |||
| _NODISCARD size_t _Fnv1a_append_range(const size_t _Val, const _Ty* const _First, | |||
| _NODISCARD constexpr size_t _Fnv1a_append_range(const size_t _Val, const _Ty* const _First, | |||
There was a problem hiding this comment.
This function cannot be constexpr because of reinterpret_casts.
| @@ -2186,21 +2186,21 @@ _NODISCARD size_t _Fnv1a_append_range(const size_t _Val, const _Ty* const _First | |||
| } | |||
|
|
|||
| template <class _Kty> | |||
| _NODISCARD size_t _Fnv1a_append_value( | |||
| _NODISCARD constexpr size_t _Fnv1a_append_value( | |||
There was a problem hiding this comment.
This function also contains reinterpret_cast.
| const size_t _Val, const _Kty& _Keyval) noexcept { // accumulate _Keyval into partial FNV-1a hash _Val | ||
| static_assert(is_trivial_v<_Kty>, "Only trivial types can be directly hashed."); | ||
| return _Fnv1a_append_bytes(_Val, &reinterpret_cast<const unsigned char&>(_Keyval), sizeof(_Kty)); | ||
| } | ||
|
|
||
| // FUNCTION TEMPLATE _Hash_representation | ||
| template <class _Kty> | ||
| _NODISCARD size_t _Hash_representation(const _Kty& _Keyval) noexcept { // bitwise hashes the representation of a key | ||
| _NODISCARD constexpr size_t _Hash_representation(const _Kty& _Keyval) noexcept { // bitwise hashes the representation of a key |
There was a problem hiding this comment.
This function calls _Fnv1a_append_value which contains reinterpret_cast.
| return _Fnv1a_append_value(_FNV_offset_basis, _Keyval); | ||
| } | ||
|
|
||
| // FUNCTION TEMPLATE _Hash_array_representation | ||
| template <class _Kty> | ||
| _NODISCARD size_t _Hash_array_representation( | ||
| _NODISCARD constexpr size_t _Hash_array_representation( |
There was a problem hiding this comment.
Another function with reinterpret_cast.
|
@AdamBucior Thank you for the review and you are correct. It clearly takes more than just adding constexpr to some functions. |
|
It can be done, I myself use constexpr FNV1-a in my custom hashing code, but the codegen is worse than the one done here with reinterpret_cast. |
|
I would not want this even if implementing it were 'free' because in vnext we want to replace this with something based on SipHash-2-4 where the hash values are not the same in successive runs of the program to make |
That's correct, as explained in https://github.com/microsoft/STL#non-goals . We welcome the addition of |
Description
constexpr for hash functions.
<type_traits> adds constexpr to the FNV-1a hash functions and helpers
<xstring> adds constexpr to struct hash<basic_string_view<_Elem, _Traits>>
Test and verification:
Verification for int, double, std::string_view
https://godbolt.org/z/jrqPg8
clang
-std=c++2a -O3
clang version 6 or higher, evaluates the function at compile time
MSVC
/std:c++latest /O3
MSVC 19.24, up MSVC 19.26.28720.3 does not evaluate the function at compile time.
Checklist
Before submitting a pull request, please ensure that:
Identifiers in product code changes are properly
_Uglyas perhttps://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
These changes introduce no known ABI breaks (adding members, renaming
members, adding virtual functions, changing whether a type is an aggregate
or trivially copyable, etc.).
Your changes are written from scratch using only this repository, the C++
Working Draft (including any cited standards), other WG21 papers (excluding
reference implementations outside of proposed standard wording), and LWG
issues as reference material. If your changes are derived from a project
that's already listed in NOTICE.txt, that's fine, but please mention it.
If your changes are derived from any other project, you must mention it
here, so we can determine whether the license is compatible and what else
needs to be done.