[PATCH] D139097: [ARM] Add option --print-raw-value to llvm-nm to dump raw symbol values in case of ARM

[Subham Kedia via Phabricator via llvm-commits]

quic-subhkedi marked 4 inline comments as done.
quic-subhkedi added inline comments.


================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:627
+  if (!SectionAddressOrError)
+    return SectionAddressOrError.takeError();
+
----------------
jhenderson wrote:
> peter.smith wrote:
> > The code base has many  `// TODO: Test this error.` comments, is it worth replicating them here as this is the same code pattern?
> > 
> > Not a strong opinion. 
> Some context: these were added by @grimar some time ago. Ideally, all our code paths should have at least one test that exercises that code path, so in this case, we'd want a test case that shows what happens when there's an error fetching the address. The TODOs were added because at the time, it either was tricky or completely impossible to craft a test case, given the tools we have. That might not be the case here (but if it is, a TODO may be appropriate).
yes it had to replicated


================
Comment at: llvm/tools/llvm-nm/Opts.td:86
+// ARM and microMIPS specific option.
+def print_raw_values:  FF<"print-raw-values", "Print raw values for symbols">, Flags<[HelpHidden]>;
----------------
jhenderson wrote:
> What's the motivation for hiding this option?
doesn't need to be hidden so removed


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

https://reviews.llvm.org/D139097