sepavloff created this revision.

Now FixedCompilationDatabase::loadFromCommandLine has no means to report
which error occurred if it fails to create compilation object. This is
a block for implementing https://reviews.llvm.org/D33013, because after that 
change driver refuses
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.


https://reviews.llvm.org/D33272

Files:
  include/clang/Tooling/CompilationDatabase.h
  lib/Frontend/CreateInvocationFromCommandLine.cpp
  lib/Tooling/CommonOptionsParser.cpp
  lib/Tooling/CompilationDatabase.cpp
  lib/Tooling/Tooling.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===================================================================
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -504,29 +504,35 @@
 
 TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
   int Argc = 0;
+  std::string ErrorMsg;
   std::unique_ptr<FixedCompilationDatabase> Database(
-      FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr));
+      FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr, ErrorMsg));
   EXPECT_FALSE(Database);
+  EXPECT_EQ(ErrorMsg, "error: no arguments specified\n");
   EXPECT_EQ(0, Argc);
 }
 
 TEST(ParseFixedCompilationDatabase, ReturnsNullWithoutDoubleDash) {
   int Argc = 2;
   const char *Argv[] = { "1", "2" };
+  std::string ErrorMsg;
   std::unique_ptr<FixedCompilationDatabase> Database(
-      FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+      FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg));
   EXPECT_FALSE(Database);
+  EXPECT_EQ(ErrorMsg, "error: double dash not found\n");
   EXPECT_EQ(2, Argc);
 }
 
 TEST(ParseFixedCompilationDatabase, ReturnsArgumentsAfterDoubleDash) {
   int Argc = 5;
   const char *Argv[] = {
     "1", "2", "--\0no-constant-folding", "-DDEF3", "-DDEF4"
   };
+  std::string ErrorMsg;
   std::unique_ptr<FixedCompilationDatabase> Database(
-      FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+      FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg));
   ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
   std::vector<CompileCommand> Result =
     Database->getCompileCommands("source");
   ASSERT_EQ(1ul, Result.size());
