From a16355c31878403443f99077cc8df8318457faf5 Mon Sep 17 00:00:00 2001 From: Chad Rosier Date: Tue, 29 Jan 2013 20:15:05 +0000 Subject: [PATCH] [driver] Refactor the driver so that a failing commands doesn't prevent subsequent commands from being executed. The diagnostics generation isn't designed for this use case, so add a note to fix this in the very near future. For now, just generated the diagnostics for the first failing command. Part of rdar://12984531 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@173825 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Driver/Compilation.h | 8 ++-- include/clang/Driver/Driver.h | 2 +- lib/Driver/Compilation.cpp | 16 +++---- lib/Driver/Driver.cpp | 75 ++++++++++++++++-------------- test/Driver/output-file-cleanup.c | 12 +++++ tools/driver/driver.cpp | 32 +++++++++---- 6 files changed, 90 insertions(+), 55 deletions(-) diff --git a/include/clang/Driver/Compilation.h b/include/clang/Driver/Compilation.h index 1872050522..15c5e40e49 100644 --- a/include/clang/Driver/Compilation.h +++ b/include/clang/Driver/Compilation.h @@ -175,10 +175,10 @@ public: /// ExecuteJob - Execute a single job. /// - /// \param FailingCommand - For non-zero results, this will be set to the - /// Command which failed. - /// \return The accumulated result code of the job. - int ExecuteJob(const Job &J, const Command *&FailingCommand) const; + /// \param FailingCommands - For non-zero results, this will be a vector of + /// failing commands and their associated result code. + void ExecuteJob(const Job &J, + SmallVectorImpl< std::pair > &FailingCommands) const; /// initCompilationForDiagnostics - Remove stale state and suppress output /// so compilation can be reexecuted to generate additional diagnostic diff --git a/include/clang/Driver/Driver.h b/include/clang/Driver/Driver.h index ae858d34bb..c1f476f3ce 100644 --- a/include/clang/Driver/Driver.h +++ b/include/clang/Driver/Driver.h @@ -272,7 +272,7 @@ public: /// to just running the subprocesses, for example reporting errors, removing /// temporary files, etc. int ExecuteCompilation(const Compilation &C, - const Command *&FailingCommand) const; + SmallVectorImpl< std::pair > &FailingCommands) const; /// generateCompilationDiagnostics - Generate diagnostics information /// including preprocessed source file(s). diff --git a/lib/Driver/Compilation.cpp b/lib/Driver/Compilation.cpp index 96e6048551..da589b9d5b 100644 --- a/lib/Driver/Compilation.cpp +++ b/lib/Driver/Compilation.cpp @@ -307,17 +307,17 @@ int Compilation::ExecuteCommand(const Command &C, return Res; } -int Compilation::ExecuteJob(const Job &J, - const Command *&FailingCommand) const { +void Compilation::ExecuteJob(const Job &J, + SmallVectorImpl< std::pair > &FailingCommands) const { if (const Command *C = dyn_cast(&J)) { - return ExecuteCommand(*C, FailingCommand); + const Command *FailingCommand = 0; + if (int Res = ExecuteCommand(*C, FailingCommand)) + FailingCommands.push_back(std::make_pair(Res, FailingCommand)); } else { const JobList *Jobs = cast(&J); - for (JobList::const_iterator - it = Jobs->begin(), ie = Jobs->end(); it != ie; ++it) - if (int Res = ExecuteJob(**it, FailingCommand)) - return Res; - return 0; + for (JobList::const_iterator it = Jobs->begin(), ie = Jobs->end(); + it != ie; ++it) + ExecuteJob(**it, FailingCommands); } } diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp index c97b3786b3..0ba61a3de8 100644 --- a/lib/Driver/Driver.cpp +++ b/lib/Driver/Driver.cpp @@ -440,11 +440,11 @@ void Driver::generateCompilationDiagnostics(Compilation &C, } // Generate preprocessed output. - FailingCommand = 0; - int Res = C.ExecuteJob(C.getJobs(), FailingCommand); + SmallVector, 4> FailingCommands; + C.ExecuteJob(C.getJobs(), FailingCommands); // If the command succeeded, we are done. - if (Res == 0) { + if (FailingCommands.empty()) { Diag(clang::diag::note_drv_command_failed_diag_msg) << "\n********************\n\n" "PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:\n" @@ -495,7 +495,7 @@ void Driver::generateCompilationDiagnostics(Compilation &C, } int Driver::ExecuteCompilation(const Compilation &C, - const Command *&FailingCommand) const { + SmallVectorImpl< std::pair > &FailingCommands) const { // Just print if -### was present. if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH)) { C.PrintJob(llvm::errs(), C.getJobs(), "\n", true); @@ -506,45 +506,52 @@ int Driver::ExecuteCompilation(const Compilation &C, if (Diags.hasErrorOccurred()) return 1; - int Res = C.ExecuteJob(C.getJobs(), FailingCommand); + C.ExecuteJob(C.getJobs(), FailingCommands); // Remove temp files. C.CleanupFileList(C.getTempFiles()); // If the command succeeded, we are done. - if (Res == 0) - return Res; + if (FailingCommands.empty()) + return 0; - // Otherwise, remove result files as well. - if (!C.getArgs().hasArg(options::OPT_save_temps)) { - const JobAction *JA = cast(&FailingCommand->getSource()); - C.CleanupFileMap(C.getResultFiles(), JA, true); + // Otherwise, remove result files and print extra information about abnormal + // failures. + for (SmallVectorImpl< std::pair >::iterator it = + FailingCommands.begin(), ie = FailingCommands.end(); it != ie; ++it) { + int Res = it->first; + const Command *FailingCommand = it->second; - // Failure result files are valid unless we crashed. - if (Res < 0) - C.CleanupFileMap(C.getFailureResultFiles(), JA, true); - } + // Remove result files if we're not saving temps. + if (!C.getArgs().hasArg(options::OPT_save_temps)) { + const JobAction *JA = cast(&FailingCommand->getSource()); + C.CleanupFileMap(C.getResultFiles(), JA, true); - // Print extra information about abnormal failures, if possible. - // - // This is ad-hoc, but we don't want to be excessively noisy. If the result - // status was 1, assume the command failed normally. In particular, if it was - // the compiler then assume it gave a reasonable error code. Failures in other - // tools are less common, and they generally have worse diagnostics, so always - // print the diagnostic there. - const Tool &FailingTool = FailingCommand->getCreator(); - - if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) { - // FIXME: See FIXME above regarding result code interpretation. - if (Res < 0) - Diag(clang::diag::err_drv_command_signalled) - << FailingTool.getShortName(); - else - Diag(clang::diag::err_drv_command_failed) - << FailingTool.getShortName() << Res; - } + // Failure result files are valid unless we crashed. + if (Res < 0) + C.CleanupFileMap(C.getFailureResultFiles(), JA, true); + } - return Res; + // Print extra information about abnormal failures, if possible. + // + // This is ad-hoc, but we don't want to be excessively noisy. If the result + // status was 1, assume the command failed normally. In particular, if it + // was the compiler then assume it gave a reasonable error code. Failures + // in other tools are less common, and they generally have worse + // diagnostics, so always print the diagnostic there. + const Tool &FailingTool = FailingCommand->getCreator(); + + if (!FailingCommand->getCreator().hasGoodDiagnostics() || Res != 1) { + // FIXME: See FIXME above regarding result code interpretation. + if (Res < 0) + Diag(clang::diag::err_drv_command_signalled) + << FailingTool.getShortName(); + else + Diag(clang::diag::err_drv_command_failed) + << FailingTool.getShortName() << Res; + } + } + return 0; } void Driver::PrintOptions(const ArgList &Args) const { diff --git a/test/Driver/output-file-cleanup.c b/test/Driver/output-file-cleanup.c index a8a16d3c39..0aee5f8fa9 100644 --- a/test/Driver/output-file-cleanup.c +++ b/test/Driver/output-file-cleanup.c @@ -36,3 +36,15 @@ invalid C code // RUN: cd %T && not %clang -S %t1.c %t2.c // RUN: test -f %t1.s // RUN: test ! -f %t2.s + +// RUN: touch %t1.c +// RUN: echo "invalid C code" > %t2.c +// RUN: touch %t3.c +// RUN: echo "invalid C code" > %t4.c +// RUN: touch %t5.c +// RUN: cd %T && not %clang -S %t1.c %t2.c %t3.c %t4.c %t5.c +// RUN: test -f %t1.s +// RUN: test ! -f %t2.s +// RUN: test -f %t3.s +// RUN: test ! -f %t4.s +// RUN: test -f %t5.s diff --git a/tools/driver/driver.cpp b/tools/driver/driver.cpp index 81bc985cc6..7ccf57a242 100644 --- a/tools/driver/driver.cpp +++ b/tools/driver/driver.cpp @@ -466,21 +466,35 @@ int main(int argc_, const char **argv_) { OwningPtr C(TheDriver.BuildCompilation(argv)); int Res = 0; - const Command *FailingCommand = 0; + SmallVector, 4> FailingCommands; if (C.get()) - Res = TheDriver.ExecuteCompilation(*C, FailingCommand); + Res = TheDriver.ExecuteCompilation(*C, FailingCommands); // Force a crash to test the diagnostics. if (::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH")) { Diags.Report(diag::err_drv_force_crash) << "FORCE_CLANG_DIAGNOSTICS_CRASH"; - Res = -1; + const Command *FailingCommand = 0; + FailingCommands.push_back(std::make_pair(-1, FailingCommand)); } - // If result status is < 0, then the driver command signalled an error. - // If result status is 70, then the driver command reported a fatal error. - // In these cases, generate additional diagnostic information if possible. - if (Res < 0 || Res == 70) - TheDriver.generateCompilationDiagnostics(*C, FailingCommand); + for (SmallVectorImpl< std::pair >::iterator it = + FailingCommands.begin(), ie = FailingCommands.end(); it != ie; ++it) { + int CommandRes = it->first; + const Command *FailingCommand = it->second; + if (!Res) + Res = CommandRes; + + // If result status is < 0, then the driver command signalled an error. + // If result status is 70, then the driver command reported a fatal error. + // In these cases, generate additional diagnostic information if possible. + if (CommandRes < 0 || CommandRes == 70) { + TheDriver.generateCompilationDiagnostics(*C, FailingCommand); + + // FIXME: generateCompilationDiagnostics() needs to be tested when there + // are multiple failing commands. + break; + } + } // If any timers were active but haven't been destroyed yet, print their // results now. This happens in -disable-free mode. @@ -496,5 +510,7 @@ int main(int argc_, const char **argv_) { Res = 1; #endif + // If we have multiple failing commands, we return the result of the first + // failing command. return Res; } -- 2.40.0