[Driver] Actually report errors during parsing instead of stopping when there's an...

This is a more principled version of r303756. That change was both very
brittle about the state of the Diags object going into the driver and
also broke tooling in funny ways.

In particular it prevented tools from capturing diagnostics properly and
made the compilation database logic fail to provide arguments to the
tool, falling back to scanning directories for JSON files.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@306822 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Driver/Compilation.h b/include/clang/Driver/Compilation.h
index 114e0b33c75ab63d62b3ccd9c4dc76a2edbdce46..036b04605ac70875f192cfea9c95a2adbddd91e8 100644 (file)
--- a/include/clang/Driver/Compilation.h
+++ b/include/clang/Driver/Compilation.h
@@ -105,10 +105,13 @@ class Compilation {
   /// Whether we're compiling for diagnostic purposes.
   bool ForDiagnostics;
 
+  /// Whether an error during the parsing of the input args.
+  bool ContainsError;
+
 public:
   Compilation(const Driver &D, const ToolChain &DefaultToolChain,
               llvm::opt::InputArgList *Args,
-              llvm::opt::DerivedArgList *TranslatedArgs);
+              llvm::opt::DerivedArgList *TranslatedArgs, bool ContainsError);
   ~Compilation();
 
   const Driver &getDriver() const { return TheDriver; }
@@ -275,6 +278,9 @@ public:
   /// Return true if we're compiling for diagnostics.
   bool isForDiagnostics() const { return ForDiagnostics; }
 
+  /// Return whether an error during the parsing of the input args.
+  bool containsError() const { return ContainsError; }
+
   /// Redirect - Redirect output of this compilation. Can only be done once.
   ///
   /// \param Redirects - array of pointers to paths. The array

diff --git a/include/clang/Driver/Driver.h b/include/clang/Driver/Driver.h
index 1009754a15d52a6987b2081e4760b8956885cb43..5a087eea1b4ec62a54747ec6ad8d31b2af8932b8 100644 (file)
--- a/include/clang/Driver/Driver.h
+++ b/include/clang/Driver/Driver.h
@@ -341,7 +341,8 @@ public:
 
   /// ParseArgStrings - Parse the given list of strings into an
   /// ArgList.
-  llvm::opt::InputArgList ParseArgStrings(ArrayRef<const char *> Args);
+  llvm::opt::InputArgList ParseArgStrings(ArrayRef<const char *> Args,
+                                          bool &ContainsError);
 
   /// BuildInputs - Construct the list of inputs and their types from 
   /// the given arguments.

diff --git a/lib/Driver/Compilation.cpp b/lib/Driver/Compilation.cpp
index 5c13e59a0d73b45fd0ca444356d6422b4d35eb39..cf86644fb8cdfea8fb39786d8526536fffbe18d3 100644 (file)
--- a/lib/Driver/Compilation.cpp
+++ b/lib/Driver/Compilation.cpp
@@ -23,10 +23,11 @@ using namespace clang;
 using namespace llvm::opt;
 
 Compilation::Compilation(const Driver &D, const ToolChain &_DefaultToolChain,
-                         InputArgList *_Args, DerivedArgList *_TranslatedArgs)
+                         InputArgList *_Args, DerivedArgList *_TranslatedArgs,
+                         bool ContainsError)
     : TheDriver(D), DefaultToolChain(_DefaultToolChain), ActiveOffloadMask(0u),
       Args(_Args), TranslatedArgs(_TranslatedArgs), Redirects(nullptr),
