Skip to content

Conversation

@creationix
Copy link
Contributor

@creationix creationix commented Aug 15, 2025

What does this PR do?

Hopefully fix #21879

How did you verify your code works?

Added a test with a seed larger than u32.

The test vector is from this tiny test I wrote to rule out upstream zig as the culprit:

const std = @import("std");
const testing = std.testing;
test "xxhash64 of short string with custom seed" {
    const input = "";
    const seed: u64 = 16269921104521594740;
    const hash = std.hash.XxHash64.hash(seed, input);
    const expected_hash: u64 = 3224619365169652240;
    try testing.expect(hash == expected_hash);
}

@RiskyMH
Copy link
Member

RiskyMH commented Aug 15, 2025

can you make a test so we can verify that it fixes it!

@creationix
Copy link
Contributor Author

ok, test added.

@creationix creationix changed the title Maybe fix large seeds on xxhash64 Fix xxhash64 to support seeds larger than u32. Aug 15, 2025
@RiskyMH
Copy link
Member

RiskyMH commented Aug 15, 2025

nice, in ~1hr all the tests should have ran

also if it does work, the other ones prob should be updated too if needed - and then maybe in a different test part section testing for longer size

Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will merge once macOS ci finishes (it's a bit behind right now)

@Jarred-Sumner Jarred-Sumner merged commit 53a3a67 into oven-sh:main Aug 16, 2025
59 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

xxhash64 is wrong for large seeds

3 participants