zturner added a comment.
Since it seems like you're going to be doing some work in here, it would be
great if you could update `lldb-test` to dump PDB symbol information. This
would allow us to easily test all sorts of things in here. For example, you
could find a PDB that returned an empty source file name, and then have the
tool dump something like:
Compiland: {foo}, Source File: {bar}
then we could just grep this output against FileCheck.
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:367-370
+ // For some reason, source file name could be empty, so we will walk through
+ // all source files of this compiland, and determine the right source file
+ // if any that is used to generate this compiland based on language
+ // indicated in compilanddetails language field.
----------------
Is it DIA that is returning the empty name?
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:371
+ // indicated in compilanddetails language field.
+ if (source_file_name.empty()) {
+ auto details_up = pdb_compiland->findOneChild<PDBSymbolCompilandDetails>();
----------------
Can you do an early return here?
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:382-389
+ (ConstString::Compare(file_extension, ConstString("CPP"),
+ false) == 0 ||
+ ConstString::Compare(file_extension, ConstString("C"),
+ false) == 0 ||
+ ConstString::Compare(file_extension, ConstString("CC"),
+ false) == 0 ||
+ ConstString::Compare(file_extension, ConstString("CXX"),
----------------
This would probably be shorter as:
```
llvm::is_contained({"cpp", "c", "cc", "cxx"},
file_extension.GetStringRef().lower());
```
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:683
+
+ if (cu_sp) {
+ m_comp_units.insert(std::make_pair(id, cu_sp));
----------------
Early return here.
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:763
auto prologue = func->findOneChild<PDBSymbolFuncDebugStart>();
- is_prologue = (addr == prologue->getVirtualAddress());
+ if (prologue.get())
+ is_prologue = (addr == prologue->getVirtualAddress());
----------------
nit: `.get()` shouldn't be necessary
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:767
auto epilogue = func->findOneChild<PDBSymbolFuncDebugEnd>();
- is_epilogue = (addr == epilogue->getVirtualAddress());
+ if (epilogue.get())
+ is_epilogue = (addr == epilogue->getVirtualAddress());
----------------
Same here.
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h:197
std::unique_ptr<llvm::pdb::IPDBSession> m_session_up;
+ std::unique_ptr<llvm::pdb::PDBSymbolExe> m_global_scope_up;
uint32_t m_cached_compile_unit_count;
----------------
Is this a performance win or just a convenience? I assumed the session itself
would cache the global scope, but maybe that assumption was wrong.
Repository:
rL LLVM
https://reviews.llvm.org/D41428
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
