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

Fix signed/unsigned conversion bug in siphash24 implementation #2979

Merged
merged 3 commits into from
Jan 22, 2019

Conversation

slfritchie
Copy link
Contributor

Fixes #2978.

Without this patch, the values calculated by the new _TestMapHashFunc unit test are both 3830601896319666767. The unit test's values were calculated & agreed upon by two reference implementations:

@slfritchie slfritchie added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Jan 16, 2019
@slfritchie slfritchie force-pushed the bug-2978 branch 2 times, most recently from f26ed27 to 2082485 Compare January 16, 2019 04:37
@mfelsche
Copy link
Contributor

Good catch!

Seeing some arm failures, could it be the value needs to be different on 32bit platforms?

@slfritchie
Copy link
Contributor Author

Yup. Rather than wait for another CI cycle, I'm trying to create an AWS ARM instance to find the right unit test values for the halfsiphash24 function used for PLATFORM_IS_ILP32. I'm hoping it'll be easy to build a 32-bit Pony compiler & runtime there.

@SeanTAllen SeanTAllen merged commit 2ea6f60 into master Jan 22, 2019
ponylang-main added a commit that referenced this pull request Jan 22, 2019
@SeanTAllen SeanTAllen deleted the bug-2978 branch January 22, 2019 17:12
@SeanTAllen
Copy link
Member

Thanks @slfritchie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants