[PATCH] D34542: Introduce symbol cache to PDB NativeSession

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth marked 3 inline comments as done.
amccarth added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Native/NativeSession.h:35
 
+  std::unique_ptr<PDBSymbol> createCompilandSymbol(DbiModuleDescriptor MI);
+
----------------
zturner wrote:
> Should this return a `PDBSymbolCompiland`?
That's a fair question.  This factory method (and the others that are in the pipeline) are typically invoked in implementations of `IPDBSymbol::getChildAtIndex`, which return `std::unique_ptr<PDBSymbol>`, so I was going with the base type, but I suppose there's no harm in returning the derived type and letting the implicit conversion happen.


================
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())));
+}
----------------
zturner wrote:
> `make_unique` again.
I need to construct an instance of the derived type but return it as a unique_ptr to the base type.  I've had trouble getting VS to accept that with std::make_unique, which is why I was using new directly.  But it seems to work with llvm::make_unique, so I guess that'll do.


https://reviews.llvm.org/D34542