[PATCH] D39042: [Tooling] Add a factory method for CommonOptionsParser

[Haojian Wu via Phabricator via cfe-commits]

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

Looks good from my side. You might want @klimek to take a look before committing.



================
Comment at: include/clang/Tooling/CommonOptionsParser.h:90
-  ///
-  /// This constructor exits program in case of error.
   CommonOptionsParser(int &argc, const char **argv,
----------------
This change is unintended?


================
Comment at: include/clang/Tooling/CommonOptionsParser.h:118
+  llvm::Error init(int &argc, const char **argv,
+                          llvm::cl::OptionCategory &Category,
+                          llvm::cl::NumOccurrencesFlag OccurrencesFlag,
----------------
nit: the code indention seems wrong after updated.


================
Comment at: lib/Tooling/CommonOptionsParser.cpp:165
+                            const char *Overview) {
+  std::unique_ptr<CommonOptionsParser> Parser(new CommonOptionsParser);
+  llvm::Error Err =
----------------
ioeric wrote:
> hokein wrote:
> > nit: using llvm::make_unique.
> `make_unique` didn't work because the constructor here is private...
Ah, I see.


https://reviews.llvm.org/D39042