[PATCH] D103079: [XCOFF] [llvm-objdump] Add XCOFF recognition of debug section types under `--section-headers` option.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/XCOFF/section-headers-truncate.test:1
+## Truncate the file to end before the section header table ends.
+# RUN: yaml2obj %s -o %t.o
----------------
Didn't notice this before, but it sounds like this functionality is unrelated to the debug section types, so should be split off into its own patch.


================
Comment at: llvm/test/tools/llvm-objdump/XCOFF/section-headers.test:3
+
+## Check the `--section-headers/-h` prints the type of sections correctly.
+
----------------
I think you can drop the quotes around the option name. I'd also normalise the order to match the previous comment (i.e. `-h/--section-headers`).


================
Comment at: llvm/test/tools/llvm-objdump/XCOFF/section-headers.test:4
+## Check the `--section-headers/-h` prints the type of sections correctly.
+
+# RUN: yaml2obj %s --docnum=1 -o %t-type.o
----------------
Delete this blank line, so that the comment is clearly tied to the specific test case.


================
Comment at: llvm/test/tools/llvm-objdump/XCOFF/section-headers.test:48
+## Check the `--section-headers/-h` option prints long section names.
+
+# RUN: yaml2obj %s --docnum=2 -o %t-longname.o
----------------
Delete this blank line, so that the comment is clearly tied to the specific test case.


================
Comment at: llvm/test/tools/llvm-objdump/XCOFF/section-headers.test:49
+
+# RUN: yaml2obj %s --docnum=2 -o %t-longname.o
+# RUN: llvm-objdump --section-headers %t-longname.o \
----------------
shchenz wrote:
> Nit: should we put the RUN lines together at the front of this file?
> Nit: should we put the RUN lines together at the front of this file?

I don't think we should do this. The RUN/CHECK/YAML blocks are for two somewhat distinct test cases, so shouldn't be intermixed. Intermixing them would make the test harder to follow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103079