[PATCH] D100583: [llvm-objdump] Add an llvm-otool tool

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Mostly looks good, with only few nits, but I hope some Mach-O users can review the functional part.

I'll also warn that MachODump.cpp may lack maintenance... so parties investing in it may need to put in some maintenance energy.



================
Comment at: llvm/test/tools/llvm-objdump/MachO/relocations.test:16
+NONVERBOSE: address  pcrel length extern type    scattered symbolnum/value
+NONVERBOSE: 00000027 1     2      1      2       0         4
+NONVERBOSE: 0000000b 1     2      1      1       0         0
----------------
Add `-NEXT:` whenever applicable.



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:104
+    // TODO Replace this with OptTable API once it adds extrahelp support.
+    outs() << "\nPass @FILE as argument to read options from FILE.\n";
+  }
----------------
I know the messages are not super consistent in the tool, but https://llvm.org/docs/CodingStandards.html#error-and-warning-messages Usually we don't have a period and don't capitalize.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2874
+  if (InputArgs.hasArg(OTOOL_x)) {
+    FilterSections.push_back(",__text");
+  }
----------------
Delete braces around one-line simple statement.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2995
   ToolName = argv[0];
+  std::unique_ptr<CommonOptTable> T;
+  OptSpecifier Unknown, HelpFlag, HelpHiddenFlag, VersionFlag;
----------------
Use `Optional<CommonOptTable>` and emplace.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:3045
     cl::PrintVersionMessage();
-    outs() << '\n';
-    TargetRegistry::printRegisteredTargetsForVersion(outs());
-    exit(0);
+    if (!Is("otool")) {
+      outs() << '\n';
----------------
Oh, otool doesn't like the ` TargetRegistry::printRegisteredTargetsForVersion` message?


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

https://reviews.llvm.org/D100583