[PATCH] D45602: Define InitLLVM to do common initialization all at once.

Stella Stamenova via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 12 20:01:46 PDT 2018


stella.stamenova added inline comments.


================
Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:957
 
-  Stem = sys::path::stem(ToolName);
+  Stem = sys::path::stem(argv[0]);
   if (Stem.contains_lower("dlltool"))
----------------
Why not use ToolName? Or remove ToolName?


================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:81
+int main(int Argc, const char **Argv) {
+  InitLLVM X(Argc, Argv);
 
----------------
ruiu wrote:
> stella.stamenova wrote:
> > By doing this in this file, ExitOnErr (which is used later in the main function) no longer has the banner set. I suspect the same may be true in some of the other files too. 
> > 
> > Perhaps InitLLVM could return an ExitOnError object which is already initialized with the correct banner?
> Looks like ExitOnError can take a banner as its constructor argument.
I noticed that several of the files that you changed set the banner of ExitOnError after InitLLVM to argv[0] (which is also what InitLLVM does), while a couple set just the name of the tool. Could you change the ones that set just the name of the tool to also use argv[0] for consistency?



https://reviews.llvm.org/D45602





More information about the llvm-commits mailing list