rust-lang / rustc-stable-hash

A stable hashing algorithm used by rustc: cross-platform, deterministic, not secure
Apache License 2.0
5 stars 3 forks source link

Remove `StableHasher::finalize` to simplify the interface #4

Closed Urgau closed 3 months ago

Urgau commented 3 months ago

This PR is the result of https://github.com/rust-lang/rustc-stable-hash/pull/3#discussion_r1647474841, where StableHasher::finalize was deemed confusing and useless.

As well as switching to an array representation ([u64; 2]) instead of a tuple, per https://github.com/rust-lang/rustc-stable-hash/pull/3#discussion_r1647815972.

r? @michaelwoerister or @WaffleLapkin

michaelwoerister commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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.