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

Document guarantee that hash (Arg x _) == hash x #242

Open
josephcsible opened this issue Feb 2, 2022 · 2 comments · May be fixed by #243
Open

Document guarantee that hash (Arg x _) == hash x #242

josephcsible opened this issue Feb 2, 2022 · 2 comments · May be fixed by #243

Comments

@josephcsible
Copy link

In haskell-unordered-containers/unordered-containers#349, I'm proposing to add functions that convert between HashMaps with k as keys and ones with Arg k v as keys, without needing to rehash anything. For this to be safe, it needs to be the case that hash (Arg x _) == hash x. This is indeed the case, but the documentation doesn't currently guarantee this. It only lets us infer the weaker guarantee x1 == x2 -> hash (Arg x1 y1) == hash (Arg x2 y2), which isn't sufficient for this purpose. Can we add the stronger guarantee to the documentation?

@phadej
Copy link
Contributor

phadej commented Feb 2, 2022

Sure. That was a reason of hashable-1.3.0.0 major release, so we can as well document that behavior on the instance of Arg, for others to rely.

I don't see any reason to change the instance ever, and if there would be a reason, the change will be a breaking change, so we'd need to consider the motivation properly.

@josephcsible
Copy link
Author

That was a reason of hashable-1.3.0.0 major release

I thought the reason for that was just to make the weaker guarantee hold, and the fact that the stronger one holds too was just a lucky coincidence.

But in any case, this is good news, thanks. I'll write a PR for this tonight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants