make deterministic_hash host-architecture-independent #38357
Conversation
|
I'll take a more detailed look at this tomorrow, but in the meanwhile this might be interesting: Line 398 in aa7a2e9 |
|
Thanks for the fix, @arielb1! I have few comments though:
Ideally, we would just use the IchHasher, which is already built for this purpose (i.e. stable hashing). The module would have to be moved out of In the long run we might really want to have a general purpose |
arielb1
force-pushed
the
deterministic-hash
branch
from
0eb10a1
to
fe4d97a
Compare
| pub fn write_unsigned_leb128(out: &mut Vec<u8>, start_position: usize, mut value: u64) -> usize { | ||
| let mut position = start_position; | ||
| #[inline] | ||
| pub fn write_unsigned_leb128_to<W>(mut value: u64, mut write: W) -> usize |
There was a problem hiding this comment.
Can you add documentation on what the requirements for write are? Something like "write is a function that writes a single byte to the output destination at the given position. It is used to write the leb128 value to positions [0..N), where N is the number of bytes the value takes up".
| @@ -9,6 +9,7 @@ | |||
| // except according to those terms. | |||
There was a problem hiding this comment.
Can we not move Fingerprint to rustc_datastructures? It's an incremental compilation specific type.
|
|
||
| #[inline] | ||
| fn write_u8(&mut self, i: u8) { | ||
| self.state.write_u8(i); |
There was a problem hiding this comment.
bytes_hashed is not incremented here.
|
|
||
| #[inline] | ||
| fn write_i8(&mut self, i: i8) { | ||
| self.state.write_i8(i); |
| /// and `i64`. | ||
| /// | ||
| /// The same goes for endianess: We always convert multi-byte integers to little | ||
| /// endian before hashing. |
There was a problem hiding this comment.
This documentation was appropriate for ArchIndependentHasher but the implementation below uses the leb128 approach to solve the problem.
|
Nominating for beta to ensure we don't forget about this if it misses the beta branch. Note that this should only go into 1.15.0 beta, not the current (as of this writing) 1.14.0 beta. |
alexcrichton
added
the
beta-nominated
arielb1
force-pushed
the
deterministic-hash
branch
from
fe4d97a
to
b5735fb
Compare
The standard implementations of Hasher have architecture-dependent results when hashing integers. This causes problems when the hashes are stored within metadata - metadata written by one host architecture can't be read by another. To fix that, implement an architecture-independent StableHasher and use it in all places an architecture-independent hasher is needed. Fixes rust-lang#38177.
arielb1
force-pushed
the
deterministic-hash
branch
from
b5735fb
to
e1d4b8f
Compare
|
@bors r+ |
|
📌 Commit e1d4b8f has been approved by |
make deterministic_hash host-architecture-independent `DefPath::deterministic_hash` used to call `std::hash::Hash`, which depends on the current architecture in several ways, which would prevent metadata written on one host architecture from being successfully read on another one. Use a hasher we control instead. Fixes #38177. r? @michaelwoerister
|
This made it to beta, de-nominating |
alexcrichton
removed
the
beta-nominated
DefPath::deterministic_hashused to callstd::hash::Hash, which depends on the current architecture in several ways, which would prevent metadata written on one host architecture from being successfully read on another one.Use a hasher we control instead.
Fixes #38177.
r? @michaelwoerister