[PATCH] D35163: [PDB] Enable NativeSession to create symbols for built-in types on demand

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Native/NativeSession.h:42
 
+  uint32_t findSymbolByTypeIndex(uint32_t TI);
+
----------------
Any reason not to use codeview::TypeIndex for this parameter? Just trying to avoid exposing internal libraries out through this interface?


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Native/NativeSession.h:86
   std::vector<std::unique_ptr<NativeRawSymbol>> SymbolCache;
+  std::unordered_map<std::uint32_t, std::uint32_t> TypeIndexToSymbolId;
 };
----------------
Generally, LLVM uses bare uint32_t rather than std::uint32_t. You could argue it's C legacy, but that's currently the norm, so I'd rather stick with it.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp:44
+  PDB_BuiltinType Type;
+  uint64_t Size;
+} BuiltinTypes[] = {
----------------
It seems unlikely that a builtin type will grow beyond 4GB.


https://reviews.llvm.org/D35163