[PATCH] D112758: [llvm] [Debuginfo] Debuginfod client library.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Debuginfod/CMakeLists.txt:1
+if(LLVM_ENABLE_CURL)
+  add_llvm_component_library(LLVMDebuginfod
----------------
labath wrote:
> This may be the first case, where the very existence of a component library is predicated on the availability of a third party dep. That's not necessarily a bad thing (it'd also be strange to enable it when it is completely non-functional), but it did catch my eye.
Yeah, I had mixed feelings about this with the machine learning - if the API is going to always be exposed, maybe the API should be in the library, and it gets compiled into a trivially uninteresting library that always fails the query - rather than having the entry point outside the library, and having that entry point not call into the library if it isn't available/couldn't do something real?


================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:34
+// Uses lowercase for compatibility with common debuginfod servers.
+std::string buildIDToString(const ArrayRef<uint8_t> BuildID) {
+  return llvm::toHex(BuildID, /*LowerCase=*/true);
----------------
You can drop the top level const here and in similar places.


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

https://reviews.llvm.org/D112758