-      ForDiagnostics(false) {
+      ForDiagnostics(false), ContainsError(ContainsError) {
   // The offloading host toolchain is the default tool chain.
   OrderedOffloadingToolchains.insert(
       std::make_pair(Action::OFK_Host, &DefaultToolChain));

diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
index f23975829eaf3c393f91962497395fd33edd47f3..faced0697ed94ff1d6c97f1d49e43e3aba61d67b 100644 (file)
--- a/lib/Driver/Driver.cpp
+++ b/lib/Driver/Driver.cpp
@@ -153,8 +153,10 @@ void Driver::setDriverModeFromOption(StringRef Opt) {
     Diag(diag::err_drv_unsupported_option_argument) << OptName << Value;
 }
 
-InputArgList Driver::ParseArgStrings(ArrayRef<const char *> ArgStrings) {
+InputArgList Driver::ParseArgStrings(ArrayRef<const char *> ArgStrings,
+                                     bool &ContainsError) {
   llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
+  ContainsError = false;
 
   unsigned IncludedFlagsBitmask;
   unsigned ExcludedFlagsBitmask;
@@ -167,27 +169,41 @@ InputArgList Driver::ParseArgStrings(ArrayRef<const char *> ArgStrings) {
                           IncludedFlagsBitmask, ExcludedFlagsBitmask);
 
   // Check for missing argument error.
-  if (MissingArgCount)
-    Diag(clang::diag::err_drv_missing_argument)
+  if (MissingArgCount) {
+    Diag(diag::err_drv_missing_argument)
         << Args.getArgString(MissingArgIndex) << MissingArgCount;
+    ContainsError |=
+        Diags.getDiagnosticLevel(diag::err_drv_missing_argument,
+                                 SourceLocation()) > DiagnosticsEngine::Warning;
+  }
 
   // Check for unsupported options.
   for (const Arg *A : Args) {
     if (A->getOption().hasFlag(options::Unsupported)) {
-      Diag(clang::diag::err_drv_unsupported_opt) << A->getAsString(Args);
+      Diag(diag::err_drv_unsupported_opt) << A->getAsString(Args);
+      ContainsError |= Diags.getDiagnosticLevel(diag::err_drv_unsupported_opt,
+                                                SourceLocation()) >
+                       DiagnosticsEngine::Warning;
       continue;
     }
 
     // Warn about -mcpu= without an argument.
     if (A->getOption().matches(options::OPT_mcpu_EQ) && A->containsValue("")) {
-      Diag(clang::diag::warn_drv_empty_joined_argument) << A->getAsString(Args);
+      Diag(diag::warn_drv_empty_joined_argument) << A->getAsString(Args);
+      ContainsError |= Diags.getDiagnosticLevel(
+                           diag::warn_drv_empty_joined_argument,
+                           SourceLocation()) > DiagnosticsEngine::Warning;
     }
   }
 
-  for (const Arg *A : Args.filtered(options::OPT_UNKNOWN))
-    Diags.Report(IsCLMode() ? diag::warn_drv_unknown_argument_clang_cl :
-                              diag::err_drv_unknown_argument)
-      << A->getAsString(Args);
+  for (const Arg *A : Args.filtered(options::OPT_UNKNOWN)) {
+    auto ID = IsCLMode() ? diag::warn_drv_unknown_argument_clang_cl
+                         : diag::err_drv_unknown_argument;
+
+    Diags.Report(ID) << A->getAsString(Args);
+    ContainsError |= Diags.getDiagnosticLevel(ID, SourceLocation()) >
+                     DiagnosticsEngine::Warning;
+  }
 
   return Args;
 }
@@ -600,9 +616,8 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
   // FIXME: This stuff needs to go into the Compilation, not the driver.
   bool CCCPrintPhases;
 
-  InputArgList Args = ParseArgStrings(ArgList.slice(1));
-  if (Diags.hasErrorOccurred())
-    return nullptr;
+  bool ContainsError;
+  InputArgList Args = ParseArgStrings(ArgList.slice(1), ContainsError);
 
   // Silence driver warnings if requested
   Diags.setIgnoreAllWarnings(Args.hasArg(options::OPT_w));
@@ -692,7 +707,8 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
       *UArgs, computeTargetTriple(*this, DefaultTargetTriple, *UArgs));
 
   // The compilation takes ownership of Args.
-  Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs);
+  Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
+                                   ContainsError);
 
   if (!HandleImmediateArgs(*C))
     return C;

diff --git a/tools/driver/driver.cpp b/tools/driver/driver.cpp
index af25d95980218f1fe9ecd2dc2d3ef7ab88b62a6f..9f37c428ff9320e78a03fa4088bbb934f19b7a10 100644 (file)
--- a/tools/driver/driver.cpp
+++ b/tools/driver/driver.cpp
@@ -462,7 +462,7 @@ int main(int argc_, const char **argv_) {
 
   std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(argv));
   int Res = 1;
-  if (C.get()) {
+  if (C && !C->containsError()) {
     SmallVector<std::pair<int, const Command *>, 4> FailingCommands;
     Res = TheDriver.ExecuteCompilation(*C, FailingCommands);
 

diff --git a/unittests/Driver/ToolChainTest.cpp b/unittests/Driver/ToolChainTest.cpp
index bce2748aa21b4d4f1cfc39c6646bf8941f073385..ec50560b202befebba307df9b1f1dccbc59ae328 100644 (file)
--- a/unittests/Driver/ToolChainTest.cpp
+++ b/unittests/Driver/ToolChainTest.cpp
@@ -152,5 +152,16 @@ TEST(ToolChainTest, DefaultDriverMode) {
   EXPECT_TRUE(CXXDriver.CCCIsCXX());
   EXPECT_TRUE(CLDriver.IsCLMode());
 }
+TEST(ToolChainTest, InvalidArgument) {
+  IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+  DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+  Driver TheDriver("/bin/clang", "arm-linux-gnueabihf", Diags);
+  std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(
+      {"-fsyntax-only", "-fan-unknown-option", "foo.cpp"}));
+  EXPECT_TRUE(C);
+  EXPECT_TRUE(C->containsError());
+}
 
 } // end anonymous namespace.