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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu 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"))
----------------
stella.stamenova wrote:
> Why not use ToolName? Or remove ToolName?
No particular reason. Reverted.


================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:81
+int main(int Argc, const char **Argv) {
+  InitLLVM X(Argc, Argv);
 
----------------
stella.stamenova wrote:
> 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?
> 
I don't want to do that at least in this patch as I want to keep this change as mechanical as possible.


https://reviews.llvm.org/D45602