[PATCH] D109977: LLVM Driver Multicall tool

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

Another potential future improvement is error reporting for subcommands:

  $ ./bin/llvm clang       
  llvm: error: no input files
  $ ./bin/clang    
  clang-15: error: no input files

Ideally, the multicall tool would produce the same error message.



================
Comment at: llvm/cmake/modules/AddLLVM.cmake:897
+    set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_TOOLS ${name})
+    target_link_libraries(${obj_name} PUBLIC ${LLVM_PTHREAD_LIB})
+    llvm_config(${obj_name} ${USE_SHARED} ${LLVM_LINK_COMPONENTS} )
----------------
The error that @MaskRay reported is because we use `PUBLIC` with `target_link_libraries()` here, but we use plain form of `target_link_libraries()` later in  `explicit_llvm_config` that is invoked `llvm_config` (see https://github.com/llvm/llvm-project/blob/1643f01232b41b93e5f3a21d89b111efab0e378a/llvm/cmake/modules/LLVM-Config.cmake#L110). Omitting `PUBLIC` here appears to be sufficient to avoid the error.


================
Comment at: llvm/tools/CMakeLists.txt:59
+# scrape up tools from other projects into itself.
+add_subdirectory(llvm-driver)
----------------
Should this be guarded by `LLVM_TOOL_LLVM_DRIVER_BUILD` to behave as the commit message says? Currently the `llvm-driver` appears to be built regardless of `LLVM_TOOL_LLVM_DRIVER_BUILD`.


================
Comment at: llvm/tools/llvm-driver/llvm-driver.cpp:43
+  if (LaunchedTool == "llvm") {
+    LaunchedTool = Argv[1];
+    ConsumeFirstArg = true;
----------------
When the tool is invoked without any arguments (that is, simply as `./bin/llvm`) this will lead to out-of-bounds array access. We should handle this case explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109977