Implements both the Hasher and Write trait for any HighwayHash #30
Conversation
I feel like every hasher struct should implement the standard Hasher trait, as it allows for more direct interoperability. The Write implemented on top is more a general convenience for simple hash function on objects implementing Read and Write: let mut reader = ...; // a file for example, anything Read-able let mut writer = ...; // any HighwayHash implementation std::io::copy(&mut reader, &mut writer)?; let hash = writer.finish(); There is surely a much better way to achieve both goals by refactoring even more heavily the HighwayHash trait.
I think this is a better design, it doesn't change the current API.
jRimbault
force-pushed
the
features/impl-hasher-and-write
branch
from
885c71e
to
e0fb433
Compare
|
(I'm moving to a new city so my responses have been slow and may continue to be slow for the next few days) This PR looks good. I'll take a closer look when I can, but the only thing I'd like to see are tests demonstrating the the traits in action (preferably a test per trait per implementation so that we can be extra paranoid against any possible regressions). |
|
I can't play with the code right now, so let me know if |
|
Will do. Take all the time you need.
|
|
Yes, no_std would be lovely! |
I don't understand why it failed on macos. I hope to gain a bit of insight by seeing the results of the tests in the other runners.
|
I'm a bit stumped on why the demo test fails on MacOS. I hoped it would fail on another runner with a more useful output, but no luck. |
|
I believe it is because the macOS CI environment doesn't support avx instructions. You may need to explicitly test for hardware support before invoking, ie: if is_x86_feature_detected!("avx2") {
// run avx test
}
if is_x86_feature_detected!("sse4.1") {
// run sse test
} |
|
Nice, looks like you've got it under control. Another task: copy the docs written in the readme to lib.rs so that docs in the readme and on docs.rs remain in sync. Then unless you have anything else, I think this would be ready to be merged. |
|
I think this PR is done. I have another branch with |
Nice! One thing to note is that SIMD instructions should still be available on core, afaik. What's not available is Thanks again! I'll cut a release once we have these features integrated. |
I'm opening this PR not really to merge that as it is (I don't think this is good), but to open the discussion about how to best implement
core::hash::Hasheron the different highway hasher structs.This is a design proof of concept, but it changes a lot of things, both in
HighwayHashtrait API, and at runtime theClone-ing of hasher structs. And I'm not too happy about these changes.Edit:
Using macros instead of trait objects allows the same thing but without changing any of the API. I think this is better. Though, if you want those features is still up to you of course.