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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
