[PATCH] D35333: Create empty shell of llvm-mt.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: llvm/tools/llvm-mt/Opts.td:3
+
+def UNSUPPORTED : OptionGroup<"UNSUPPORTED">;
+def MANIFEST : Separate<["/", "-"], "MANIFEST">, HelpText<"Used to specify each manifest that need to be processed.">, MetaVarName<"manifest">;
----------------
By convention, option names are written in lowercase. So `unsupported` instead of `UNSUPPORTED`. Apply all other options in this file.


================
Comment at: llvm/tools/llvm-mt/Opts.td:6
+def IDENTITY : Joined<["/", "-"], "IDENTITY:">, HelpText<"Not supported.">, MetaVarName<"identity">, Group<UNSUPPORTED>;
+def RGS : Joined<["/", "-"], "RGS:">, HelpText<"Not supported.">, MetaVarName<"script">, Group<UNSUPPORTED>;
+def TLB : Joined<["/", "-"], "TLB:">, HelpText<"Not supported.">, MetaVarName<"file">, Group<UNSUPPORTED>;
----------------
Help text shouldn't end with a full stop.


================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:48
+               HELPTEXT, METAVAR, VALUES)                                      \
+  {                                                                            \
+      PREFIX,      NAME,      HELPTEXT,                                        \
----------------
This open parenthesis is not at the same indentation depth as the closing one.


================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:66
+LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg) {
+  errs() << "llvm-mt error: " << Msg;
+  exit(1);
----------------
It is convenient to write out "\n" here, instead of embedding "\n" as part of Msg.


================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:71
+
+int main(int argc_, const char *argv_[]) {
+  sys::PrintStackTraceOnErrorSignal(argv_[0]);
----------------
Please follow the LLVM naming convention.


================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:91
+  	if (Arg->getOption().matches(OPT_UNSUPPORTED)) {
+  		outs() << "Ignoring " << Arg->getOption().getName() << " option; not supported.\n";
+  	}
----------------
; -> :


================
Comment at: llvm/tools/llvm-mt/llvm-mt.cpp:117
+  return 0;
+
+}
----------------
Remove this blank line.


https://reviews.llvm.org/D35333