Skip to content

Conversation

@Urgau
Copy link
Member

@Urgau Urgau commented Jun 20, 2024

This PR is the result of #3 (comment), where StableHasher::finalize was deemed confusing and useless.

As well as switching to an array representation ([u64; 2]) instead of a tuple, per #3 (comment).

r? @michaelwoerister or @WaffleLapkin

@Urgau Urgau force-pushed the remove-stablehasher-finalize branch from a19011a to eceea7a Compare June 20, 2024 16:53
@michaelwoerister
Copy link
Member

Looks good to me, although the switch from (u64, u64) to [u64; 2] seems a bit arbitrary. Is there a reason to prefer one over the other?

@WaffleLapkin
Copy link
Member

@michaelwoerister this is not the most important thing, but [u64; 2] allows strictly more than (u64, u64). generally when the collection is actually homogeneous it's better to use arrays. both halfs here represent the same thing, "part of the hash", there is no semantic difference between them.

so yeah. using a tuple is not wrong, but array is nicer.

@michaelwoerister michaelwoerister merged commit ca68834 into rust-lang:main Jun 24, 2024
@michaelwoerister
Copy link
Member

Thanks for the feedback, @WaffleLapkin! I have a slight bias towards not touching lines of code unless necessary but in this case the numbers of lines touched isn't that big.

If we do something like proposed here, then the array version is the only one that works.

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.

3 participants