[PATCH] D53002: Create a new symbol file plugin for cross-platform PDB symbolization

[Leonard Mosescu via Phabricator via llvm-commits]

lemo accepted this revision.
lemo added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbIndex.cpp:34
+PdbIndex::create(std::unique_ptr<llvm::pdb::PDBFile> file) {
+  assert(file);
+  auto ExpectedDBI = file->getPDBDbiStream();
----------------
zturner wrote:
> lemo wrote:
> > lldbassert() ? 
> > 
> > (same comment for the rest of assert()s in the CR)
> `lldbassert` is nice to validate things that you expect should happen but want to log an error if the condition is not held.  `assert` is better when you want to guarantee invariants of your API.  In this case I use `assert` because the precondition of this function is "`file` is non-null".  `lldbassert` wouldn't help here, because the very next line of code is to just dereference the pointer and we'd crash anyway.
I believe that lldbassert has been fixed (lldb_private::lldb_assert() unconditionally ends with calling abort())


https://reviews.llvm.org/D53002