[PATCH] D136702: [llvm-cov] Look up object files using debuginfod.

Mon Nov 7 01:24:42 PST 2022

jhenderson added a comment.

Couple of drive-by comments, as my Herald rule triggered due to the llvm-objdump/llvm-symbolizer changes (which look fine to me).

================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:61
+  if (!Urls) {
+    consumeError(Urls.takeError());
+    return false;
----------------
In other parts of the codebase, `consumeError` without any explanation is generally a code smell. A comment explaining why it's appropriate instead of returning the error up the stack would be worthwhile here.

================
Comment at: llvm/tools/llvm-cov/CodeCoverage.cpp:657
+      "debuginfod", cl::Optional,
+      cl::desc("Use debuginfod to look up object files from profile."),
+      cl::init(canUseDebuginfod()));
----------------
There seems to be an unfortunate mixture of help descriptions ending with full stops and others that don't. It seems no full stop in this file is more prevalent, so please remove it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136702