[PATCH] D71116: [test][tools] Add missing/Improve testing

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-nm/elf-extern-only.test:5
+# RUN: llvm-nm %t.o --extern-only --no-sort | FileCheck %s
+# RUN: llvm-nm %t.o -g --no-sort | FileCheck %s
+
----------------
grimar wrote:
> I do not see `-g` in the llvm-nm`s help text.
> Had to look at GNU.
> (this and similar comments above are unrelated to this patch, I've put them just FTR).
LLVM's command-line handler by default hides aliases. I don't know why. In some cases, we've explicitly marked them as not hidden, but not in others. I've got cleaning up the help text (a long way down) on my backlog, so hopefully it'll get fixed at some point.

I believe all the aliases should be in the LLVM Command Guide documentation by the way.


================
Comment at: llvm/test/tools/llvm-nm/elf-extern-only.test:8
+## Using --no-sort ensures that the local symbols (if present) would appear
+## first in the output.
+# CHECK-NOT: local
----------------
grimar wrote:
> Not sure the description is clear to me. `--no-sort` just doesn't change symbols order, right?
> (help text says "Show symbols in order encountered")
> 
> What if we place one more local symbol at the end of the symbol table? (yes, it is not a valid ELF case probably,
> but why not for this test case).
I've modified the comment a bit to make that point clearer.


================
Comment at: llvm/test/tools/llvm-readobj/elf-dynamic-tags.test:3
+## Also show that -d is an alias for --dynamic-table.
 # RUN: yaml2obj %s > %t
 # RUN: llvm-readobj --dynamic-table %t | FileCheck %s --check-prefix=LLVM
----------------
grimar wrote:
> Add an empty line after the comment?
Okay, although I only usually do this where there is more than one case. In this case, there aren't any other blocks covered by this comment.


================
Comment at: llvm/test/tools/llvm-readobj/elf-relocations.test:50
+
+## Show that --expand-relocs expands the relocation dump for LLVM style only.
+# RUN: llvm-readobj -r --expand-relocs %t64 \
----------------
grimar wrote:
> I.e. `--expand-relocs` is no-op for GNU? Perhaps worth to mention that in the comment here and below.
That's what the "LLVM style only" bit is meant to say. I'll clarify it a bit more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71116