[PATCH] D56629: [llvm-readelf] Allow single-letter flags to be merged.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/dyn-symbols.test:3-4
+
+# Check the two-letter aliases -dt is equivalent to --dyn-symbols
+# full flag names.
+RUN: llvm-readobj -dt %p/Inputs/dynamic-table-so.x86 > %t.readobj-dt-alias
----------------
aliases -> alias, names -> name


================
Comment at: test/tools/llvm-readobj/dyn-symbols.test:7
+RUN: llvm-readobj --dyn-symbols %p/Inputs/dynamic-table-so.x86 > %t.readobj-dt-no-alias
+RUN: cmp %t.readobj-dt-alias %t.readobj-dt-no-alias
+
----------------
Would diff make more sense here, since these are text files?


================
Comment at: test/tools/llvm-readobj/dyn-symbols.test:38
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name: _ITM_deregisterTMCloneTable
+# CHECK-NEXT:     Value: 0x0
----------------
Here and below, could you check that there are no extraneous characters at the end of the name, by putting a {{$}} at the end, please? This would match other names for which this is a prefix. You might need to capture the string offset as a result.


================
Comment at: test/tools/llvm-readobj/merged.test:10
+# were supported.
+RUN: not llvm-readobj -aeWhSrnudlVgIs %p/Inputs/trivial.obj.elf-i386 |& FileCheck %s --check-prefix=UNKNOWN
+
----------------
rupprecht wrote:
> jhenderson wrote:
> > What's with the `|&` ?
> It redirects stderr (i.e. the "unknown command line arg" we're checking) in a pipeline: https://www.gnu.org/software/bash/manual/bash.html#Pipelines
> 
> But 2>&1 works just as well and is more widely used (I can't find |& in any llvm tests, so lit probably doesn't support it), so I'll just use that.
Ah, I didn't know about that. Cool, but yes, use 2>&1 instead!


================
Comment at: tools/llvm-readobj/llvm-readobj.cpp:633
+  // The following two-letter aliases are only provided for readobj, as readelf
+  // allows single-letter to be grouped together.
+  static cl::alias SectionRelocationsShort(
----------------
missing "args" after the latest change, i.e. "single-letter" to "single-letter args"


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56629