[PATCH] D118413: [Debuginfod] [Symbolizer] Break debuginfod out of libLLVM.

[Daniel Thornburgh via Phabricator via llvm-commits]

mysterymath added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/Symbolize/DebugInfoFetcher.h:32-36
+  struct Concept {
+    virtual Optional<std::string>
+    fetchBuildID(ArrayRef<uint8_t> BuildID) const = 0;
+  };
+  template <typename T> class Model;
----------------
phosek wrote:
> Do we really need the concept? Wouldn't a simple inheritance with virtual functions be sufficient? IIUC the reason for using concept is if you want to opt types that you do not own into the abstract interface, but that's not what we need here since all the types are part of LLVM.
The other advantage is removing the requirement for any DIFetcher implementation to depend on Symbolizer. A concept allows the DIFetcher abstraction to properly belong to Symbolizer, i.e., point of use rather than point of implementation.

But, it's a fair amount of syntactic/semantic overhead for just that; my golang proclivities are probably showing a bit. I'm quite ambivalent about this; if this seems like overkill for the gain, let me know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118413