[PATCH] D34542: Introduce symbol cache to PDB NativeSession

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Native/NativeSession.h:35
 
+  std::unique_ptr<PDBSymbol> createCompilandSymbol(DbiModuleDescriptor MI);
+
----------------
Should this return a `PDBSymbolCompiland`?


================
Comment at: llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp:75-76
+  const auto Id = static_cast<uint32_t>(SymbolCache.size());
+  SymbolCache.emplace_back(std::unique_ptr<NativeRawSymbol>(
+      new NativeCompilandSymbol(*this, Id, MI)));
+  return std::unique_ptr<PDBSymbol>(new PDBSymbolCompiland(
----------------
`push_back` instead of `emplace_back` (same as below)


================
Comment at: llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp:77-78
+      new NativeCompilandSymbol(*this, Id, MI)));
+  return std::unique_ptr<PDBSymbol>(new PDBSymbolCompiland(
+      *this, std::unique_ptr<IPDBRawSymbol>(SymbolCache[Id]->clone())));
+}
----------------
`make_unique` again.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp:88
+  SymbolCache.emplace_back(
+      std::unique_ptr<NativeRawSymbol>(new NativeExeSymbol(*this, Id)));
+  auto RawSymbol = SymbolCache[Id]->clone();
----------------
`make_unique`?  Also, since you're passing the value type directly, you should use `push_back` instead of `emplace_back`


https://reviews.llvm.org/D34542