[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