clayborg added a comment.

In D65952#1624799 <https://reviews.llvm.org/D65952#1624799>, @labath wrote:

> In D65952#1623297 <https://reviews.llvm.org/D65952#1623297>, @clayborg wrote:
>
> > So I am confused. Are we keeping SymbolVendor around for locating symbols 
> > files or are we getting rid of it entirely?
>
>
> Well... right now my idea was to keep it around as an class with just some 
> static methods for the purposes of locating symbol files, though I'm open to 
> other ideas too. After this patch the SymbolVendor class will contain just 
> one method (FindPlugin), but I think that in the future it could be used to 
> host functionality that is common for different symbol vendors. Here, I'm 
> mainly thinking of `Symbols/LocateSymbolFile.cpp` (formerly 
> `Host/Symbols.cpp`), which contains a bunch of functionality for searching 
> for symbol files (so it could fall under the symbol vendor mandate), and it 
> is currently being called by other symbol vendors to do their job. However, 
> it is also being called from placed other than symbol vendors, and so more 
> refactoring would be needed for that to fit in neatly.


IMHO: The functionality that locates symbol files should be moved into the 
Platform and we can get rid of SymbolVendor completely. So sounds like 
refactoring around that is a good idea. I am fine with this patch standing and 
addressing that refactor in a next patch?


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

https://reviews.llvm.org/D65952



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

Reply via email to