From f5f1f63df1b65b19c63a06ff10921f08ce043a81 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Fri, 30 Jun 2017 13:21:27 +0000 Subject: [PATCH] [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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@306822 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Driver/Compilation.h | 8 +++++- include/clang/Driver/Driver.h | 3 ++- lib/Driver/Compilation.cpp | 5 ++-- lib/Driver/Driver.cpp | 42 +++++++++++++++++++++--------- tools/driver/driver.cpp | 2 +- unittests/Driver/ToolChainTest.cpp | 11 ++++++++ 6 files changed, 53 insertions(+), 18 deletions(-) diff --git a/include/clang/Driver/Compilation.h b/include/clang/Driver/Compilation.h index 114e0b33c7..036b04605a 100644 --- 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 1009754a15..5a087eea1b 100644 --- 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 Args); + llvm::opt::InputArgList ParseArgStrings(ArrayRef 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 5c13e59a0d..cf86644fb8 100644 --- 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 f23975829e..faced0697e 100644 --- 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 ArgStrings) { +InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings, + bool &ContainsError) { llvm::PrettyStackTraceString CrashInfo("Command line argument parsing"); + ContainsError = false; unsigned IncludedFlagsBitmask; unsigned ExcludedFlagsBitmask; @@ -167,27 +169,41 @@ InputArgList Driver::ParseArgStrings(ArrayRef 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 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 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 af25d95980..9f37c428ff 100644 --- a/tools/driver/driver.cpp +++ b/tools/driver/driver.cpp @@ -462,7 +462,7 @@ int main(int argc_, const char **argv_) { std::unique_ptr C(TheDriver.BuildCompilation(argv)); int Res = 1; - if (C.get()) { + if (C && !C->containsError()) { SmallVector, 4> FailingCommands; Res = TheDriver.ExecuteCompilation(*C, FailingCommands); diff --git a/unittests/Driver/ToolChainTest.cpp b/unittests/Driver/ToolChainTest.cpp index bce2748aa2..ec50560b20 100644 --- 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 DiagID(new DiagnosticIDs()); + struct TestDiagnosticConsumer : public DiagnosticConsumer {}; + IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions(); + DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer); + Driver TheDriver("/bin/clang", "arm-linux-gnueabihf", Diags); + std::unique_ptr C(TheDriver.BuildCompilation( + {"-fsyntax-only", "-fan-unknown-option", "foo.cpp"})); + EXPECT_TRUE(C); + EXPECT_TRUE(C->containsError()); +} } // end anonymous namespace. -- 2.50.1