davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: source/Core/Module.cpp:1286
+    if (SymbolVendor *vendor = GetSymbolVendor())
+      vendor->CreateSections(*GetUnifiedSectionList());
   }
----------------
labath wrote:
> clayborg wrote:
> > should we pass "obj_file" down into the SymbolVendor::CreateSections(...) 
> > call for the case where the symbol vendor is using the same ObjectFile that 
> > it can just return?
> I don't think an extra parameter is needed to achieve this, as this is 
> something that the symbol vendor should already know about based on how it 
> was constructed.
> 
> And after looking up the implementation, it seems this is already how it 
> works right now: SymbolVendorELF (the only non-trivial implementation of this 
> function) is only constructed if it manages to find debug info in an 
> alternative symbol file (there's an `if 
> (llvm::sys::fs::equivalent(candidate_file_spec.GetPath(), 
> module_file_spec.GetPath()))` check in 
> `Symbols::LocateExecutableSymbolFile`). If we fail to construct a 
> SymbolVendorELF then a default SymbolVendor instance is created in 
> `SymbolVendor::FindPlugin` (and that one has a no-op implementation of this 
> function).
> 
I agree with Pavel's reasoning here.


https://reviews.llvm.org/D42955



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

Reply via email to