[Lldb-commits] [PATCH] D65952: SymbolVendor: Have plugins return symbol files directly

Thu Aug 22 11:15:06 PDT 2019

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