[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:42:30 PDT 2018


stella.stamenova accepted this revision.
stella.stamenova added a comment.
This revision is now accepted and ready to land.

The last comment would be nice to have, but not necessary. LGTM.



================
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:
> > 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.
Then in llvm-pdbutil.cpp and here, you should undo the change to the declaration of ExitOnError and bring back the setBanner statements in the main() functions.


https://reviews.llvm.org/D45602





More information about the llvm-commits mailing list