Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve detection and enforcement of is_avalanching trait #92

Open
vmilea opened this issue Oct 1, 2023 · 3 comments
Open

Improve detection and enforcement of is_avalanching trait #92

vmilea opened this issue Oct 1, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@vmilea
Copy link

vmilea commented Oct 1, 2023

First of all, thanks for making this awesome library!

Is your feature request related to a problem? Please describe.

The mechanism for tagging high quality hashes could be improved a bit, so it's easier to integrate in existing code bases.

Describe the solution you'd like

  1. I prefer to specialize std::hash, but the primary template ankerl::unordered_dense::hash which falls back to std::hash doesn't currently propagate the is_avalanching trait. So it won't be detected unless hashes are moved to ankerl::unordered_dense namespace.
  2. In my project, all hashes are supposed to be high quality, but it's easy to forget to add the is_avalanching trait. It would be nice to have a require_avalanching policy on the container that can be asserted at compile time.
  3. An additional assume_avalanching policy sounds a little dangerous, but would avoid having to litter code with is_avalanching traits.

Describe alternatives you've considered

My approach for (2) is to wrap the default hash type:

template <typename Hash>
struct require_avalanching : Hash
{
    static_assert(requires { typename Hash::is_avalanching; });
};

// alias with default policy
template <class Key, class Hash = require_avalanching<ankerl::unordered_dense::hash<Key>>, ...>
using hash_set = ankerl::unordered_dense::set<Key, Hash, ...>;
@vmilea vmilea added the enhancement New feature or request label Oct 1, 2023
@martinus
Copy link
Owner

martinus commented Oct 3, 2023

Hi @vmilea, these are all very sensible points. I'll see that I can fix all of them.
About (2), I'm thinking whats the best way to implement this is. E.g. I can provide typedefs for the maps in a custom namespace, or globally enable the check with a macro.
I then also need a way to opt out, so a way to specify that a hash is not avalanching (maybe with is_not_avalanching or something like that). That also needs to work when the hash can't be modified.

@vmilea
Copy link
Author

vmilea commented Oct 3, 2023

I think an extra template parameter similar to Bucket would suffice. The user can add typedefs as needed.

A basic enum may work:

enum class hash_policy {
    assume_avalanching,
    require_avalanching,
    detect_avalanching,
};

But a struct HashAdapter would be more powerful. Then the user can create their own adapter, with the escape hatches they desire, even when a hash can't be modified.

@vmilea
Copy link
Author

vmilea commented Oct 3, 2023

To be clear: the HashAdapter wraps the hash, but is a distinct template parameter on the container. So if the user changes the Hash, the avalanching policy continues to be enforced. Unlike the "alternatives you've considered" code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants