[PATCH] D88715: [llvm-objdump] --source: drop the warning when there is no debug info

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1018
+  } else {
+    // TODO Untested.
+    reportWarning("failed to parse debug information for " + ObjectFilename +
----------------
MaskRay wrote:
> rupprecht wrote:
> > It would be good to figure out a test case for this
> The last paragraph of the description mentions this.
> 
> "The only code path is probably a broken symbol table, but we probably already emit a warning
> in that case" (I have checked various code paths)
> 
> If it is a symbol table problem, it should not be a "debug info problem".
The error message still says "failed to parse debug information", did you have plans to change that if it's not a debug info problem?


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1019-1021
+    reportWarning("failed to parse debug information for " + ObjectFilename +
+                      ": " + toString(ExpectedLineInfo.takeError()),
+                  ObjectFilename);
----------------
MaskRay wrote:
> rupprecht wrote:
> > `ObjectFilename` is in the warning twice now. It can be omitted from the error message string; passing it as the second arg should make it print something like: `warn: ObjectFilename: failed to parse debug information`
> The code path is likely dead. This can be considered when ExpectedLineInfo does return an `Error`. (Note, there is no test for multiple warnings)
I'm not sure what you mean by dead here -- are you saying `Symbolizer->symbolizeCode()` never returns an error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88715