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