[yaml2obj] Refactor command line parsing

[MaskRay]

Summary

• Hide unrelated options.
• Add "OVERVIEW: " to yaml2obj -h/--help.
• Place options under a yaml2obj category.
• Disallow -docnum. Currently -docnum is the only yaml2obj specific long option that is affected.
• Specify cl::init("-") and cl::Prefix for OutputFilename. The latter allows -ofile

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[MaskRay]

Add cl::Prefix

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[MaskRay]

<input> -> <input file>

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[grimar]

With this patch applied I observe the different behavior between --help-list and -help-list:

umb@ubuntu:~/LLVM/LLVM/llvm-project/build/bin$ ./yaml2obj -help-list
OVERVIEW: Create an object file from YAML description
USAGE: yaml2obj [options] <input file>

OPTIONS:

Generic Options:

  --help          - Display available options (--help-hidden for more)
  --help-list     - Display list of available options (--help-list-hidden for more)
  --version       - Display the version of this program

yaml2obj Options:

  --docnum=<uint> - Read specified document from input (default = 1)
  -o=<filename>   - Output filename

umb@ubuntu:~/LLVM/LLVM/llvm-project/build/bin$ ./yaml2obj --help-list
OVERVIEW: Create an object file from YAML description
USAGE: yaml2obj [options] <input file>

OPTIONS:
  --docnum=<uint> - Read specified document from input (default = 1)
  --help          - Display available options (--help-hidden for more)
  -o=<filename>   - Output filename
  --version       - Display the version of this program

The original behavior was:

umb@ubuntu:~/LLVM/LLVM/llvm-project/build/bin$ ./yaml2obj --help-list
USAGE: yaml2obj [options] <input>

OPTIONS:
  --color         - Use colors in output (default=autodetect)
  --docnum=<uint> - Read specified document from input (default = 1)
  --help          - Display available options (--help-hidden for more)
  -o=<filename>   - Output filename
  --version       - Display the version of this program
umb@ubuntu:~/LLVM/LLVM/llvm-project/build/bin$ ./yaml2obj -help-list
USAGE: yaml2obj [options] <input>

OPTIONS:
  --color         - Use colors in output (default=autodetect)
  --docnum=<uint> - Read specified document from input (default = 1)
  --help          - Display available options (--help-hidden for more)
  -o=<filename>   - Output filename
  --version       - Display the version of this program

llvm/test/tools/yaml2obj/help.test

4

I'd use "# RUN:" for consistency with our other tests.

14

Would --implicit-check-not="Options:" be a better way to test it?

llvm/test/tools/yaml2obj/output-file.yaml

6

The test name is "invalid output file", but the new test case checks the
valid usage of the "-o" option isn't?

Perhaps you can rename the test file and keep the new test here.
Also it probably deserves a comment about what is tested.

18

Misaligned.

[MaskRay]

Address comments

[MaskRay]

In D73982#1858729, @grimar wrote:

With this patch applied I observe the different behavior between --help-list and -help-list:

umb@ubuntu:~/LLVM/LLVM/llvm-project/build/bin$ ./yaml2obj -help-list
OVERVIEW: Create an object file from YAML description
USAGE: yaml2obj [options] <input file>

OPTIONS:

Generic Options:

  --help          - Display available options (--help-hidden for more)
  --help-list     - Display list of available options (--help-list-hidden for more)
  --version       - Display the version of this program

yaml2obj Options:

  --docnum=<uint> - Read specified document from input (default = 1)
  -o=<filename>   - Output filename

umb@ubuntu:~/LLVM/LLVM/llvm-project/build/bin$ ./yaml2obj --help-list
OVERVIEW: Create an object file from YAML description
USAGE: yaml2obj [options] <input file>

OPTIONS:
  --docnum=<uint> - Read specified document from input (default = 1)
  --help          - Display available options (--help-hidden for more)
  -o=<filename>   - Output filename
  --version       - Display the version of this program

The original behavior was:

umb@ubuntu:~/LLVM/LLVM/llvm-project/build/bin$ ./yaml2obj --help-list
USAGE: yaml2obj [options] <input>

OPTIONS:
  --color         - Use colors in output (default=autodetect)
  --docnum=<uint> - Read specified document from input (default = 1)
  --help          - Display available options (--help-hidden for more)
  -o=<filename>   - Output filename
  --version       - Display the version of this program
umb@ubuntu:~/LLVM/LLVM/llvm-project/build/bin$ ./yaml2obj -help-list
USAGE: yaml2obj [options] <input>

OPTIONS:
  --color         - Use colors in output (default=autodetect)
  --docnum=<uint> - Read specified document from input (default = 1)
  --help          - Display available options (--help-hidden for more)
  -o=<filename>   - Output filename
  --version       - Display the version of this program

--help-list is -h -e -l -p ... now. ParseCommandLineOptions prints help information after parsing -h and quits, so -e is not rejected.

This change is to make cl::Prefix short options more reliable (cannot conflict with long option names). (We only have one yaml2obj specific long option, so there should not be any problem..)

llvm/test/tools/yaml2obj/output-file.yaml

6

It was already renamed :)

[merge_guards_bot]

Unit tests: unknown.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[jhenderson]

