[PATCH] D150987: [llvm-nm] Add --line-numbers flag

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/test/tools/llvm-nm/X86/line-numbers.test:12
+#   int main() {
+#     return e;
+#   }
----------------
jhenderson wrote:
> As well as an undefined data symbol, you might want an undefined function too.
`e` is not declared in this file. I agree that testing a function will be useful. Ideally, also test an undefined symbol that is not referenced (`.globl und` without a label definition gives an unreferenced undefined symbol)


================
Comment at: llvm/test/tools/llvm-nm/X86/line-numbers.test:20
+CHECK: 0000000000000010 T bar	/tmp{{[/\\]}}tmp.c:7
+CHECK: 0000000000000000 B c	/tmp{{[/\\]}}tmp.c:1
+CHECK: 0000000000000008 B d	/tmp{{[/\\]}}tmp.c:2
----------------
Add `-NEXT:` whenever applicable.

By default the symbol list is sorted by name/size/addressed, so we don't need to worry about shuffled symbol table by llvm-mc updates.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:2417
   StringRef V = Args.getLastArgValue(OPT_format_EQ, "bsd");
+  LineNumbers = Args.hasArg(OPT_line_numbers);
   if (V == "bsd")
----------------
This separates `OPT_format_EQ` and its values.  Consider moving this immediately before `NoLLVMBitcode`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150987