[PATCH] D109977: LLVM Driver Multicall tool

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> llvm-driver can be disabled from builds by setting LLVM_TOOL_LLVM_DRIVER_BUILD=Off.

I think this should be opt-in. The new `llvm` executable takes a lot of space and not needed by many developers/build bots.
It's useful to some groups (distributions) but they can specify the option themselves.
I think the modified code is quite stable, so don't worry about regressions just because this is not opt-in.



================
Comment at: llvm/tools/llvm-driver/llvm-driver.cpp:24
+// --help or --version are passed directly to the llvm driver.
+int UnknownMain(int Argc, char **Argv) {
+  cl::OptionCategory LLVMDriverCategory("llvm options");
----------------
`static int unknownMain`



================
Comment at: llvm/tools/llvm-driver/llvm-driver.cpp:25
+int UnknownMain(int Argc, char **Argv) {
+  cl::OptionCategory LLVMDriverCategory("llvm options");
+#define LLVM_DRIVER_TOOL(tool, entry, key)                                     \
----------------
Prefer OptTable to cl:: for user-facing utilities. If simple, just open coding the help message.


================
Comment at: llvm/tools/llvm-driver/llvm-driver.cpp:37
+
+  llvm_unreachable("We should never get here, parsing should always exit.");
+  return 1;
----------------
https://llvm.org/docs/CodingStandards.html#error-and-warning-messages


================
Comment at: llvm/tools/llvm-driver/llvm-driver.cpp:50
+  bool ConsumeFirstArg = false;
+  if (LaunchedTool == "llvm") {
+    if (Argc < 2)
----------------
Some distributions may want to use something like llvm-15. See some binary utilities how the version is handled.


================
Comment at: llvm/tools/llvm-driver/llvm-driver.cpp:58
+  // if it is launched through a symlink that is the tool name.
+  typedef int (*MainFunction)(int, char **);
+  MainFunction Func = StringSwitch<MainFunction>(LaunchedTool)
----------------
If only used once, don't add a typedef (using is preferred)


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

https://reviews.llvm.org/D109977