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

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 9 10:23:06 PDT 2018


zturner added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:307
       std::make_shared<Function>(sc.comp_unit, pdb_func.getSymIndexId(),
-                                 func_type_uid, mangled, func_type, func_range);
+                                 func_type_uid, mangled, nullptr, func_range);
 
----------------
aleksandr.urakov wrote:
> zturner wrote:
> > aleksandr.urakov wrote:
> > > Why shouldn't we pass a type here? Isn't it unrelated to the new plugin?
> > Nice catch!  We actually should pass a type here.  The reason I'm not doing it is because that would require implementing the `PDBASTParser` class.  Obviously we need to do that eventually, but for now it wasn't necessary in order for LLDB to not complain.  So I was trying to keep the patch small and just implement the minimum amount needed for *something* to work.  But yes, we will need to do this.
> But here is the `SymbolFilePDB` part, not `SymbolFileNativePDB`, and `PDBASTParser` is already implemented for it? And we even retrieve the type in the line 297... If I understand right, it will be never called if `LLDB_USE_NATIVE_PDB_READER` is on?
Oh, yea you are right.  I changed this in `SymbolFilePDB` because I was testing whether LLDB would still display something nice.  THen I forgot to change it back.  So yea, I'll revert this back to how it was.  Sorry about that


https://reviews.llvm.org/D53002





More information about the llvm-commits mailing list