<div dir="ltr">Interesting, didn't see this in any of my local builds. I should be able to commit a fix shortly.</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Mar 16, 2018 at 1:04 AM Mikael Holmén <<a href="mailto:mikael.holmen@ericsson.com">mikael.holmen@ericsson.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Zachary,<br>
<br>
Not sure if it's this commit or any of the others changing in<br>
HashTable.h that causes it but at least clang 3.6 and gcc 5.4.0<br>
complains when building HashTableTest.cpp on trunk now.<br>
<br>
With clang:<br>
<br>
/repo/app/clang/3.6/bin/clang++ -march=corei7 -DGTEST_HAS_RTTI=0<br>
-DGTEST_HAS_TR1_TUPLE=0 -DGTEST_LANG_CXX11=1 -D_DEBUG -D_GNU_SOURCE<br>
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS<br>
-Iunittests/DebugInfo/PDB -I../unittests/DebugInfo/PDB<br>
-I/usr/include/libxml2 -Iinclude -I../include<br>
-I../utils/unittest/googletest/include<br>
-I../utils/unittest/googlemock/include<br>
-I/repo/app/valgrind/3.11.0/include -fPIC -fvisibility-inlines-hidden<br>
-Werror -Werror=date-time -std=c++11 -Wall -W -Wno-unused-parameter<br>
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic<br>
-Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor<br>
-Wdelete-non-virtual-dtor -Wstring-conversion -fcolor-diagnostics<br>
-ffunction-sections -fdata-sections -O3 -UNDEBUG<br>
-Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments<br>
-fno-exceptions -fno-rtti -MMD -MT<br>
unittests/DebugInfo/PDB/CMakeFiles/DebugInfoPDBTests.dir/HashTableTest.cpp.o<br>
-MF<br>
unittests/DebugInfo/PDB/CMakeFiles/DebugInfoPDBTests.dir/HashTableTest.cpp.o.d<br>
-o<br>
unittests/DebugInfo/PDB/CMakeFiles/DebugInfoPDBTests.dir/HashTableTest.cpp.o<br>
-c ../unittests/DebugInfo/PDB/HashTableTest.cpp<br>
In file included from ../unittests/DebugInfo/PDB/HashTableTest.cpp:10:<br>
../include/llvm/DebugInfo/PDB/Native/HashTable.h:282:73: error:<br>
comparison of integers of different signs: 'uint32_t' (aka 'unsigned<br>
int') and 'const int' [-Werror,-Wsign-compare]<br>
assert(Traits.storageKeyToLookupKey(Buckets[Entry.index()].first)<br>
== K);<br>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~<br>
^ ~<br>
/usr/include/assert.h:89:5: note: expanded from macro 'assert'<br>
((expr) \<br>
^<br>
../include/llvm/DebugInfo/PDB/Native/HashTable.h:256:12: note: in<br>
instantiation of function template specialization<br>
'llvm::pdb::HashTable<unsigned int, llvm::pdb::PdbHashTraits<uint32_t><br>
>::set_as_internal<int>' requested here<br>
return set_as_internal(K, std::move(V), None);<br>
^<br>
../unittests/DebugInfo/PDB/HashTableTest.cpp:44:9: note: in<br>
instantiation of function template specialization<br>
'llvm::pdb::HashTable<unsigned int, llvm::pdb::PdbHashTraits<uint32_t><br>
>::set_as<int>' requested here<br>
Table.set_as(3, 7);<br>
^<br>
In file included from ../unittests/DebugInfo/PDB/HashTableTest.cpp:10:<br>
../include/llvm/DebugInfo/PDB/Native/HashTable.h:230:60: error:<br>
comparison of integers of different signs: 'uint32_t' (aka 'unsigned<br>
int') and 'const int' [-Werror,-Wsign-compare]<br>
if (Traits.storageKeyToLookupKey(Buckets[I].first) == K)<br>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~<br>
../unittests/DebugInfo/PDB/HashTableTest.cpp:46:32: note: in<br>
instantiation of function template specialization<br>
'llvm::pdb::HashTable<unsigned int, llvm::pdb::PdbHashTraits<uint32_t><br>
>::find_as<int>' requested here<br>
ASSERT_NE(Table.end(), Table.find_as(3));<br>
^<br>
../utils/unittest/googletest/include/gtest/gtest.h:1960:54: note:<br>
expanded from macro 'ASSERT_NE'<br>
# define ASSERT_NE(val1, val2) GTEST_ASSERT_NE(val1, val2)<br>
^<br>
../utils/unittest/googletest/include/gtest/gtest.h:1942:63: note:<br>
expanded from macro 'GTEST_ASSERT_NE'<br>
ASSERT_PRED_FORMAT2(::testing::internal::CmpHelperNE, val1, val2)<br>
^<br>
../utils/unittest/googletest/include/gtest/gtest_pred_impl.h:166:40:<br>
note: expanded from macro 'ASSERT_PRED_FORMAT2'<br>
GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_FATAL_FAILURE_)<br>
^<br>
../utils/unittest/googletest/include/gtest/gtest_pred_impl.h:147:43:<br>
note: expanded from macro 'GTEST_PRED_FORMAT2_'<br>
GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2), \<br>
^<br>
../utils/unittest/googletest/include/gtest/gtest_pred_impl.h:77:52:<br>
note: expanded from macro 'GTEST_ASSERT_'<br>
if (const ::testing::AssertionResult gtest_ar = (expression)) \<br>
^<br>
2 errors generated.<br>
<br>
<br>
and with gcc:<br>
<br>
[230/411] Building CXX object<br>
unittests/DebugInfo/PDB/CMakeFiles/DebugInfoPDBTests.dir/HashTableTest.cpp.o<br>
In file included from ../unittests/DebugInfo/PDB/HashTableTest.cpp:10:0:<br>
../include/llvm/DebugInfo/PDB/Native/HashTable.h: In instantiation of<br>
'llvm::pdb::HashTable<ValueT, TraitsT>::iterator<br>
llvm::pdb::HashTable<ValueT, TraitsT>::find_as(const Key&) const [with<br>
Key = int; ValueT = unsigned int; TraitsT =<br>
llvm::pdb::PdbHashTraits<unsigned int>; llvm::pdb::HashTable<ValueT,<br>
TraitsT>::iterator = llvm::pdb::HashTableIterator<unsigned int,<br>
llvm::pdb::PdbHashTraits<unsigned int> >]':<br>
../unittests/DebugInfo/PDB/HashTableTest.cpp:46:3: required from here<br>
../include/llvm/DebugInfo/PDB/Native/HashTable.h:230:60: warning:<br>
comparison between signed and unsigned integer expressions [-Wsign-compare]<br>
if (Traits.storageKeyToLookupKey(Buckets[I].first) == K)<br>
^<br>
In file included from<br>
/proj/bbi_twh/wh_bbi/x86_64-Linux2/bbigcc/<a href="http://1.5.4.0/crosscompiler/include/c++/5.4.0/cassert:43:0" rel="noreferrer" target="_blank">1.5.4.0/crosscompiler/include/c++/5.4.0/cassert:43:0</a>,<br>
from ../include/llvm/Support/BinaryStreamArray.h:17,<br>
from ../include/llvm/Support/BinaryStreamReader.h:15,<br>
from ../include/llvm/DebugInfo/PDB/Native/HashTable.h:16,<br>
from ../unittests/DebugInfo/PDB/HashTableTest.cpp:10:<br>
../include/llvm/DebugInfo/PDB/Native/HashTable.h: In instantiation of<br>
'bool llvm::pdb::HashTable<ValueT, TraitsT>::set_as_internal(const Key&,<br>
ValueT, llvm::Optional<unsigned int>) [with Key = int; ValueT = unsigned<br>
int; TraitsT = llvm::pdb::PdbHashTraits<unsigned int>]':<br>
../include/llvm/DebugInfo/PDB/Native/HashTable.h:256:27: required from<br>
'bool llvm::pdb::HashTable<ValueT, TraitsT>::set_as(const Key&, ValueT)<br>
[with Key = int; ValueT = unsigned int; TraitsT =<br>
llvm::pdb::PdbHashTraits<unsigned int>]'<br>
../unittests/DebugInfo/PDB/HashTableTest.cpp:44:20: required from here<br>
../include/llvm/DebugInfo/PDB/Native/HashTable.h:282:73: warning:<br>
comparison between signed and unsigned integer expressions [-Wsign-compare]<br>
<br>
assert(Traits.storageKeyToLookupKey(Buckets[Entry.index()].first) == K);<br>
<br>
<br>
Regards,<br>
Mikael<br>
<br>
On 03/15/2018 06:38 PM, Zachary Turner via llvm-commits wrote:<br>
> Author: zturner<br>
> Date: Thu Mar 15 10:38:26 2018<br>
> New Revision: 327647<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=327647&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=327647&view=rev</a><br>
> Log:<br>
> Refactor the PDB HashTable class.<br>
><br>
> It previously only worked when the key and value types were<br>
> both 4 byte integers. We now have a use case for a non trivial<br>
> value type, so we need to extend it to support arbitrary value<br>
> types, which means templatizing it.<br>
><br>
> Modified:<br>
> llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h<br>
> llvm/trunk/include/llvm/DebugInfo/PDB/Native/NamedStreamMap.h<br>
> llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h<br>
> llvm/trunk/lib/DebugInfo/PDB/Native/HashTable.cpp<br>
> llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp<br>
> llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp<br>
> llvm/trunk/tools/llvm-pdbutil/Analyze.cpp<br>
> llvm/trunk/unittests/DebugInfo/PDB/HashTableTest.cpp<br>
><br>
> Modified: llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h?rev=327647&r1=327646&r2=327647&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h?rev=327647&r1=327646&r2=327647&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h (original)<br>
> +++ llvm/trunk/include/llvm/DebugInfo/PDB/Native/HashTable.h Thu Mar 15 10:38:26 2018<br>
> @@ -12,6 +12,7 @@<br>
><br>
> #include "llvm/ADT/SparseBitVector.h"<br>
> #include "llvm/ADT/iterator.h"<br>
> +#include "llvm/DebugInfo/PDB/Native/RawError.h"<br>
> #include "llvm/Support/Endian.h"<br>
> #include "llvm/Support/Error.h"<br>
> #include <cstdint><br>
> @@ -26,73 +27,196 @@ class BinaryStreamWriter;<br>
><br>
> namespace pdb {<br>
><br>
> -class HashTable;<br>
> +template <typename ValueT, typename TraitsT> class HashTable;<br>
><br>
> +template <typename ValueT, typename TraitsT><br>
> class HashTableIterator<br>
> - : public iterator_facade_base<HashTableIterator, std::forward_iterator_tag,<br>
> - std::pair<uint32_t, uint32_t>> {<br>
> - friend HashTable;<br>
> -<br>
> - HashTableIterator(const HashTable &Map, uint32_t Index, bool IsEnd);<br>
> + : public iterator_facade_base<HashTableIterator<ValueT, TraitsT>,<br>
> + std::forward_iterator_tag,<br>
> + std::pair<uint32_t, ValueT>> {<br>
> + friend HashTable<ValueT, TraitsT>;<br>
> +<br>
> + HashTableIterator(const HashTable<ValueT, TraitsT> &Map, uint32_t Index,<br>
> + bool IsEnd)<br>
> + : Map(&Map), Index(Index), IsEnd(IsEnd) {}<br>
><br>
> public:<br>
> - HashTableIterator(const HashTable &Map);<br>
> + HashTableIterator(const HashTable<ValueT, TraitsT> &Map) : Map(&Map) {<br>
> + int I = Map.Present.find_first();<br>
> + if (I == -1) {<br>
> + Index = 0;<br>
> + IsEnd = true;<br>
> + } else {<br>
> + Index = static_cast<uint32_t>(I);<br>
> + IsEnd = false;<br>
> + }<br>
> + }<br>
><br>
> - HashTableIterator &operator=(const HashTableIterator &R);<br>
> - bool operator==(const HashTableIterator &R) const;<br>
> - const std::pair<uint32_t, uint32_t> &operator*() const;<br>
> - HashTableIterator &operator++();<br>
> + HashTableIterator &operator=(const HashTableIterator &R) {<br>
> + Map = R.Map;<br>
> + return *this;<br>
> + }<br>
> + bool operator==(const HashTableIterator &R) const {<br>
> + if (IsEnd && R.IsEnd)<br>
> + return true;<br>
> + if (IsEnd != R.IsEnd)<br>
> + return false;<br>
> +<br>
> + return (Map == R.Map) && (Index == R.Index);<br>
> + }<br>
> + const std::pair<uint32_t, ValueT> &operator*() const {<br>
> + assert(Map->Present.test(Index));<br>
> + return Map->Buckets[Index];<br>
> + }<br>
> + HashTableIterator &operator++() {<br>
> + while (Index < Map->Buckets.size()) {<br>
> + ++Index;<br>
> + if (Map->Present.test(Index))<br>
> + return *this;<br>
> + }<br>
> +<br>
> + IsEnd = true;<br>
> + return *this;<br>
> + }<br>
><br>
> private:<br>
> bool isEnd() const { return IsEnd; }<br>
> uint32_t index() const { return Index; }<br>
><br>
> - const HashTable *Map;<br>
> + const HashTable<ValueT, TraitsT> *Map;<br>
> uint32_t Index;<br>
> bool IsEnd;<br>
> };<br>
><br>
> +template <typename T> struct PdbHashTraits {};<br>
> +<br>
> +template <> struct PdbHashTraits<uint32_t> {<br>
> + uint32_t hashLookupKey(uint32_t N) const { return N; }<br>
> + uint32_t storageKeyToLookupKey(uint32_t N) const { return N; }<br>
> + uint32_t lookupKeyToStorageKey(uint32_t N) { return N; }<br>
> +};<br>
> +<br>
> +template <typename ValueT, typename TraitsT = PdbHashTraits<ValueT>><br>
> class HashTable {<br>
> - friend class HashTableIterator;<br>
> + using iterator = HashTableIterator<ValueT, TraitsT>;<br>
> + friend iterator;<br>
><br>
> struct Header {<br>
> support::ulittle32_t Size;<br>
> support::ulittle32_t Capacity;<br>
> };<br>
><br>
> - using BucketList = std::vector<std::pair<uint32_t, uint32_t>>;<br>
> + using BucketList = std::vector<std::pair<uint32_t, ValueT>>;<br>
><br>
> public:<br>
> - HashTable();<br>
> - explicit HashTable(uint32_t Capacity);<br>
> + HashTable() { Buckets.resize(8); }<br>
> +<br>
> + explicit HashTable(TraitsT Traits) : HashTable(8, std::move(Traits)) {}<br>
> + HashTable(uint32_t Capacity, TraitsT Traits) : Traits(Traits) {<br>
> + Buckets.resize(Capacity);<br>
> + }<br>
> +<br>
> + Error load(BinaryStreamReader &Stream) {<br>
> + const Header *H;<br>
> + if (auto EC = Stream.readObject(H))<br>
> + return EC;<br>
> + if (H->Capacity == 0)<br>
> + return make_error<RawError>(raw_error_code::corrupt_file,<br>
> + "Invalid Hash Table Capacity");<br>
> + if (H->Size > maxLoad(H->Capacity))<br>
> + return make_error<RawError>(raw_error_code::corrupt_file,<br>
> + "Invalid Hash Table Size");<br>
> +<br>
> + Buckets.resize(H->Capacity);<br>
> +<br>
> + if (auto EC = readSparseBitVector(Stream, Present))<br>
> + return EC;<br>
> + if (Present.count() != H->Size)<br>
> + return make_error<RawError>(raw_error_code::corrupt_file,<br>
> + "Present bit vector does not match size!");<br>
> +<br>
> + if (auto EC = readSparseBitVector(Stream, Deleted))<br>
> + return EC;<br>
> + if (Present.intersects(Deleted))<br>
> + return make_error<RawError>(raw_error_code::corrupt_file,<br>
> + "Present bit vector interesects deleted!");<br>
> +<br>
> + for (uint32_t P : Present) {<br>
> + if (auto EC = Stream.readInteger(Buckets[P].first))<br>
> + return EC;<br>
> + const ValueT *Value;<br>
> + if (auto EC = Stream.readObject(Value))<br>
> + return EC;<br>
> + Buckets[P].second = *Value;<br>
> + }<br>
> +<br>
> + return Error::success();<br>
> + }<br>
><br>
> - Error load(BinaryStreamReader &Stream);<br>
> + uint32_t calculateSerializedLength() const {<br>
> + uint32_t Size = sizeof(Header);<br>
><br>
> - uint32_t calculateSerializedLength() const;<br>
> - Error commit(BinaryStreamWriter &Writer) const;<br>
> + int NumBitsP = Present.find_last() + 1;<br>
> + int NumBitsD = Deleted.find_last() + 1;<br>
><br>
> - void clear();<br>
> + // Present bit set number of words, followed by that many actual words.<br>
> + Size += sizeof(uint32_t);<br>
> + Size += alignTo(NumBitsP, sizeof(uint32_t));<br>
><br>
> - uint32_t capacity() const;<br>
> - uint32_t size() const;<br>
> + // Deleted bit set number of words, followed by that many actual words.<br>
> + Size += sizeof(uint32_t);<br>
> + Size += alignTo(NumBitsD, sizeof(uint32_t));<br>
><br>
> - HashTableIterator begin() const;<br>
> - HashTableIterator end() const;<br>
> + // One (Key, ValueT) pair for each entry Present.<br>
> + Size += (sizeof(uint32_t) + sizeof(ValueT)) * size();<br>
> +<br>
> + return Size;<br>
> + }<br>
><br>
> - /// Find the entry with the specified key value.<br>
> - HashTableIterator find(uint32_t K) const;<br>
> + Error commit(BinaryStreamWriter &Writer) const {<br>
> + Header H;<br>
> + H.Size = size();<br>
> + H.Capacity = capacity();<br>
> + if (auto EC = Writer.writeObject(H))<br>
> + return EC;<br>
> +<br>
> + if (auto EC = writeSparseBitVector(Writer, Present))<br>
> + return EC;<br>
> +<br>
> + if (auto EC = writeSparseBitVector(Writer, Deleted))<br>
> + return EC;<br>
> +<br>
> + for (const auto &Entry : *this) {<br>
> + if (auto EC = Writer.writeInteger(Entry.first))<br>
> + return EC;<br>
> + if (auto EC = Writer.writeObject(Entry.second))<br>
> + return EC;<br>
> + }<br>
> + return Error::success();<br>
> + }<br>
> +<br>
> + void clear() {<br>
> + Buckets.resize(8);<br>
> + Present.clear();<br>
> + Deleted.clear();<br>
> + }<br>
> +<br>
> + uint32_t capacity() const { return Buckets.size(); }<br>
> + uint32_t size() const { return Present.count(); }<br>
> +<br>
> + iterator begin() const { return iterator(*this); }<br>
> + iterator end() const { return iterator(*this, 0, true); }<br>
><br>
> /// Find the entry whose key has the specified hash value, using the specified<br>
> /// traits defining hash function and equality.<br>
> - template <typename Traits, typename Key, typename Context><br>
> - HashTableIterator find_as(const Key &K, const Context &Ctx) const {<br>
> - uint32_t H = Traits::hash(K, Ctx) % capacity();<br>
> + template <typename Key> iterator find_as(const Key &K) const {<br>
> + uint32_t H = Traits.hashLookupKey(K) % capacity();<br>
> uint32_t I = H;<br>
> Optional<uint32_t> FirstUnused;<br>
> do {<br>
> if (isPresent(I)) {<br>
> - if (Traits::realKey(Buckets[I].first, Ctx) == K)<br>
> - return HashTableIterator(*this, I, false);<br>
> + if (Traits.storageKeyToLookupKey(Buckets[I].first) == K)<br>
> + return iterator(*this, I, false);<br>
> } else {<br>
> if (!FirstUnused)<br>
> FirstUnused = I;<br>
> @@ -111,40 +235,26 @@ public:<br>
> // table were Present. But this would violate the load factor constraints<br>
> // that we impose, so it should never happen.<br>
> assert(FirstUnused);<br>
> - return HashTableIterator(*this, *FirstUnused, true);<br>
> + return iterator(*this, *FirstUnused, true);<br>
> }<br>
><br>
> - /// Set the entry with the specified key to the specified value.<br>
> - void set(uint32_t K, uint32_t V);<br>
> -<br>
> /// Set the entry using a key type that the specified Traits can convert<br>
> /// from a real key to an internal key.<br>
> - template <typename Traits, typename Key, typename Context><br>
> - bool set_as(const Key &K, uint32_t V, Context &Ctx) {<br>
> - return set_as_internal<Traits, Key, Context>(K, V, None, Ctx);<br>
> + template <typename Key> bool set_as(const Key &K, ValueT V) {<br>
> + return set_as_internal(K, std::move(V), None);<br>
> }<br>
><br>
> - void remove(uint32_t K);<br>
> -<br>
> - template <typename Traits, typename Key, typename Context><br>
> - void remove_as(const Key &K, Context &Ctx) {<br>
> - auto Iter = find_as<Traits, Key, Context>(K, Ctx);<br>
> - // It wasn't here to begin with, just exit.<br>
> - if (Iter == end())<br>
> - return;<br>
> -<br>
> - assert(Present.test(Iter.index()));<br>
> - assert(!Deleted.test(Iter.index()));<br>
> - Deleted.set(Iter.index());<br>
> - Present.reset(Iter.index());<br>
> + template <typename Key> ValueT get(const Key &K) const {<br>
> + auto Iter = find_as(K);<br>
> + assert(Iter != end());<br>
> + return (*Iter).second;<br>
> }<br>
><br>
> - uint32_t get(uint32_t K);<br>
> -<br>
> protected:<br>
> bool isPresent(uint32_t K) const { return Present.test(K); }<br>
> bool isDeleted(uint32_t K) const { return Deleted.test(K); }<br>
><br>
> + TraitsT Traits;<br>
> BucketList Buckets;<br>
> mutable SparseBitVector<> Present;<br>
> mutable SparseBitVector<> Deleted;<br>
> @@ -152,13 +262,12 @@ protected:<br>
> private:<br>
> /// Set the entry using a key type that the specified Traits can convert<br>
> /// from a real key to an internal key.<br>
> - template <typename Traits, typename Key, typename Context><br>
> - bool set_as_internal(const Key &K, uint32_t V, Optional<uint32_t> InternalKey,<br>
> - Context &Ctx) {<br>
> - auto Entry = find_as<Traits, Key, Context>(K, Ctx);<br>
> + template <typename Key><br>
> + bool set_as_internal(const Key &K, ValueT V, Optional<uint32_t> InternalKey) {<br>
> + auto Entry = find_as(K);<br>
> if (Entry != end()) {<br>
> assert(isPresent(Entry.index()));<br>
> - assert(Traits::realKey(Buckets[Entry.index()].first, Ctx) == K);<br>
> + assert(Traits.storageKeyToLookupKey(Buckets[Entry.index()].first) == K);<br>
> // We're updating, no need to do anything special.<br>
> Buckets[Entry.index()].second = V;<br>
> return false;<br>
> @@ -167,21 +276,20 @@ private:<br>
> auto &B = Buckets[Entry.index()];<br>
> assert(!isPresent(Entry.index()));<br>
> assert(Entry.isEnd());<br>
> - B.first = InternalKey ? *InternalKey : Traits::lowerKey(K, Ctx);<br>
> + B.first = InternalKey ? *InternalKey : Traits.lookupKeyToStorageKey(K);<br>
> B.second = V;<br>
> Present.set(Entry.index());<br>
> Deleted.reset(Entry.index());<br>
><br>
> - grow<Traits, Key, Context>(Ctx);<br>
> + grow();<br>
><br>
> - assert((find_as<Traits, Key, Context>(K, Ctx)) != end());<br>
> + assert((find_as(K)) != end());<br>
> return true;<br>
> }<br>
><br>
> - static uint32_t maxLoad(uint32_t capacity);<br>
> + static uint32_t maxLoad(uint32_t capacity) { return capacity * 2 / 3 + 1; }<br>
><br>
> - template <typename Traits, typename Key, typename Context><br>
> - void grow(Context &Ctx) {<br>
> + void grow() {<br>
> uint32_t S = size();<br>
> if (S < maxLoad(capacity()))<br>
> return;<br>
> @@ -193,11 +301,10 @@ private:<br>
> // Growing requires rebuilding the table and re-hashing every item. Make a<br>
> // copy with a larger capacity, insert everything into the copy, then swap<br>
> // it in.<br>
> - HashTable NewMap(NewCapacity);<br>
> + HashTable NewMap(NewCapacity, Traits);<br>
> for (auto I : Present) {<br>
> - auto RealKey = Traits::realKey(Buckets[I].first, Ctx);<br>
> - NewMap.set_as_internal<Traits, Key, Context>(RealKey, Buckets[I].second,<br>
> - Buckets[I].first, Ctx);<br>
> + auto LookupKey = Traits.storageKeyToLookupKey(Buckets[I].first);<br>
> + NewMap.set_as_internal(LookupKey, Buckets[I].second, Buckets[I].first);<br>
> }<br>
><br>
> Buckets.swap(NewMap.Buckets);<br>
> @@ -206,13 +313,11 @@ private:<br>
> assert(capacity() == NewCapacity);<br>
> assert(size() == S);<br>
> }<br>
> -<br>
> - static Error readSparseBitVector(BinaryStreamReader &Stream,<br>
> - SparseBitVector<> &V);<br>
> - static Error writeSparseBitVector(BinaryStreamWriter &Writer,<br>
> - SparseBitVector<> &Vec);<br>
> };<br>
><br>
> +Error readSparseBitVector(BinaryStreamReader &Stream, SparseBitVector<> &V);<br>
> +Error writeSparseBitVector(BinaryStreamWriter &Writer, SparseBitVector<> &Vec);<br>
> +<br>
> } // end namespace pdb<br>
><br>
> } // end namespace llvm<br>
><br>
> Modified: llvm/trunk/include/llvm/DebugInfo/PDB/Native/NamedStreamMap.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/Native/NamedStreamMap.h?rev=327647&r1=327646&r2=327647&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/Native/NamedStreamMap.h?rev=327647&r1=327646&r2=327647&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/DebugInfo/PDB/Native/NamedStreamMap.h (original)<br>
> +++ llvm/trunk/include/llvm/DebugInfo/PDB/Native/NamedStreamMap.h Thu Mar 15 10:38:26 2018<br>
> @@ -25,6 +25,17 @@ class BinaryStreamWriter;<br>
><br>
> namespace pdb {<br>
><br>
> +class NamedStreamMap;<br>
> +<br>
> +struct NamedStreamMapTraits {<br>
> + NamedStreamMap *NS;<br>
> +<br>
> + explicit NamedStreamMapTraits(NamedStreamMap &NS);<br>
> + uint16_t hashLookupKey(StringRef S) const;<br>
> + StringRef storageKeyToLookupKey(uint32_t Offset) const;<br>
> + uint32_t lookupKeyToStorageKey(StringRef S);<br>
> +};<br>
> +<br>
> class NamedStreamMap {<br>
> friend class NamedStreamMapBuilder;<br>
><br>
> @@ -46,9 +57,10 @@ public:<br>
> StringMap<uint32_t> entries() const;<br>
><br>
> private:<br>
> + NamedStreamMapTraits HashTraits;<br>
> /// Closed hash table from Offset -> StreamNumber, where Offset is the offset<br>
> /// of the stream name in NamesBuffer.<br>
> - HashTable OffsetIndexMap;<br>
> + HashTable<support::ulittle32_t, NamedStreamMapTraits> OffsetIndexMap;<br>
><br>
> /// Buffer of string data.<br>
> std::vector<char> NamesBuffer;<br>
><br>
> Modified: llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h?rev=327647&r1=327646&r2=327647&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h?rev=327647&r1=327646&r2=327647&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h (original)<br>
> +++ llvm/trunk/include/llvm/DebugInfo/PDB/Native/TpiStream.h Thu Mar 15 10:38:26 2018<br>
> @@ -51,7 +51,7 @@ public:<br>
> uint32_t getNumHashBuckets() const;<br>
> FixedStreamArray<support::ulittle32_t> getHashValues() const;<br>
> FixedStreamArray<codeview::TypeIndexOffset> getTypeIndexOffsets() const;<br>
> - HashTable &getHashAdjusters();<br>
> + HashTable<support::ulittle32_t> &getHashAdjusters();<br>
><br>
> codeview::CVTypeRange types(bool *HadError) const;<br>
> const codeview::CVTypeArray &typeArray() const { return TypeRecords; }<br>
> @@ -75,7 +75,7 @@ private:<br>
> std::unique_ptr<BinaryStream> HashStream;<br>
> FixedStreamArray<support::ulittle32_t> HashValues;<br>
> FixedStreamArray<codeview::TypeIndexOffset> TypeIndexOffsets;<br>
> - HashTable HashAdjusters;<br>
> + HashTable<support::ulittle32_t> HashAdjusters;<br>
><br>
> const TpiStreamHeader *Header;<br>
> };<br>
><br>
> Modified: llvm/trunk/lib/DebugInfo/PDB/Native/HashTable.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/HashTable.cpp?rev=327647&r1=327646&r2=327647&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/HashTable.cpp?rev=327647&r1=327646&r2=327647&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/DebugInfo/PDB/Native/HashTable.cpp (original)<br>
> +++ llvm/trunk/lib/DebugInfo/PDB/Native/HashTable.cpp Thu Mar 15 10:38:26 2018<br>
> @@ -22,130 +22,7 @@<br>
> using namespace llvm;<br>
> using namespace llvm::pdb;<br>
><br>
> -namespace {<br>
> -struct IdentityTraits {<br>
> - static uint32_t hash(uint32_t K, const HashTable &Ctx) { return K; }<br>
> - static uint32_t realKey(uint32_t K, const HashTable &Ctx) { return K; }<br>
> - static uint32_t lowerKey(uint32_t K, const HashTable &Ctx) { return K; }<br>
> -};<br>
> -} // namespace<br>
> -<br>
> -HashTable::HashTable() : HashTable(8) {}<br>
> -<br>
> -HashTable::HashTable(uint32_t Capacity) { Buckets.resize(Capacity); }<br>
> -<br>
> -Error HashTable::load(BinaryStreamReader &Stream) {<br>
> - const Header *H;<br>
> - if (auto EC = Stream.readObject(H))<br>
> - return EC;<br>
> - if (H->Capacity == 0)<br>
> - return make_error<RawError>(raw_error_code::corrupt_file,<br>
> - "Invalid Hash Table Capacity");<br>
> - if (H->Size > maxLoad(H->Capacity))<br>
> - return make_error<RawError>(raw_error_code::corrupt_file,<br>
> - "Invalid Hash Table Size");<br>
> -<br>
> - Buckets.resize(H->Capacity);<br>
> -<br>
> - if (auto EC = readSparseBitVector(Stream, Present))<br>
> - return EC;<br>
> - if (Present.count() != H->Size)<br>
> - return make_error<RawError>(raw_error_code::corrupt_file,<br>
> - "Present bit vector does not match size!");<br>
> -<br>
> - if (auto EC = readSparseBitVector(Stream, Deleted))<br>
> - return EC;<br>
> - if (Present.intersects(Deleted))<br>
> - return make_error<RawError>(raw_error_code::corrupt_file,<br>
> - "Present bit vector interesects deleted!");<br>
> -<br>
> - for (uint32_t P : Present) {<br>
> - if (auto EC = Stream.readInteger(Buckets[P].first))<br>
> - return EC;<br>
> - if (auto EC = Stream.readInteger(Buckets[P].second))<br>
> - return EC;<br>
> - }<br>
> -<br>
> - return Error::success();<br>
> -}<br>
> -<br>
> -uint32_t HashTable::calculateSerializedLength() const {<br>
> - uint32_t Size = sizeof(Header);<br>
> -<br>
> - int NumBitsP = Present.find_last() + 1;<br>
> - int NumBitsD = Deleted.find_last() + 1;<br>
> -<br>
> - // Present bit set number of words, followed by that many actual words.<br>
> - Size += sizeof(uint32_t);<br>
> - Size += alignTo(NumBitsP, sizeof(uint32_t));<br>
> -<br>
> - // Deleted bit set number of words, followed by that many actual words.<br>
> - Size += sizeof(uint32_t);<br>
> - Size += alignTo(NumBitsD, sizeof(uint32_t));<br>
> -<br>
> - // One (Key, Value) pair for each entry Present.<br>
> - Size += 2 * sizeof(uint32_t) * size();<br>
> -<br>
> - return Size;<br>
> -}<br>
> -<br>
> -Error HashTable::commit(BinaryStreamWriter &Writer) const {<br>
> - Header H;<br>
> - H.Size = size();<br>
> - H.Capacity = capacity();<br>
> - if (auto EC = Writer.writeObject(H))<br>
> - return EC;<br>
> -<br>
> - if (auto EC = writeSparseBitVector(Writer, Present))<br>
> - return EC;<br>
> -<br>
> - if (auto EC = writeSparseBitVector(Writer, Deleted))<br>
> - return EC;<br>
> -<br>
> - for (const auto &Entry : *this) {<br>
> - if (auto EC = Writer.writeInteger(Entry.first))<br>
> - return EC;<br>
> - if (auto EC = Writer.writeInteger(Entry.second))<br>
> - return EC;<br>
> - }<br>
> - return Error::success();<br>
> -}<br>
> -<br>
> -void HashTable::clear() {<br>
> - Buckets.resize(8);<br>
> - Present.clear();<br>
> - Deleted.clear();<br>
> -}<br>
> -<br>
> -uint32_t HashTable::capacity() const { return Buckets.size(); }<br>
> -<br>
> -uint32_t HashTable::size() const { return Present.count(); }<br>
> -<br>
> -HashTableIterator HashTable::begin() const { return HashTableIterator(*this); }<br>
> -<br>
> -HashTableIterator HashTable::end() const {<br>
> - return HashTableIterator(*this, 0, true);<br>
> -}<br>
> -<br>
> -HashTableIterator HashTable::find(uint32_t K) const {<br>
> - return find_as<IdentityTraits>(K, *this);<br>
> -}<br>
> -<br>
> -void HashTable::set(uint32_t K, uint32_t V) {<br>
> - set_as<IdentityTraits, uint32_t>(K, V, *this);<br>
> -}<br>
> -<br>
> -void HashTable::remove(uint32_t K) { remove_as<IdentityTraits>(K, *this); }<br>
> -<br>
> -uint32_t HashTable::get(uint32_t K) {<br>
> - auto I = find(K);<br>
> - assert(I != end());<br>
> - return (*I).second;<br>
> -}<br>
> -<br>
> -uint32_t HashTable::maxLoad(uint32_t capacity) { return capacity * 2 / 3 + 1; }<br>
> -<br>
> -Error HashTable::readSparseBitVector(BinaryStreamReader &Stream,<br>
> +Error llvm::pdb::readSparseBitVector(BinaryStreamReader &Stream,<br>
> SparseBitVector<> &V) {<br>
> uint32_t NumWords;<br>
> if (auto EC = Stream.readInteger(NumWords))<br>
> @@ -167,7 +44,7 @@ Error HashTable::readSparseBitVector(Bin<br>
> return Error::success();<br>
> }<br>
><br>
> -Error HashTable::writeSparseBitVector(BinaryStreamWriter &Writer,<br>
> +Error llvm::pdb::writeSparseBitVector(BinaryStreamWriter &Writer,<br>
> SparseBitVector<> &Vec) {<br>
> int ReqBits = Vec.find_last() + 1;<br>
> uint32_t NumWords = alignTo(ReqBits, sizeof(uint32_t)) / sizeof(uint32_t);<br>
> @@ -191,48 +68,3 @@ Error HashTable::writeSparseBitVector(Bi<br>
> }<br>
> return Error::success();<br>
> }<br>
> -<br>
> -HashTableIterator::HashTableIterator(const HashTable &Map, uint32_t Index,<br>
> - bool IsEnd)<br>
> - : Map(&Map), Index(Index), IsEnd(IsEnd) {}<br>
> -<br>
> -HashTableIterator::HashTableIterator(const HashTable &Map) : Map(&Map) {<br>
> - int I = Map.Present.find_first();<br>
> - if (I == -1) {<br>
> - Index = 0;<br>
> - IsEnd = true;<br>
> - } else {<br>
> - Index = static_cast<uint32_t>(I);<br>
> - IsEnd = false;<br>
> - }<br>
> -}<br>
> -<br>
> -HashTableIterator &HashTableIterator::operator=(const HashTableIterator &R) {<br>
> - Map = R.Map;<br>
> - return *this;<br>
> -}<br>
> -<br>
> -bool HashTableIterator::operator==(const HashTableIterator &R) const {<br>
> - if (IsEnd && R.IsEnd)<br>
> - return true;<br>
> - if (IsEnd != R.IsEnd)<br>
> - return false;<br>
> -<br>
> - return (Map == R.Map) && (Index == R.Index);<br>
> -}<br>
> -<br>
> -const std::pair<uint32_t, uint32_t> &HashTableIterator::operator*() const {<br>
> - assert(Map->Present.test(Index));<br>
> - return Map->Buckets[Index];<br>
> -}<br>
> -<br>
> -HashTableIterator &HashTableIterator::operator++() {<br>
> - while (Index < Map->Buckets.size()) {<br>
> - ++Index;<br>
> - if (Map->Present.test(Index))<br>
> - return *this;<br>
> - }<br>
> -<br>
> - IsEnd = true;<br>
> - return *this;<br>
> -}<br>
><br>
> Modified: llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp?rev=327647&r1=327646&r2=327647&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp?rev=327647&r1=327646&r2=327647&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp (original)<br>
> +++ llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp Thu Mar 15 10:38:26 2018<br>
> @@ -27,26 +27,27 @@<br>
> using namespace llvm;<br>
> using namespace llvm::pdb;<br>
><br>
> -namespace {<br>
> -struct NamedStreamMapTraits {<br>
> - static uint16_t hash(StringRef S, const NamedStreamMap &NS) {<br>
> - // In the reference implementation, this uses<br>
> - // HASH Hasher<ULONG*, USHORT*>::hashPbCb(PB pb, size_t cb, ULONG ulMod).<br>
> - // Here, the type HASH is a typedef of unsigned short.<br>
> - // ** It is not a bug that we truncate the result of hashStringV1, in fact<br>
> - // it is a bug if we do not! **<br>
> - return static_cast<uint16_t>(hashStringV1(S));<br>
> - }<br>
> - static StringRef realKey(uint32_t Offset, const NamedStreamMap &NS) {<br>
> - return NS.getString(Offset);<br>
> - }<br>
> - static uint32_t lowerKey(StringRef S, NamedStreamMap &NS) {<br>
> - return NS.appendStringData(S);<br>
> - }<br>
> -};<br>
> -} // namespace<br>
> +NamedStreamMapTraits::NamedStreamMapTraits(NamedStreamMap &NS) : NS(&NS) {}<br>
> +<br>
> +uint16_t NamedStreamMapTraits::hashLookupKey(StringRef S) const {<br>
> + // In the reference implementation, this uses<br>
> + // HASH Hasher<ULONG*, USHORT*>::hashPbCb(PB pb, size_t cb, ULONG ulMod).<br>
> + // Here, the type HASH is a typedef of unsigned short.<br>
> + // ** It is not a bug that we truncate the result of hashStringV1, in fact<br>
> + // it is a bug if we do not! **<br>
> + return static_cast<uint16_t>(hashStringV1(S));<br>
> +}<br>
> +<br>
> +StringRef NamedStreamMapTraits::storageKeyToLookupKey(uint32_t Offset) const {<br>
> + return NS->getString(Offset);<br>
> +}<br>
> +<br>
> +uint32_t NamedStreamMapTraits::lookupKeyToStorageKey(StringRef S) {<br>
> + return NS->appendStringData(S);<br>
> +}<br>
><br>
> -NamedStreamMap::NamedStreamMap() {}<br>
> +NamedStreamMap::NamedStreamMap()<br>
> + : HashTraits(*this), OffsetIndexMap(HashTraits) {}<br>
><br>
> Error NamedStreamMap::load(BinaryStreamReader &Stream) {<br>
> uint32_t StringBufferSize;<br>
> @@ -98,7 +99,7 @@ uint32_t NamedStreamMap::hashString(uint<br>
> }<br>
><br>
> bool NamedStreamMap::get(StringRef Stream, uint32_t &StreamNo) const {<br>
> - auto Iter = OffsetIndexMap.find_as<NamedStreamMapTraits>(Stream, *this);<br>
> + auto Iter = OffsetIndexMap.find_as(Stream);<br>
> if (Iter == OffsetIndexMap.end())<br>
> return false;<br>
> StreamNo = (*Iter).second;<br>
> @@ -122,5 +123,5 @@ uint32_t NamedStreamMap::appendStringDat<br>
> }<br>
><br>
> void NamedStreamMap::set(StringRef Stream, uint32_t StreamNo) {<br>
> - OffsetIndexMap.set_as<NamedStreamMapTraits>(Stream, StreamNo, *this);<br>
> + OffsetIndexMap.set_as(Stream, support::ulittle32_t(StreamNo));<br>
> }<br>
><br>
> Modified: llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp?rev=327647&r1=327646&r2=327647&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp?rev=327647&r1=327646&r2=327647&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp (original)<br>
> +++ llvm/trunk/lib/DebugInfo/PDB/Native/TpiStream.cpp Thu Mar 15 10:38:26 2018<br>
> @@ -152,7 +152,9 @@ FixedStreamArray<TypeIndexOffset> TpiStr<br>
> return TypeIndexOffsets;<br>
> }<br>
><br>
> -HashTable &TpiStream::getHashAdjusters() { return HashAdjusters; }<br>
> +HashTable<support::ulittle32_t> &TpiStream::getHashAdjusters() {<br>
> + return HashAdjusters;<br>
> +}<br>
><br>
> CVTypeRange TpiStream::types(bool *HadError) const {<br>
> return make_range(TypeRecords.begin(HadError), TypeRecords.end());<br>
><br>
> Modified: llvm/trunk/tools/llvm-pdbutil/Analyze.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbutil/Analyze.cpp?rev=327647&r1=327646&r2=327647&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbutil/Analyze.cpp?rev=327647&r1=327646&r2=327647&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/tools/llvm-pdbutil/Analyze.cpp (original)<br>
> +++ llvm/trunk/tools/llvm-pdbutil/Analyze.cpp Thu Mar 15 10:38:26 2018<br>
> @@ -125,7 +125,7 @@ Error AnalysisStyle::dump() {<br>
><br>
> const auto &Collisions = CollisionsIter->second;<br>
> outs() << TypeName << "\n";<br>
> - outs() << formatv(" [HEAD] {0:x} {1} {2}\n", A.second,<br>
> + outs() << formatv(" [HEAD] {0:x} {1} {2}\n", uint32_t(A.second),<br>
> getLeafTypeName(HeadRecord.Type), TypeName);<br>
> for (const auto &Chain : Collisions) {<br>
> if (Chain.TI == TI)<br>
><br>
> Modified: llvm/trunk/unittests/DebugInfo/PDB/HashTableTest.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/DebugInfo/PDB/HashTableTest.cpp?rev=327647&r1=327646&r2=327647&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/DebugInfo/PDB/HashTableTest.cpp?rev=327647&r1=327646&r2=327647&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/unittests/DebugInfo/PDB/HashTableTest.cpp (original)<br>
> +++ llvm/trunk/unittests/DebugInfo/PDB/HashTableTest.cpp Thu Mar 15 10:38:26 2018<br>
> @@ -8,10 +8,14 @@<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> #include "llvm/DebugInfo/PDB/Native/HashTable.h"<br>
> +<br>
> +#include "llvm/DebugInfo/PDB/Native/Hash.h"<br>
> #include "llvm/DebugInfo/PDB/Native/NamedStreamMap.h"<br>
> +#include "llvm/Support/Allocator.h"<br>
> #include "llvm/Support/BinaryByteStream.h"<br>
> #include "llvm/Support/BinaryStreamReader.h"<br>
> #include "llvm/Support/BinaryStreamWriter.h"<br>
> +#include "llvm/Support/StringSaver.h"<br>
> #include "llvm/Testing/Support/Error.h"<br>
><br>
> #include "gtest/gtest.h"<br>
> @@ -23,7 +27,8 @@ using namespace llvm::pdb;<br>
> using namespace llvm::support;<br>
><br>
> namespace {<br>
> -class HashTableInternals : public HashTable {<br>
> +<br>
> +class HashTableInternals : public HashTable<uint32_t> {<br>
> public:<br>
> using HashTable::Buckets;<br>
> using HashTable::Present;<br>
> @@ -32,18 +37,18 @@ public:<br>
> }<br>
><br>
> TEST(HashTableTest, TestSimple) {<br>
> - HashTable Table;<br>
> + HashTableInternals Table;<br>
> EXPECT_EQ(0u, Table.size());<br>
> EXPECT_GT(Table.capacity(), 0u);<br>
><br>
> - Table.set(3, 7);<br>
> + Table.set_as(3, 7);<br>
> EXPECT_EQ(1u, Table.size());<br>
> - ASSERT_NE(Table.end(), Table.find(3));<br>
> + ASSERT_NE(Table.end(), Table.find_as(3));<br>
> EXPECT_EQ(7u, Table.get(3));<br>
> }<br>
><br>
> TEST(HashTableTest, TestCollision) {<br>
> - HashTable Table;<br>
> + HashTableInternals Table;<br>
> EXPECT_EQ(0u, Table.size());<br>
> EXPECT_GT(Table.capacity(), 0u);<br>
><br>
> @@ -53,39 +58,33 @@ TEST(HashTableTest, TestCollision) {<br>
> uint32_t N1 = Table.capacity() + 1;<br>
> uint32_t N2 = 2 * N1;<br>
><br>
> - Table.set(N1, 7);<br>
> - Table.set(N2, 12);<br>
> + Table.set_as(N1, 7);<br>
> + Table.set_as(N2, 12);<br>
> EXPECT_EQ(2u, Table.size());<br>
> - ASSERT_NE(Table.end(), Table.find(N1));<br>
> - ASSERT_NE(Table.end(), Table.find(N2));<br>
> + ASSERT_NE(Table.end(), Table.find_as(N1));<br>
> + ASSERT_NE(Table.end(), Table.find_as(N2));<br>
><br>
> EXPECT_EQ(7u, Table.get(N1));<br>
> EXPECT_EQ(12u, Table.get(N2));<br>
> }<br>
><br>
> TEST(HashTableTest, TestRemove) {<br>
> - HashTable Table;<br>
> + HashTableInternals Table;<br>
> EXPECT_EQ(0u, Table.size());<br>
> EXPECT_GT(Table.capacity(), 0u);<br>
><br>
> - Table.set(1, 2);<br>
> - Table.set(3, 4);<br>
> + Table.set_as(1, 2);<br>
> + Table.set_as(3, 4);<br>
> EXPECT_EQ(2u, Table.size());<br>
> - ASSERT_NE(Table.end(), Table.find(1));<br>
> - ASSERT_NE(Table.end(), Table.find(3));<br>
> + ASSERT_NE(Table.end(), Table.find_as(1));<br>
> + ASSERT_NE(Table.end(), Table.find_as(3));<br>
><br>
> EXPECT_EQ(2u, Table.get(1));<br>
> EXPECT_EQ(4u, Table.get(3));<br>
> -<br>
> - Table.remove(1u);<br>
> - EXPECT_EQ(1u, Table.size());<br>
> - EXPECT_EQ(Table.end(), Table.find(1));<br>
> - ASSERT_NE(Table.end(), Table.find(3));<br>
> - EXPECT_EQ(4u, Table.get(3));<br>
> }<br>
><br>
> TEST(HashTableTest, TestCollisionAfterMultipleProbes) {<br>
> - HashTable Table;<br>
> + HashTableInternals Table;<br>
> EXPECT_EQ(0u, Table.size());<br>
> EXPECT_GT(Table.capacity(), 0u);<br>
><br>
> @@ -96,31 +95,17 @@ TEST(HashTableTest, TestCollisionAfterMu<br>
> uint32_t N2 = N1 + 1;<br>
> uint32_t N3 = 2 * N1;<br>
><br>
> - Table.set(N1, 7);<br>
> - Table.set(N2, 11);<br>
> - Table.set(N3, 13);<br>
> + Table.set_as(N1, 7);<br>
> + Table.set_as(N2, 11);<br>
> + Table.set_as(N3, 13);<br>
> EXPECT_EQ(3u, Table.size());<br>
> - ASSERT_NE(Table.end(), Table.find(N1));<br>
> - ASSERT_NE(Table.end(), Table.find(N2));<br>
> - ASSERT_NE(Table.end(), Table.find(N3));<br>
> + ASSERT_NE(Table.end(), Table.find_as(N1));<br>
> + ASSERT_NE(Table.end(), Table.find_as(N2));<br>
> + ASSERT_NE(Table.end(), Table.find_as(N3));<br>
><br>
> EXPECT_EQ(7u, Table.get(N1));<br>
> EXPECT_EQ(11u, Table.get(N2));<br>
> EXPECT_EQ(13u, Table.get(N3));<br>
> -<br>
> - // Remove the one that had been filled in the middle, then insert another one<br>
> - // with a collision. It should fill the newly emptied slot.<br>
> - Table.remove(N2);<br>
> - uint32_t N4 = N1 * 3;<br>
> - Table.set(N4, 17);<br>
> - EXPECT_EQ(3u, Table.size());<br>
> - ASSERT_NE(Table.end(), Table.find(N1));<br>
> - ASSERT_NE(Table.end(), Table.find(N3));<br>
> - ASSERT_NE(Table.end(), Table.find(N4));<br>
> -<br>
> - EXPECT_EQ(7u, Table.get(N1));<br>
> - EXPECT_EQ(13u, Table.get(N3));<br>
> - EXPECT_EQ(17u, Table.get(N4));<br>
> }<br>
><br>
> TEST(HashTableTest, Grow) {<br>
> @@ -128,15 +113,15 @@ TEST(HashTableTest, Grow) {<br>
> // guaranteed to trigger a grow. Then verify that the size is the same, the<br>
> // capacity is larger, and all the original items are still in the table.<br>
><br>
> - HashTable Table;<br>
> + HashTableInternals Table;<br>
> uint32_t OldCapacity = Table.capacity();<br>
> for (uint32_t I = 0; I < OldCapacity; ++I) {<br>
> - Table.set(OldCapacity + I * 2 + 1, I * 2 + 3);<br>
> + Table.set_as(OldCapacity + I * 2 + 1, I * 2 + 3);<br>
> }<br>
> EXPECT_EQ(OldCapacity, Table.size());<br>
> EXPECT_GT(Table.capacity(), OldCapacity);<br>
> for (uint32_t I = 0; I < OldCapacity; ++I) {<br>
> - ASSERT_NE(Table.end(), Table.find(OldCapacity + I * 2 + 1));<br>
> + ASSERT_NE(Table.end(), Table.find_as(OldCapacity + I * 2 + 1));<br>
> EXPECT_EQ(I * 2 + 3, Table.get(OldCapacity + I * 2 + 1));<br>
> }<br>
> }<br>
> @@ -145,7 +130,7 @@ TEST(HashTableTest, Serialization) {<br>
> HashTableInternals Table;<br>
> uint32_t Cap = Table.capacity();<br>
> for (uint32_t I = 0; I < Cap; ++I) {<br>
> - Table.set(Cap + I * 2 + 1, I * 2 + 3);<br>
> + Table.set_as(Cap + I * 2 + 1, I * 2 + 3);<br>
> }<br>
><br>
> std::vector<uint8_t> Buffer(Table.calculateSerializedLength());<br>
> @@ -207,3 +192,73 @@ TEST(HashTableTest, NamedStreamMap) {<br>
> EXPECT_EQ(7U, N);<br>
> } while (std::next_permutation(Streams.begin(), Streams.end()));<br>
> }<br>
> +<br>
> +namespace {<br>
> +struct FooBar {<br>
> + std::string S;<br>
> + uint32_t X;<br>
> + uint32_t Y;<br>
> + double Z;<br>
> +};<br>
> +<br>
> +} // namespace<br>
> +<br>
> +namespace llvm {<br>
> +namespace pdb {<br>
> +template <> struct PdbHashTraits<FooBar> {<br>
> + std::vector<char> Buffer;<br>
> +<br>
> + PdbHashTraits() { Buffer.push_back(0); }<br>
> +<br>
> + uint32_t hashLookupKey(StringRef S) const {<br>
> + return llvm::pdb::hashStringV1(S);<br>
> + }<br>
> +<br>
> + StringRef storageKeyToLookupKey(uint32_t N) const {<br>
> + if (N >= Buffer.size())<br>
> + return StringRef();<br>
> +<br>
> + return StringRef(Buffer.data() + N);<br>
> + }<br>
> +<br>
> + uint32_t lookupKeyToStorageKey(StringRef S) {<br>
> + uint32_t N = Buffer.size();<br>
> + Buffer.insert(Buffer.end(), S.begin(), S.end());<br>
> + Buffer.push_back('\0');<br>
> + return N;<br>
> + }<br>
> +};<br>
> +} // namespace pdb<br>
> +} // namespace llvm<br>
> +<br>
> +TEST(HashTableTest, NonTrivialValueType) {<br>
> + HashTable<FooBar> Table;<br>
> + uint32_t Cap = Table.capacity();<br>
> + for (uint32_t I = 0; I < Cap; ++I) {<br>
> + FooBar F;<br>
> + F.S = utostr(I);<br>
> + F.X = I;<br>
> + F.Y = I + 1;<br>
> + F.Z = static_cast<double>(I + 2);<br>
> + Table.set_as(utostr(I), F);<br>
> + }<br>
> +<br>
> + std::vector<uint8_t> Buffer(Table.calculateSerializedLength());<br>
> + MutableBinaryByteStream Stream(Buffer, little);<br>
> + BinaryStreamWriter Writer(Stream);<br>
> + EXPECT_THAT_ERROR(Table.commit(Writer), Succeeded());<br>
> + // We should have written precisely the number of bytes we calculated earlier.<br>
> + EXPECT_EQ(Buffer.size(), Writer.getOffset());<br>
> +<br>
> + HashTable<FooBar> Table2;<br>
> + BinaryStreamReader Reader(Stream);<br>
> + EXPECT_THAT_ERROR(Table2.load(Reader), Succeeded());<br>
> + // We should have read precisely the number of bytes we calculated earlier.<br>
> + EXPECT_EQ(Buffer.size(), Reader.getOffset());<br>
> +<br>
> + EXPECT_EQ(Table.size(), Table2.size());<br>
> + EXPECT_EQ(Table.capacity(), Table2.capacity());<br>
> + // EXPECT_EQ(Table.Buckets, Table2.Buckets);<br>
> + // EXPECT_EQ(Table.Present, Table2.Present);<br>
> + // EXPECT_EQ(Table.Deleted, Table2.Deleted);<br>
> +}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
><br>
</blockquote></div>