libbf icon indicating copy to clipboard operation
libbf copied to clipboard

Have default hash function returns a `digest`

Open kamikat opened this issue 8 years ago • 4 comments

A more consistent signature of a default hash function.

kamikat avatar Apr 10 '16 18:04 kamikat

The change to digest makes sense to me.

But you're introducing another subtle change silently: changing the digest type from size_t to uint64_t. So far, I'm going with the design of std::hash, which also returns size_t as a digest type, which is the most efficient unsigned type for a given architecture.

Frankly, the whole hashing design needs a major overhaul according to Howard's proposal.

mavam avatar Apr 10 '16 19:04 mavam

Oh... yes, the commit includes a change of digest's typedef to uint64_t to help clarify a storage property of digest. I'd better commit that separately.

size_t is semantically interpreted as a return type of sizeof, which should not be an appropriate type to describe a hash result. A digest type should be a better solution to that problem.

It comes to the definition of a digest. A type signature of uint64_t describes the storage property, which can help developer understanding digest type.

kamikat avatar Apr 11 '16 09:04 kamikat

size_t is semantically interpreted as a return type of sizeof, which should not be an appropriate type to describe a hash result. A digest type should be a better solution to that problem.

I'm not saying that size_t is the best choice, it's just what's used by std::hash to date, and that was the rationale to go with it intially. If you look at the design of the standard library, you will see that size_t is used for all sorts of "size" related computations, e.g., std::vector<T>::size.

I fully agree that size_t should not be the digest type, it just lends itself as natural fit in the very awkward design of the C++ hashing.

I will merge the PR if you remove the single uint64_t typedef. And thanks for contributing! :)

mavam avatar Apr 11 '16 14:04 mavam

Thank you for the explanation. It seems that should be a design issue about C++ hashing.

kamikat avatar Apr 11 '16 16:04 kamikat