[PATCH] D62255: [llvm-objdump][test] Improve testing of some switches #3

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Thank you for improving tests! I only have a couple of nits.



================
Comment at: test/tools/llvm-objdump/X86/disasm-specific-funcs-mangled-name.test:6
+# RUN: llvm-objdump -d --disassemble-functions=_Z3foov %t.o | FileCheck %s
+# RUN: llvm-objdump -d --disassemble-functions=foo() %t.o | FileCheck %s --check-prefix=NOFOO
+# RUN: llvm-objdump -d -C --disassemble-functions=foo() %t.o | FileCheck %s --check-prefix=NOFOO
----------------
jhenderson wrote:
> rupprecht wrote:
> > `--disassemble-functions=foo()` should be single quoted to handle (), e.g.
> > 
> > ```
> > $ echo foo()
> > bash: syntax error near unexpected token `('
> > ```
> I'll check this before committing. I had this vague impression that lit quotes the arguments, but I might be completely wrong.
Even if lit supports it, quoting makes it easier to copy-n-paste the command.

In zsh it starts a function definition:

```
% echo foo()  
function>
```


================
Comment at: test/tools/llvm-objdump/X86/disasm-specific-funcs-mangled-name.test:10
+
+# CHECK:       Disassembly of section .text:
+# CHECK-EMPTY:
----------------
`Disassembly of section .text:` and the next line (empty) may be omitted.


================
Comment at: test/tools/llvm-objdump/X86/disassemble-long-instructions.test:1
+## This test shows that llvm-objdump can print the disassembly a long instruction.
+# RUN: yaml2obj %s -o %t.o
----------------
> print the disassembly a long instruction.

Should this be reworded a bit?


So we have both `disasm-*` and `disassemble-*` tests... You may rename the existing ones and rename this to `disasm-*` if you want to keep consistency.


================
Comment at: test/tools/llvm-objdump/X86/elf-disassembly-no-symtab.test:1
+## Show that llvm-objdump can handle a missing symbol table when printing
+## references and labels.
----------------
This patch adds

`elf-disassembly-no-symtab.test`
`elf-disassemble-relocs.test`
`elf-disasm-dynamic-symbols.test`

I hope the inconsistency is not intentional..


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62255