<div dir="ltr">Looks like someone beat me to it.</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Mar 16, 2018 at 9:21 AM Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><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" target="_blank">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></blockquote></div>