clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
================
Comment at: lldb/source/Utility/ConstString.cpp:177-183
uint8_t hash(const llvm::StringRef &s) const {
- uint32_t h = llvm::djbHash(s);
+ return hash(StringPool::hash(s));
+ }
+
+ uint8_t hash(uint32_t h) const {
return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
}
----------------
Can we rename these functions to something other than "hash"? It is a bit
confusing to have a function called "hash" that returns a uint8_t when what
this is really doing is getting use the string pool index. Maybe
"GetPoolIndex"? I could see this function accidentally being used to hash a
string instead of using StringPool::hash()
================
Comment at: llvm/include/llvm/ADT/StringMap.h:69
+ /// Overload that explicitly takes precomputed hash(Key).
+ unsigned LookupBucketFor(StringRef Key, unsigned FullHashValue);
----------------
Use "uint32_t" instead of unsigned to clarify the exact width of the integer.
Or create a "HashType" typedef and then use that.
================
Comment at: llvm/include/llvm/ADT/StringMap.h:106
+ static unsigned hash(StringRef Key) { return llvm::djbHash(Key, 0); }
+
----------------
================
Comment at: llvm/lib/Support/StringMap.cpp:83
+unsigned StringMapImpl::LookupBucketFor(StringRef Name,
+ unsigned FullHashValue) {
+#ifdef EXPENSIVE_CHECKS
----------------
================
Comment at: llvm/lib/Support/StringMap.cpp:141
/// This does not modify the map.
-int StringMapImpl::FindKey(StringRef Key) const {
+int StringMapImpl::FindKey(StringRef Key, unsigned FullHashValue) const {
if (NumBuckets == 0)
----------------
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122974/new/
https://reviews.llvm.org/D122974
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
