labath added inline comments.

================
Comment at: source/Commands/CommandObjectTarget.cpp:4246
           if (symbol_file) {
-            ObjectFile *object_file = symbol_file->GetObjectFile();
 
----------------
lemo wrote:
> note I had to bypass this check: we don't (yet) have a ObjectFilePDB so the 
> SymbolFileNativePDB always points to the associated PE binary. 
> 
> the check itself seems valuable as a diagnostic but not strictly required. 
> Should I add a TODO comment and/or open a bug to revisit this?
I not sure this is a good idea. Isn't this the only way of providing feedback 
about whether the symbols were actually added? If we are unable to load the 
symbol file specified (perhaps because the user made a typo, or the file is 
corrupted), then the symbol vendor will just create a default SymbolFile backed 
by the original object file. Doesn't that mean this will basically always 
return true now?

I think this is strictly worse that the previous solution as it lets the 
objectless-symbol-file hack leak out of SymbolFilePDB.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55142/new/

https://reviews.llvm.org/D55142



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to