[clang-tools-extra] d47ee52 - [clang-tooling] Prevent llvm::fatal_error on invalid CLI option
via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 29 01:21:18 PST 2021
Author: serge-sans-paille
Date: 2021-01-29T10:15:06+01:00
New Revision: d47ee525f9e9289815db0864b03e866fc8e5ba01
URL: https://github.com/llvm/llvm-project/commit/d47ee525f9e9289815db0864b03e866fc8e5ba01
DIFF: https://github.com/llvm/llvm-project/commit/d47ee525f9e9289815db0864b03e866fc8e5ba01.diff
LOG: [clang-tooling] Prevent llvm::fatal_error on invalid CLI option
Fail gracefully instead. Prevent further misuse by enforcing the factory builder
instead of the constructor.
Differential Revision: https://reviews.llvm.org/D94420
Added:
Modified:
clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp
clang-tools-extra/clang-move/tool/ClangMove.cpp
clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
clang-tools-extra/test/clang-query/invalid-command-line.cpp
clang-tools-extra/test/clang-tidy/infrastructure/invalid-command-line.cpp
clang/docs/LibASTMatchersTutorial.rst
clang/include/clang/Tooling/CommonOptionsParser.h
clang/lib/Tooling/CommonOptionsParser.cpp
clang/tools/clang-check/ClangCheck.cpp
clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
clang/tools/clang-refactor/ClangRefactor.cpp
clang/tools/clang-rename/ClangRename.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
index babc207cb0e0..5f30cbf8fb47 100644
--- a/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
+++ b/clang-tools-extra/clang-change-namespace/tool/ClangChangeNamespace.cpp
@@ -99,8 +99,13 @@ llvm::ErrorOr<std::vector<std::string>> GetAllowedSymbolPatterns() {
int main(int argc, const char **argv) {
llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
- tooling::CommonOptionsParser OptionsParser(argc, argv,
- ChangeNamespaceCategory);
+ auto ExpectedParser =
+ tooling::CommonOptionsParser::create(argc, argv, ChangeNamespaceCategory);
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ tooling::CommonOptionsParser &OptionsParser = ExpectedParser.get();
const auto &Files = OptionsParser.getSourcePathList();
tooling::RefactoringTool Tool(OptionsParser.getCompilations(), Files);
llvm::ErrorOr<std::vector<std::string>> AllowedPatterns =
diff --git a/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp b/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
index 8508721bb85a..b2d0efecc206 100644
--- a/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
+++ b/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
@@ -128,7 +128,14 @@ bool Merge(llvm::StringRef MergeDir, llvm::StringRef OutputFile) {
} // namespace find_all_symbols
int main(int argc, const char **argv) {
- CommonOptionsParser OptionsParser(argc, argv, FindAllSymbolsCategory);
+ auto ExpectedParser =
+ CommonOptionsParser::create(argc, argv, FindAllSymbolsCategory);
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+
+ CommonOptionsParser &OptionsParser = ExpectedParser.get();
ClangTool Tool(OptionsParser.getCompilations(),
OptionsParser.getSourcePathList());
diff --git a/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp
index 50a0c49ba647..3a11a22def19 100644
--- a/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp
+++ b/clang-tools-extra/clang-include-fixer/tool/ClangIncludeFixer.cpp
@@ -263,7 +263,13 @@ void writeToJson(llvm::raw_ostream &OS, const IncludeFixerContext& Context) {
}
int includeFixerMain(int argc, const char **argv) {
- tooling::CommonOptionsParser options(argc, argv, IncludeFixerCategory);
+ auto ExpectedParser =
+ tooling::CommonOptionsParser::create(argc, argv, IncludeFixerCategory);
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ tooling::CommonOptionsParser &options = ExpectedParser.get();
tooling::ClangTool tool(options.getCompilations(),
options.getSourcePathList());
diff --git a/clang-tools-extra/clang-move/tool/ClangMove.cpp b/clang-tools-extra/clang-move/tool/ClangMove.cpp
index 7e16dc2b3b94..d4bd9a36517d 100644
--- a/clang-tools-extra/clang-move/tool/ClangMove.cpp
+++ b/clang-tools-extra/clang-move/tool/ClangMove.cpp
@@ -95,7 +95,13 @@ cl::opt<bool> DumpDecls(
int main(int argc, const char **argv) {
llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
- tooling::CommonOptionsParser OptionsParser(argc, argv, ClangMoveCategory);
+ auto ExpectedParser =
+ tooling::CommonOptionsParser::create(argc, argv, ClangMoveCategory);
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ tooling::CommonOptionsParser &OptionsParser = ExpectedParser.get();
if (OldDependOnNew && NewDependOnOld) {
llvm::errs() << "Provide either --old_depend_on_new or "
diff --git a/clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp b/clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
index 5150dc4bc557..375306765a52 100644
--- a/clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
+++ b/clang-tools-extra/clang-reorder-fields/tool/ClangReorderFields.cpp
@@ -50,8 +50,14 @@ static cl::opt<bool> Inplace("i", cl::desc("Overwrite edited files."),
const char Usage[] = "A tool to reorder fields in C/C++ structs/classes.\n";
int main(int argc, const char **argv) {
- tooling::CommonOptionsParser OP(argc, argv, ClangReorderFieldsCategory,
- Usage);
+ auto ExpectedParser = tooling::CommonOptionsParser::create(
+ argc, argv, ClangReorderFieldsCategory, cl::OneOrMore, Usage);
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+
+ tooling::CommonOptionsParser &OP = ExpectedParser.get();
auto Files = OP.getSourcePathList();
tooling::RefactoringTool Tool(OP.getCompilations(), Files);
diff --git a/clang-tools-extra/test/clang-query/invalid-command-line.cpp b/clang-tools-extra/test/clang-query/invalid-command-line.cpp
index 901aad8c1f23..e3e8af1d5e7a 100644
--- a/clang-tools-extra/test/clang-query/invalid-command-line.cpp
+++ b/clang-tools-extra/test/clang-query/invalid-command-line.cpp
@@ -1,4 +1,4 @@
// RUN: not clang-query --invalid-arg 2>&1 | FileCheck %s
-// CHECK: error: [CommonOptionsParser]: clang-query{{(\.exe)?}}: Unknown command line argument '--invalid-arg'. Try: 'clang-query{{(\.exe)?}} --help'
+// CHECK: error: clang-query{{(\.exe)?}}: Unknown command line argument '--invalid-arg'. Try: 'clang-query{{(\.exe)?}} --help'
// CHECK-NEXT: clang-query{{(\.exe)?}}: Did you mean '--extra-arg'?
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/invalid-command-line.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/invalid-command-line.cpp
index be84a0881895..c06b09d90004 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/invalid-command-line.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/invalid-command-line.cpp
@@ -1,4 +1,4 @@
// RUN: not clang-tidy --invalid-arg 2>&1 | FileCheck %s
-// CHECK: error: [CommonOptionsParser]: clang-tidy{{(\.exe)?}}: Unknown command line argument '--invalid-arg'. Try: 'clang-tidy{{(\.exe)?}} --help'
+// CHECK: error: clang-tidy{{(\.exe)?}}: Unknown command line argument '--invalid-arg'. Try: 'clang-tidy{{(\.exe)?}} --help'
// CHECK-NEXT: clang-tidy{{(\.exe)?}}: Did you mean '--extra-arg'?
diff --git a/clang/docs/LibASTMatchersTutorial.rst b/clang/docs/LibASTMatchersTutorial.rst
index 22285c5f0fa9..f70173e9f83c 100644
--- a/clang/docs/LibASTMatchersTutorial.rst
+++ b/clang/docs/LibASTMatchersTutorial.rst
@@ -141,7 +141,13 @@ documentation <LibTooling.html>`_.
static cl::extrahelp MoreHelp("\nMore help text...\n");
int main(int argc, const char **argv) {
- CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);
+ auto ExpectedParser = CommonOptionsParser::create(argc, argv, MyToolCategory);
+ if (!ExpectedParser) {
+ // Fail gracefully for unsupported options.
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ CommonOptionsParser& OptionsParser = ExpectedParser.get();
ClangTool Tool(OptionsParser.getCompilations(),
OptionsParser.getSourcePathList());
return Tool.run(newFrontendActionFactory<clang::SyntaxOnlyAction>().get());
@@ -282,7 +288,13 @@ And change ``main()`` to:
.. code-block:: c++
int main(int argc, const char **argv) {
- CommonOptionsParser OptionsParser(argc, argv, MyToolCategory);
+ auto ExpectedParser = CommonOptionsParser::create(argc, argv, MyToolCategory);
+ if (!ExpectedParser) {
+ // Fail gracefully for unsupported options.
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ CommonOptionsParser& OptionsParser = ExpectedParser.get();
ClangTool Tool(OptionsParser.getCompilations(),
OptionsParser.getSourcePathList());
diff --git a/clang/include/clang/Tooling/CommonOptionsParser.h b/clang/include/clang/Tooling/CommonOptionsParser.h
index a5bfeeeaf77f..0f072c2886ab 100644
--- a/clang/include/clang/Tooling/CommonOptionsParser.h
+++ b/clang/include/clang/Tooling/CommonOptionsParser.h
@@ -63,21 +63,8 @@ namespace tooling {
/// }
/// \endcode
class CommonOptionsParser {
-public:
- /// Parses command-line, initializes a compilation database.
- ///
- /// This constructor can change argc and argv contents, e.g. consume
- /// command-line options used for creating FixedCompilationDatabase.
- ///
- /// All options not belonging to \p Category become hidden.
- ///
- /// This constructor exits program in case of error.
- CommonOptionsParser(int &argc, const char **argv,
- llvm::cl::OptionCategory &Category,
- const char *Overview = nullptr)
- : CommonOptionsParser(argc, argv, Category, llvm::cl::OneOrMore,
- Overview) {}
+protected:
/// Parses command-line, initializes a compilation database.
///
/// This constructor can change argc and argv contents, e.g. consume
@@ -86,16 +73,17 @@ class CommonOptionsParser {
/// All options not belonging to \p Category become hidden.
///
/// It also allows calls to set the required number of positional parameters.
- CommonOptionsParser(int &argc, const char **argv,
- llvm::cl::OptionCategory &Category,
- llvm::cl::NumOccurrencesFlag OccurrencesFlag,
- const char *Overview = nullptr);
+ CommonOptionsParser(
+ int &argc, const char **argv, llvm::cl::OptionCategory &Category,
+ llvm::cl::NumOccurrencesFlag OccurrencesFlag = llvm::cl::OneOrMore,
+ const char *Overview = nullptr);
+public:
/// A factory method that is similar to the above constructor, except
/// this returns an error instead exiting the program on error.
static llvm::Expected<CommonOptionsParser>
create(int &argc, const char **argv, llvm::cl::OptionCategory &Category,
- llvm::cl::NumOccurrencesFlag OccurrencesFlag,
+ llvm::cl::NumOccurrencesFlag OccurrencesFlag = llvm::cl::OneOrMore,
const char *Overview = nullptr);
/// Returns a reference to the loaded compilations database.
diff --git a/clang/lib/Tooling/CommonOptionsParser.cpp b/clang/lib/Tooling/CommonOptionsParser.cpp
index 5d881aab1e0d..6301544dbb28 100644
--- a/clang/lib/Tooling/CommonOptionsParser.cpp
+++ b/clang/lib/Tooling/CommonOptionsParser.cpp
@@ -115,8 +115,7 @@ llvm::Error CommonOptionsParser::init(
// Stop initializing if command-line option parsing failed.
if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
OS.flush();
- return llvm::make_error<llvm::StringError>("[CommonOptionsParser]: " +
- ErrorMessage,
+ return llvm::make_error<llvm::StringError>(ErrorMessage,
llvm::inconvertibleErrorCode());
}
diff --git a/clang/tools/clang-check/ClangCheck.cpp b/clang/tools/clang-check/ClangCheck.cpp
index 6565aa229030..a04936478eb3 100644
--- a/clang/tools/clang-check/ClangCheck.cpp
+++ b/clang/tools/clang-check/ClangCheck.cpp
@@ -160,7 +160,13 @@ int main(int argc, const char **argv) {
llvm::InitializeAllAsmPrinters();
llvm::InitializeAllAsmParsers();
- CommonOptionsParser OptionsParser(argc, argv, ClangCheckCategory);
+ auto ExpectedParser =
+ CommonOptionsParser::create(argc, argv, ClangCheckCategory);
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ CommonOptionsParser &OptionsParser = ExpectedParser.get();
ClangTool Tool(OptionsParser.getCompilations(),
OptionsParser.getSourcePathList());
diff --git a/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp b/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
index b91bf798aa7f..8aba1301ef9a 100644
--- a/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ b/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -119,8 +119,13 @@ int main(int argc, const char **argv) {
const char *Overview = "\nThis tool collects the USR name and location "
"of external definitions in the source files "
"(excluding headers).\n";
- CommonOptionsParser OptionsParser(argc, argv, ClangExtDefMapGenCategory,
- cl::ZeroOrMore, Overview);
+ auto ExpectedParser = CommonOptionsParser::create(
+ argc, argv, ClangExtDefMapGenCategory, cl::ZeroOrMore, Overview);
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ CommonOptionsParser &OptionsParser = ExpectedParser.get();
ClangTool Tool(OptionsParser.getCompilations(),
OptionsParser.getSourcePathList());
diff --git a/clang/tools/clang-refactor/ClangRefactor.cpp b/clang/tools/clang-refactor/ClangRefactor.cpp
index 8b44c7fa6ede..0c82fd78c782 100644
--- a/clang/tools/clang-refactor/ClangRefactor.cpp
+++ b/clang/tools/clang-refactor/ClangRefactor.cpp
@@ -612,9 +612,14 @@ int main(int argc, const char **argv) {
ClangRefactorTool RefactorTool;
- CommonOptionsParser Options(
+ auto ExpectedParser = CommonOptionsParser::create(
argc, argv, cl::GeneralCategory, cl::ZeroOrMore,
"Clang-based refactoring tool for C, C++ and Objective-C");
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ CommonOptionsParser &Options = ExpectedParser.get();
if (auto Err = RefactorTool.Init()) {
llvm::errs() << llvm::toString(std::move(Err)) << "\n";
diff --git a/clang/tools/clang-rename/ClangRename.cpp b/clang/tools/clang-rename/ClangRename.cpp
index 6dcd33aeb164..141ba379ed05 100644
--- a/clang/tools/clang-rename/ClangRename.cpp
+++ b/clang/tools/clang-rename/ClangRename.cpp
@@ -98,7 +98,13 @@ static cl::opt<bool> Force("force",
cl::cat(ClangRenameOptions));
int main(int argc, const char **argv) {
- tooling::CommonOptionsParser OP(argc, argv, ClangRenameOptions);
+ auto ExpectedParser =
+ tooling::CommonOptionsParser::create(argc, argv, ClangRenameOptions);
+ if (!ExpectedParser) {
+ llvm::errs() << ExpectedParser.takeError();
+ return 1;
+ }
+ tooling::CommonOptionsParser &OP = ExpectedParser.get();
if (!Input.empty()) {
// Populate QualifiedNames and NewNames from a YAML file.
More information about the cfe-commits
mailing list