[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D75750#1991873 <https://reviews.llvm.org/D75750#1991873>, @labath wrote:

> In D75750#1988694 <https://reviews.llvm.org/D75750#1988694>, @jankratochvil wrote:
>
> > The current plan discussed with @kwk is to create the new `SymbolServer` abstract superclass and some its inherited implementation and move there the appropriate parts of existing `lldb/source/Symbol/LocateSymbolFile.cpp`. Current `SymbolVendor` implementations would then iterate new `SymbolServer`s by the existing `LocateExecutableSymbolFile` function. That may be enough for a patch of its own.
>
>
> I'll have to see the actual patch for a definitive opinion, but I have to say that a priori I am sceptical of this direction. And yes, that should definitely be a separate patch.


This separate `SymbolServer` is following @clayborg's comment above <https://reviews.llvm.org/D75750#1950734>.
You proposed to merge `SymbolServer` with `SymbolVendor` in @labath's comment above <https://reviews.llvm.org/D75750#1952130>.
I found more clean the separate `SymbolServer` variant as there is orthogonal functionality of locating the files (on disk or from symbol server - `SymbolServer`) vs. extracting the unique ID from current file (extracting build-id - `SymbolVendor` functionality). So from the both proposed solutions I preferred the @clayborg's comment above <https://reviews.llvm.org/D75750#1950734>.
I hope there is no misunderstanding which could lead to @kwk implementing a third solution nobody wants.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750