[PATCH] D77038: Fix bug in BitVector and SmallBitVector DenseMap hashing
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 17 19:19:29 PDT 2020
jdoerfert added a comment.
In D77038#1990099 <https://reviews.llvm.org/D77038#1990099>, @bmoody wrote:
> I'm unsure about adding a test that BitVector and SmallBitVector hash exactly the same way, because in principle I think this is a consequence of implementation details and not a property that we actually want per se. In future the implementations might diverge such that they must hash differently (or at least, that it would be convenient for them to hash differently), and IMO that would be ok. But I'm willing to add the test if you disagree.
[Drive by and meant as a generic comment] If the test states this concern it is still valuable to have it. Once the implementations diverge the test fails, the concern is rediscovered, and the author can decide if the divergence was intentional/good or accidental/bad. In the former case the test might be removed but it at least protects against the accident case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77038/new/
https://reviews.llvm.org/D77038
More information about the llvm-commits
mailing list