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

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 30 06:21:27 PDT 2017


Author: d0k
Date: Fri Jun 30 06:21:27 2017
New Revision: 306822

URL: http://llvm.org/viewvc/llvm-project?rev=306822&view=rev
Log:
[Driver] Actually report errors during parsing instead of stopping when there's an error somewhere.

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.

Modified:
    cfe/trunk/include/clang/Driver/Compilation.h
    cfe/trunk/include/clang/Driver/Driver.h
    cfe/trunk/lib/Driver/Compilation.cpp
    cfe/trunk/lib/Driver/Driver.cpp
    cfe/trunk/tools/driver/driver.cpp
    cfe/trunk/unittests/Driver/ToolChainTest.cpp

Modified: cfe/trunk/include/clang/Driver/Compilation.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=306822&r1=306821&r2=306822&view=diff
==============================================================================
--- cfe/trunk/include/clang/Driver/Compilation.h (original)
+++ cfe/trunk/include/clang/Driver/Compilation.h Fri Jun 30 06:21:27 2017
@@ -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

Modified: cfe/trunk/include/clang/Driver/Driver.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=306822&r1=306821&r2=306822&view=diff
==============================================================================
--- cfe/trunk/include/clang/Driver/Driver.h (original)
+++ cfe/trunk/include/clang/Driver/Driver.h Fri Jun 30 06:21:27 2017
@@ -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.

Modified: cfe/trunk/lib/Driver/Compilation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=306822&r1=306821&r2=306822&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/Compilation.cpp (original)
+++ cfe/trunk/lib/Driver/Compilation.cpp Fri Jun 30 06:21:27 2017
@@ -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));

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=306822&r1=306821&r2=306822&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Fri Jun 30 06:21:27 2017
@@ -153,8 +153,10 @@ void Driver::setDriverModeFromOption(Str
     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(Arr
                           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(Ar
   // 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(Ar
       *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;

Modified: cfe/trunk/tools/driver/driver.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=306822&r1=306821&r2=306822&view=diff
==============================================================================
--- cfe/trunk/tools/driver/driver.cpp (original)
+++ cfe/trunk/tools/driver/driver.cpp Fri Jun 30 06:21:27 2017
@@ -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);
 

Modified: cfe/trunk/unittests/Driver/ToolChainTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Driver/ToolChainTest.cpp?rev=306822&r1=306821&r2=306822&view=diff
==============================================================================
--- cfe/trunk/unittests/Driver/ToolChainTest.cpp (original)
+++ cfe/trunk/unittests/Driver/ToolChainTest.cpp Fri Jun 30 06:21:27 2017
@@ -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.




More information about the cfe-commits mailing list