Skip to content

feat: implement Hash trait for structs that has implemented PartialEq/Eq#143

Closed
leiysky wants to merge 1 commit intoBurntSushi:masterfrom
leiysky:dev
Closed

feat: implement Hash trait for structs that has implemented PartialEq/Eq#143
leiysky wants to merge 1 commit intoBurntSushi:masterfrom
leiysky:dev

Conversation

@leiysky
Copy link
Copy Markdown
Contributor

@leiysky leiysky commented Oct 19, 2024

Some of the core structs(e.g. Duration, Time) has implemented Hash trait while some of them hasn't.

This pull request implements Hash trait for the rest of the structs that should have implemented it(which are PartialEq and Eq).

@BurntSushi
Copy link
Copy Markdown
Owner

#32 probably means we're getting rid of Eq on Span, so I would hold off adding Hash to it.

@leiysky
Copy link
Copy Markdown
Contributor Author

leiysky commented Oct 21, 2024

#32 probably means we're getting rid of Eq on Span, so I would hold off adding Hash to it.

Sure, what about the rest part?

Timestamp can be converted to SignedDuration and Zoned can be converted to std::time::SystemTime, so I think it's reasonable to implement Hash for them.

BurntSushi pushed a commit that referenced this pull request Nov 30, 2024
This mostly implements `Hash` for all types that also implement
`PartialEq` and `Eq`, but notably omits one for `Span`. The reason being
that I'm currently planning on removing the `PartialEq` and `Eq`
implementations of `Span`, so I'd prefer not to add more API surface
area that relies on it.

Closes #143
@BurntSushi BurntSushi added the rollup A PR that closes multiple open issues or other PRs. label Nov 30, 2024
@tisonkun
Copy link
Copy Markdown
Contributor

@BurntSushi may we have a new release with this patch included? I have a downstream project need it :P

@leiysky leiysky deleted the dev branch December 23, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rollup A PR that closes multiple open issues or other PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants