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

2017-10-24 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL316433: [Tooling] Add a factory method for CommonOptionsParser (authored by ioeric). Repository: rL LLVM https://reviews.llvm.org/D39042 Files: cfe/trunk/include/clang/Tooling/CommonOptionsParser.h

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

2017-10-19 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. Still LGTM. https://reviews.llvm.org/D39042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-10-19 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 119583. ioeric added a comment. - Make the factory method return an expected object instead of unique_ptr. - clang-format code. https://reviews.llvm.org/D39042 Files: include/clang/Tooling/CommonOptionsParser.h lib/Tooling/CommonOptionsParser.cpp

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

2017-10-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 119502. ioeric added a comment. - Move unique_ptr result. https://reviews.llvm.org/D39042 Files: include/clang/Tooling/CommonOptionsParser.h lib/Tooling/CommonOptionsParser.cpp Index: lib/Tooling/CommonOptionsParser.cpp

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

2017-10-18 Thread 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

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

2017-10-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 119465. ioeric marked 2 inline comments as done. ioeric added a comment. - Address review comments. https://reviews.llvm.org/D39042 Files: include/clang/Tooling/CommonOptionsParser.h lib/Tooling/CommonOptionsParser.cpp Index:

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

2017-10-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments. Comment at: include/clang/Tooling/CommonOptionsParser.h:115 + + static llvm::Error init(int , const char **argv, + llvm::cl::OptionCategory , hokein wrote: > We can get rid of the `static` here by calling

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

2017-10-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: include/clang/Tooling/CommonOptionsParser.h:95 + static llvm::Expected + create(int , const char **argv, llvm::cl::OptionCategory , worth some documentation? Comment at:

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

2017-10-18 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision. This returns error instead of exiting the program in case of error. https://reviews.llvm.org/D39042 Files: include/clang/Tooling/CommonOptionsParser.h lib/Tooling/CommonOptionsParser.cpp Index: lib/Tooling/CommonOptionsParser.cpp