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

Make Choice and CtOption be Hash #138

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dvdplm
Copy link

@dvdplm dvdplm commented Aug 22, 2024

While rare, sometimes it is convenient to use CtOption in hashmaps/sets. This PR suggests adding Hash to CtOption and Choice to allow that use case, see e.g. crypto-bigint::Checked.

It also aligns with std::option::Option, which is also Hash.

@dvdplm
Copy link
Author

dvdplm commented Aug 22, 2024

One question I have is if making BlackBox be Hash as well is desirable?

@tarcieri
Copy link
Contributor

What's the use case for Choice being Hash? You want to use a Choice as a key for a HashMap?

I think all of these types, including and especially BlackBox, deliberately keep their internals opaque and only allow access using very specific and deliberate methods. BlackBox for example is only intended to be accessed via its get method and a Hash impl would bypass that.

@dvdplm
Copy link
Author

dvdplm commented Aug 25, 2024

Choice being Hash is just a consequence of making CtOption Hash.

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.

2 participants