[PATCH] D109977: LLVM Driver Multicall tool

Alex Brachet via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jun 4 10:51:54 PDT 2022


abrachet marked 5 inline comments as done.
abrachet added a comment.

In D109977#3378312 <https://reviews.llvm.org/D109977#3378312>, @phosek wrote:

> 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.

It's difficult to make the error message the same, ie `clang-15`, but hopefully the name the tool was invoked with is enough.



================
Comment at: llvm/tools/llvm-driver/llvm-driver.cpp:43
+  if (LaunchedTool == "llvm") {
+    LaunchedTool = Argv[1];
+    ConsumeFirstArg = true;
----------------
phosek wrote:
> 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.
This will now print the help message


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:404
 
-int main(int argc, char **argv) {
+int llvm_objcopy_main(int argc, char **argv) {
   InitLLVM X(argc, argv);
----------------
beanz wrote:
> aganea wrote:
> > Shouldn't we say:
> > ```
> > int objcopy_main(int argc, char **argv) {
> > ```
> > here and the other places + all supporting code, if we want `llvm objcopy` (without the dash) like @phosek suggests?
> I had a different thought for that. I think we want the tools to respond to llvm-objcopy since we will want them to exist in parallel to binutils tools just like the current tools do today.
> 
> Many of the current tools also support symlink-redirection, to support that we'll need to have a multiplex where multiple tool names point to the same `main` function.
> 
> Handling that was my point (1) in the `main` commit message, and I intended to work on it in a follow-on commit.
> Shouldn't we say:
> ```
> int objcopy_main(int argc, char **argv) {
> ```
> here and the other places + all supporting code, if we want `llvm objcopy` (without the dash) like @phosek suggests?

I've kept name of the function as is, but `llvm objcopy` is now supported, and `llvm llvm-objcopy` is not


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

https://reviews.llvm.org/D109977



More information about the cfe-commits mailing list