[PATCH] D113106: demangle xcoff label symbol for llvm-nm

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/demangle.test:17
+
+# NM-DEMANGLE:      00000000 t
+# NM-DEMANGLE-NEXT: 00000040 T .func1(int)
----------------
I'd suggest doing one of the following:
1) adding --match-full-lines to your FileCheck command here
2) omitting the symbol value and adding `{{$}}` to the end of the symbol (omitting the symbol value removes noise from the test).

This will prevent this line accidentally matching another symbol with a name. It will also ensure the demangling output looks right on other lines.


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/demangle.test:20
+# NM-DEMANGLE-NEXT: 00000000 t .func0()
+# NM-DEMANGLE-NEXT: 00000120 b .bss
+# NM-DEMANGLE-NEXT: 00000100 d .data
----------------
This test is purely about demangling functionality. Aside from having a single test-case where a symbol isn't demangled, I don't think you need all these other cases that don't demangle.

Similarly, you can omit most of the test cases where demangling does occur, as long as you have one per code path.

I think the following set of test cases is all you should have - ignore the other symbols (or more preferably just rebase this patch on top of @Esme's yaml2obj work, so that you can use yaml2obj to generate exactly the right set of symbols):
# Empty name
# Name consisting solely of `.`
# Name starting with `.` that can't be demangled
# Name starting with `.` that can be demangled
# Name not starting with `.` that can't be demangled
# Name not starting with `.` that can be demangled


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/demangle.test:31
+# NM-DEMANGLE-NEXT: 0000011c W weak_value
+~
----------------
Why's this line here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113106