@@ -543,9 +549,11 @@
 TEST(ParseFixedCompilationDatabase, ReturnsEmptyCommandLine) {
   int Argc = 3;
   const char *Argv[] = { "1", "2", "--\0no-constant-folding" };
+  std::string ErrorMsg;
   std::unique_ptr<FixedCompilationDatabase> Database(
       FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
   ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
   std::vector<CompileCommand> Result =
     Database->getCompileCommands("source");
   ASSERT_EQ(1ul, Result.size());
@@ -560,9 +568,11 @@
 TEST(ParseFixedCompilationDatabase, HandlesPositionalArgs) {
   const char *Argv[] = {"1", "2", "--", "-c", "somefile.cpp", "-DDEF3"};
   int Argc = sizeof(Argv) / sizeof(char*);
+  std::string ErrorMsg;
   std::unique_ptr<FixedCompilationDatabase> Database(
       FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
   ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
   std::vector<CompileCommand> Result =
     Database->getCompileCommands("source");
   ASSERT_EQ(1ul, Result.size());
@@ -579,9 +589,11 @@
 TEST(ParseFixedCompilationDatabase, HandlesArgv0) {
   const char *Argv[] = {"1", "2", "--", "mytool", "somefile.cpp"};
   int Argc = sizeof(Argv) / sizeof(char*);
+  std::string ErrorMsg;
   std::unique_ptr<FixedCompilationDatabase> Database(
       FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
   ASSERT_TRUE((bool)Database);
+  ASSERT_TRUE(ErrorMsg.empty());
   std::vector<CompileCommand> Result =
     Database->getCompileCommands("source");
   ASSERT_EQ(1ul, Result.size());
Index: lib/Tooling/Tooling.cpp
===================================================================
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -260,6 +260,8 @@
   Driver->setCheckInputsExist(false);
   const std::unique_ptr<clang::driver::Compilation> Compilation(
       Driver->BuildCompilation(llvm::makeArrayRef(Argv)));
+  if (!Compilation)
+    return false;
   const llvm::opt::ArgStringList *const CC1Args = getCC1Arguments(
       &Diagnostics, Compilation.get());
   if (!CC1Args) {
Index: lib/Tooling/CompilationDatabase.cpp
===================================================================
--- lib/Tooling/CompilationDatabase.cpp
+++ 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 <sstream>
 #include <system_error>
 using namespace clang;
@@ -150,23 +151,21 @@
 // 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));
-    }
-    if (Other)
-      Other->HandleDiagnostic(DiagLevel, Info);
+
+    // If driver failed to create compilation object, show the diagnostics
+    // to user.
+    } else if (DiagLevel >= DiagnosticsEngine::Error)
+      Other.HandleDiagnostic(DiagLevel, Info);
   }
 
-  DiagnosticConsumer *Other;
+  DiagnosticConsumer &Other;
   SmallVector<std::string, 2> UnusedInputs;
 };
 
@@ -205,9 +204,12 @@
 ///          \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<const char *> Args,
-                                std::vector<std::string> &Result) {
+                                std::vector<std::string> &Result,
+                                std::string &ErrorMsg) {
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
-  UnusedInputDiagConsumer DiagClient;
+  llvm::raw_string_ostream Output(ErrorMsg);
+  TextDiagnosticPrinter DiagnosticPrinter(Output, &*DiagOpts);
+  UnusedInputDiagConsumer DiagClient(DiagnosticPrinter);
   DiagnosticsEngine Diagnostics(
       IntrusiveRefCntPtr<clang::DiagnosticIDs>(new DiagnosticIDs()),
       &*DiagOpts, &DiagClient, false);
@@ -245,6 +247,8 @@
 
   const std::unique_ptr<driver::Compilation> Compilation(
       NewDriver->BuildCompilation(Args));
+  if (!Compilation)
+    return false;
 
   const driver::JobList &Jobs = Compilation->getJobs();
 
@@ -258,8 +262,7 @@
   }
 
   if (CompileAnalyzer.Inputs.empty()) {
-    // No compile jobs found.
-    // FIXME: Emit a warning of some kind?
+    ErrorMsg = "error: no compile jobs found\n";
     return false;
   }
 
@@ -281,15 +284,22 @@
 }
 
 FixedCompilationDatabase *FixedCompilationDatabase::loadFromCommandLine(
-    int &Argc, const char *const *Argv, Twine Directory) {
+    int &Argc, const char *const *Argv, std::string &ErrorMsg,
+    Twine Directory) {
+  if (Argc == 0) {
+    ErrorMsg = "error: no arguments specified\n";
+    return nullptr;
+  }
   const char *const *DoubleDash = std::find(Argv, Argv + Argc, StringRef("--"));
-  if (DoubleDash == Argv + Argc)
+  if (DoubleDash == Argv + Argc) {
+    ErrorMsg = "error: double dash not found\n";
     return nullptr;
+  }
   std::vector<const char *> CommandLine(DoubleDash + 1, Argv + Argc);
   Argc = DoubleDash - Argv;
 
   std::vector<std::string> StrippedArgs;
-  if (!stripPositionalArgs(CommandLine, StrippedArgs))
+  if (!stripPositionalArgs(CommandLine, StrippedArgs, ErrorMsg))
     return nullptr;
   return new FixedCompilationDatabase(Directory, StrippedArgs);
 }
Index: lib/Tooling/CommonOptionsParser.cpp
===================================================================
--- lib/Tooling/CommonOptionsParser.cpp
+++ lib/Tooling/CommonOptionsParser.cpp
@@ -116,16 +116,20 @@
 
   cl::HideUnrelatedOptions(Category);
 
-  Compilations.reset(FixedCompilationDatabase::loadFromCommandLine(argc, argv));
+  std::string ErrorMessage;
+  Compilations.reset(
+      FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage));
+  if (!Compilations) {
+    llvm::errs() << ErrorMessage;
+  }
   cl::ParseCommandLineOptions(argc, argv, Overview);
   cl::PrintOptionValues();
 
   SourcePathList = SourcePaths;
   if ((OccurrencesFlag == cl::ZeroOrMore || OccurrencesFlag == cl::Optional) &&
       SourcePathList.empty())
     return;
   if (!Compilations) {
-    std::string ErrorMessage;
     if (!BuildPath.empty()) {
       Compilations =
           CompilationDatabase::autoDetectFromDirectory(BuildPath, ErrorMessage);
Index: lib/Frontend/CreateInvocationFromCommandLine.cpp
===================================================================
--- lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -52,6 +52,8 @@
   TheDriver.setCheckInputsExist(false);
 
   std::unique_ptr<driver::Compilation> 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)) {
Index: include/clang/Tooling/CompilationDatabase.h
===================================================================
--- include/clang/Tooling/CompilationDatabase.h
+++ include/clang/Tooling/CompilationDatabase.h
@@ -186,11 +186,20 @@
   /// 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,
+                                                       std::string &ErrorMsg,
                                                        Twine Directory = ".");
 
+  static FixedCompilationDatabase *loadFromCommandLine(int &Argc,
+                                                       const char *const *Argv,
+                                                       Twine Directory = ".") {
+    std::string ErrorMsg;
+    return loadFromCommandLine(Argc, Argv, ErrorMsg, Directory);
+  }
+
   /// \brief Constructs a compilation data base from a specified directory
   /// and command line.
   FixedCompilationDatabase(Twine Directory, ArrayRef<std::string> CommandLine);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to