llvm/test/tools/yaml2obj/help.test

6

Delete --check-prefixes=CHECK,LIST

llvm/test/tools/yaml2obj/output-file.yaml

13

Actually, "No such file or directory" is only OS-dependent via the capitalization of the first letter. We have lots of other tests that check the message, using a regex for that first letter.

llvm/tools/yaml2obj/yaml2obj.cpp

31–32

I thought using static was more in keeping with LLVM-style?

51

from a YAML

[MaskRay]

Address review comments

llvm/tools/yaml2obj/yaml2obj.cpp

31–32

LLVM style prefers static.

However, namespaces are used in other places, e.g. llvm-nm. llvm-readobj interestingly uses a named namespace.

static makes the code a bit more verbose but I can change you are strong about this.

[merge_guards_bot]

Unit tests: fail. 62501 tests passed, 8 failed and 844 were skipped.

failed: LLVM.tools/yaml2obj/help.test
failed: lld.ELF/compressed-input-alignment.test
failed: lld.ELF/invalid/bad-reloc-target.test
failed: lld.ELF/invalid/common-symbol-alignment.test
failed: lld.ELF/invalid/dynamic-section-broken.test
failed: lld.ELF/invalid/symtab-sh-info.s
failed: lld.ELF/mips-elf-flags-err.test
failed: lld.ELF/mips-fp-flags-err.test

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[MaskRay]

Rebase to re-trigger @merge_guards_bot

[jhenderson]

LGTM, once @grimar is happy.

llvm/tools/yaml2obj/yaml2obj.cpp

31–32

Frankly, I prefer namespaces. I don't know why LLVM style is for static.

[grimar]

LGTM

[MaskRay]

Fix a test

llvm/test/tools/yaml2obj/help.test

+## Show that help text is printed correctly when requested.
+
+# RUN: yaml2obj -h | FileCheck %s --check-prefixes=CHECK,CATEG --implicit-check-not=Options:
+# RUN: yaml2obj --help | FileCheck %s --check-prefixes=CHECK,CATEG --implicit-check-not=Options:
+# RUN: yaml2obj --help-list | FileCheck %s --implicit-check-not=Options:
+
+# CHECK: OVERVIEW: Create an object file from YAML description
+# CHECK: USAGE: yaml2obj{{(.exe)?}} [options] <input file>{{$}}
+# CHECK: OPTIONS:
+# CATEG: Generic Options:
+# CATEG: yaml2obj Options:

llvm/test/tools/yaml2obj/invalid-output-file.yaml

-# RUN: not yaml2obj -o %p/path/does/not/exist 2>&1 | FileCheck %s
-
-## Don't check the OS-dependent message "No such file or directory".
-# CHECK: yaml2obj: error: failed to open '{{.*}}/path/does/not/exist': {{.*}}

llvm/test/tools/yaml2obj/output-file.yaml

+## Test that -o sets the output file name.
+
+# RUN: rm -f %t
+# RUN: yaml2obj %s -o %t
+# RUN: ls %t
+# RUN: rm -f %t
+# RUN: yaml2obj %s -o%t
+# RUN: ls %t
+
+# RUN: not yaml2obj -o %p/path/does/not/exist 2>&1 | FileCheck %s
+
+# CHECK: yaml2obj: error: failed to open '{{.*}}/path/does/not/exist': {{[Nn]}}o such file or directory
+
+!ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_NONE

llvm/tools/yaml2obj/yaml2obj.cpp

 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
 #include <system_error>
 
 using namespace llvm;
 
-static cl::opt<std::string>
-  Input(cl::Positional, cl::desc("<input>"), cl::init("-"));
+namespace {
+cl::OptionCategory Cat("yaml2obj Options");
 
-static cl::opt<unsigned>
+cl::opt<std::string> Input(cl::Positional, cl::desc("<input file>"),
+                           cl::init("-"), cl::cat(Cat));
+
+cl::opt<unsigned>
     DocNum("docnum", cl::init(1),
-           cl::desc("Read specified document from input (default = 1)"));
+           cl::desc("Read specified document from input (default = 1)"),
+           cl::cat(Cat));
 
-static cl::opt<std::string> OutputFilename("o", cl::desc("Output filename"),
-                                           cl::value_desc("filename"));
+cl::opt<std::string> OutputFilename("o", cl::desc("Output filename"),
+                                    cl::value_desc("filename"), cl::init("-"),
+                                    cl::Prefix, cl::cat(Cat));
+} // namespace
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
-  cl::ParseCommandLineOptions(argc, argv);
-
-  if (OutputFilename.empty())
-    OutputFilename = "-";
+  cl::HideUnrelatedOptions(Cat);
+  cl::ParseCommandLineOptions(
+      argc, argv, "Create an object file from a YAML description", nullptr,
+      nullptr, /*LongOptionsUseDoubleDash=*/true);
 
   auto ErrHandler = [](const Twine &Msg) {
     WithColor::error(errs(), "yaml2obj") << Msg << "\n";
   };
 
   std::error_code EC;
   std::unique_ptr<ToolOutputFile> Out(
       new ToolOutputFile(OutputFilename, EC, sys::fs::OF_None));