From c46064c2a9eea9dac5c51d038cfd8f72863e2652 Mon Sep 17 00:00:00 2001 From: Serge Pavlov Date: Wed, 24 May 2017 11:57:37 +0000 Subject: Method loadFromCommandLine should be able to report errors Now FixedCompilationDatabase::loadFromCommandLine has no means to report which error occurred if it fails to create compilation object. This is a block for implementing D33013, because after that change driver will refuse to create compilation if command line contains erroneous options. This change adds additional argument to loadFromCommandLine, which is assigned error message text if compilation object was not created. This is the same way as other methods of CompilationDatabase report failure. Differential Revision: https://reviews.llvm.org/D33272 llvm-svn: 303741 --- clang/include/clang/Tooling/CompilationDatabase.h | 7 ++-- .../Frontend/CreateInvocationFromCommandLine.cpp | 2 ++ clang/lib/Tooling/CommonOptionsParser.cpp | 7 ++-- clang/lib/Tooling/CompilationDatabase.cpp | 42 +++++++++++++--------- clang/lib/Tooling/Tooling.cpp | 2 ++ .../unittests/Tooling/CompilationDatabaseTest.cpp | 32 +++++++++++------ 6 files changed, 61 insertions(+), 31 deletions(-) (limited to 'clang') diff --git a/clang/include/clang/Tooling/CompilationDatabase.h b/clang/include/clang/Tooling/CompilationDatabase.h index 7f3f5869841..e988b84b6ea 100644 --- a/clang/include/clang/Tooling/CompilationDatabase.h +++ b/clang/include/clang/Tooling/CompilationDatabase.h @@ -176,10 +176,11 @@ public: /// the number of arguments before "--", if "--" was found in the argument /// list. /// \param Argv Points to the command line arguments. + /// \param ErrorMsg Contains error text if the function returns null pointer. /// \param Directory The base directory used in the FixedCompilationDatabase. - static FixedCompilationDatabase *loadFromCommandLine(int &Argc, - const char *const *Argv, - Twine Directory = "."); + static std::unique_ptr loadFromCommandLine( + int &Argc, const char *const *Argv, std::string &ErrorMsg, + Twine Directory = "."); /// \brief Constructs a compilation data base from a specified directory /// and command line. diff --git a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp index 16269064b6e..49d459e78c4 100644 --- a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp +++ b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp @@ -52,6 +52,8 @@ std::unique_ptr clang::createInvocationFromCommandLine( TheDriver.setCheckInputsExist(false); std::unique_ptr C(TheDriver.BuildCompilation(Args)); + if (!C) + return nullptr; // Just print the cc1 options if -### was present. if (C->getArgs().hasArg(driver::options::OPT__HASH_HASH_HASH)) { diff --git a/clang/lib/Tooling/CommonOptionsParser.cpp b/clang/lib/Tooling/CommonOptionsParser.cpp index 5a44061cbd4..9e9689e6b25 100644 --- a/clang/lib/Tooling/CommonOptionsParser.cpp +++ b/clang/lib/Tooling/CommonOptionsParser.cpp @@ -116,7 +116,11 @@ CommonOptionsParser::CommonOptionsParser( cl::HideUnrelatedOptions(Category); - Compilations.reset(FixedCompilationDatabase::loadFromCommandLine(argc, argv)); + std::string ErrorMessage; + Compilations = + FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage); + if (!Compilations && !ErrorMessage.empty()) + llvm::errs() << ErrorMessage; cl::ParseCommandLineOptions(argc, argv, Overview); cl::PrintOptionValues(); @@ -125,7 +129,6 @@ CommonOptionsParser::CommonOptionsParser( SourcePathList.empty()) return; if (!Compilations) { - std::string ErrorMessage; if (!BuildPath.empty()) { Compilations = CompilationDatabase::autoDetectFromDirectory(BuildPath, ErrorMessage); diff --git a/clang/lib/Tooling/CompilationDatabase.cpp b/clang/lib/Tooling/CompilationDatabase.cpp index 8ca0b2df701..77c5b547ca0 100644 --- a/clang/lib/Tooling/CompilationDatabase.cpp +++ b/clang/lib/Tooling/CompilationDatabase.cpp @@ -27,6 +27,7 @@ #include "llvm/Option/Arg.h" #include "llvm/Support/Host.h" #include "llvm/Support/Path.h" +#include "llvm/Support/raw_ostream.h" #include #include using namespace clang; @@ -150,23 +151,21 @@ private: // options. class UnusedInputDiagConsumer : public DiagnosticConsumer { public: - UnusedInputDiagConsumer() : Other(nullptr) {} - - // Useful for debugging, chain diagnostics to another consumer after - // recording for our own purposes. - UnusedInputDiagConsumer(DiagnosticConsumer *Other) : Other(Other) {} + UnusedInputDiagConsumer(DiagnosticConsumer &Other) : Other(Other) {} void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) override { if (Info.getID() == clang::diag::warn_drv_input_file_unused) { // Arg 1 for this diagnostic is the option that didn't get used. UnusedInputs.push_back(Info.getArgStdStr(0)); + } else if (DiagLevel >= DiagnosticsEngine::Error) { + // If driver failed to create compilation object, show the diagnostics + // to user. + Other.HandleDiagnostic(DiagLevel, Info); } - if (Other) - Other->HandleDiagnostic(DiagLevel, Info); } - DiagnosticConsumer *Other; + DiagnosticConsumer &Other; SmallVector UnusedInputs; }; @@ -205,9 +204,12 @@ private: /// \li false if \c Args cannot be used for compilation jobs (e.g. /// contains an option like -E or -version). static bool stripPositionalArgs(std::vector Args, - std::vector &Result) { + std::vector &Result, + std::string &ErrorMsg) { IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); - UnusedInputDiagConsumer DiagClient; + llvm::raw_string_ostream Output(ErrorMsg); + TextDiagnosticPrinter DiagnosticPrinter(Output, &*DiagOpts); + UnusedInputDiagConsumer DiagClient(DiagnosticPrinter); DiagnosticsEngine Diagnostics( IntrusiveRefCntPtr(new DiagnosticIDs()), &*DiagOpts, &DiagClient, false); @@ -245,6 +247,8 @@ static bool stripPositionalArgs(std::vector Args, const std::unique_ptr Compilation( NewDriver->BuildCompilation(Args)); + if (!Compilation) + return false; const driver::JobList &Jobs = Compilation->getJobs(); @@ -258,8 +262,7 @@ static bool stripPositionalArgs(std::vector Args, } if (CompileAnalyzer.Inputs.empty()) { - // No compile jobs found. - // FIXME: Emit a warning of some kind? + ErrorMsg = "warning: no compile jobs found\n"; return false; } @@ -280,8 +283,14 @@ static bool stripPositionalArgs(std::vector Args, return true; } -FixedCompilationDatabase *FixedCompilationDatabase::loadFromCommandLine( - int &Argc, const char *const *Argv, Twine Directory) { +std::unique_ptr +FixedCompilationDatabase::loadFromCommandLine(int &Argc, + const char *const *Argv, + std::string &ErrorMsg, + Twine Directory) { + ErrorMsg.clear(); + if (Argc == 0) + return nullptr; const char *const *DoubleDash = std::find(Argv, Argv + Argc, StringRef("--")); if (DoubleDash == Argv + Argc) return nullptr; @@ -289,9 +298,10 @@ FixedCompilationDatabase *FixedCompilationDatabase::loadFromCommandLine( Argc = DoubleDash - Argv; std::vector StrippedArgs; - if (!stripPositionalArgs(CommandLine, StrippedArgs)) + if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg)) return nullptr; - return new FixedCompilationDatabase(Directory, StrippedArgs); + return std::unique_ptr( + new FixedCompilationDatabase(Directory, StrippedArgs)); } FixedCompilationDatabase:: diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 9e1181281f1..2e093dd9afc 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -260,6 +260,8 @@ bool ToolInvocation::run() { Driver->setCheckInputsExist(false); const std::unique_ptr Compilation( Driver->BuildCompilation(llvm::makeArrayRef(Argv))); + if (!Compilation) + return false; const llvm::opt::ArgStringList *const CC1Args = getCC1Arguments( &Diagnostics, Compilation.get()); if (!CC1Args) { diff --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp b/clang/unittests/Tooling/CompilationDatabaseTest.cpp index 1a6fffec939..5a6693eb4db 100644 --- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp +++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp @@ -504,18 +504,22 @@ TEST(FixedCompilationDatabase, GetAllCompileCommands) { TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) { int Argc = 0; - std::unique_ptr Database( - FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr)); + std::string ErrorMsg; + std::unique_ptr Database = + FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr, ErrorMsg); EXPECT_FALSE(Database); + EXPECT_TRUE(ErrorMsg.empty()); EXPECT_EQ(0, Argc); } TEST(ParseFixedCompilationDatabase, ReturnsNullWithoutDoubleDash) { int Argc = 2; const char *Argv[] = { "1", "2" }; + std::string ErrorMsg; std::unique_ptr Database( - FixedCompilationDatabase::loadFromCommandLine(Argc, Argv)); + FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg)); EXPECT_FALSE(Database); + EXPECT_TRUE(ErrorMsg.empty()); EXPECT_EQ(2, Argc); } @@ -524,9 +528,11 @@ TEST(ParseFixedCompilationDatabase, ReturnsArgumentsAfterDoubleDash) { const char *Argv[] = { "1", "2", "--\0no-constant-folding", "-DDEF3", "-DDEF4" }; + std::string ErrorMsg; std::unique_ptr Database( - FixedCompilationDatabase::loadFromCommandLine(Argc, Argv)); + FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg)); ASSERT_TRUE((bool)Database); + ASSERT_TRUE(ErrorMsg.empty()); std::vector Result = Database->getCompileCommands("source"); ASSERT_EQ(1ul, Result.size()); @@ -543,9 +549,11 @@ TEST(ParseFixedCompilationDatabase, ReturnsArgumentsAfterDoubleDash) { TEST(ParseFixedCompilationDatabase, ReturnsEmptyCommandLine) { int Argc = 3; const char *Argv[] = { "1", "2", "--\0no-constant-folding" }; - std::unique_ptr Database( - FixedCompilationDatabase::loadFromCommandLine(Argc, Argv)); + std::string ErrorMsg; + std::unique_ptr Database = + FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg); ASSERT_TRUE((bool)Database); + ASSERT_TRUE(ErrorMsg.empty()); std::vector Result = Database->getCompileCommands("source"); ASSERT_EQ(1ul, Result.size()); @@ -560,9 +568,11 @@ TEST(ParseFixedCompilationDatabase, ReturnsEmptyCommandLine) { TEST(ParseFixedCompilationDatabase, HandlesPositionalArgs) { const char *Argv[] = {"1", "2", "--", "-c", "somefile.cpp", "-DDEF3"}; int Argc = sizeof(Argv) / sizeof(char*); - std::unique_ptr Database( - FixedCompilationDatabase::loadFromCommandLine(Argc, Argv)); + std::string ErrorMsg; + std::unique_ptr Database = + FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg); ASSERT_TRUE((bool)Database); + ASSERT_TRUE(ErrorMsg.empty()); std::vector Result = Database->getCompileCommands("source"); ASSERT_EQ(1ul, Result.size()); @@ -579,9 +589,11 @@ TEST(ParseFixedCompilationDatabase, HandlesPositionalArgs) { TEST(ParseFixedCompilationDatabase, HandlesArgv0) { const char *Argv[] = {"1", "2", "--", "mytool", "somefile.cpp"}; int Argc = sizeof(Argv) / sizeof(char*); - std::unique_ptr Database( - FixedCompilationDatabase::loadFromCommandLine(Argc, Argv)); + std::string ErrorMsg; + std::unique_ptr Database = + FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg); ASSERT_TRUE((bool)Database); + ASSERT_TRUE(ErrorMsg.empty()); std::vector Result = Database->getCompileCommands("source"); ASSERT_EQ(1ul, Result.size()); -- cgit v1.2.1