r173825 - [driver] Refactor the driver so that a failing commands doesn't prevent
Richard Smith
richard at metafoo.co.uk
Tue Feb 26 13:12:45 PST 2013
On Tue, Feb 26, 2013 at 9:50 AM, Chad Rosier <mcrosier at apple.com> wrote:
> Matthew,
>
> On Feb 26, 2013, at 6:11 AM, Matthew Curtis <mcurtis at codeaurora.org>
> wrote:
>
> Hello Chad,
>
> Could you provide a little more background on why it's important to
> execute subsequent commands after one fails?
>
>
> It is required for Unix conformance testing. :)
>
>
> This change is currently causing some noise for failed compilations on
> hexagon:
>
> hello-world.c:17:26: error: expected ';' after expression
> printf("hello world\n")
> ^
> ;
> 1 error generated.
> Assembler messages:
> Error: can't open /tmp/hello-world-ZhHzJ1.s for reading: No such file or
> directory
> <...>/qc/../gnu/bin/hexagon-ld: cannot find /tmp/hello-world-KKCeEQ.o: No
> such file or directory
> clang: error: hexagon-as command failed with exit code 1 (use -v to see
> invocation)
> clang: error: hexagon-ld command failed with exit code 1 (use -v to see
> invocation)
>
>
> Should it only apply to commands that do not depend on the output of a
> failing command?
>
>
> It would be nice to detect such dependencies, but I don't plan on
> implementing it any time soon.
>
>
> I'm just trying to understand if this feature is perhaps not appropriate
> for the hexagon target or if I should try to do something to quiet or
> inhibit downstream commands.
>
>
> If it's a huge distraction, we might consider adding a driver flag that
> would cause the previous behavior.
>
This has made all failing multi-stage compiles ugly. Do we have a good
reason not to restore the previous behavior by default? This just looks
stupid:
$ clang++ my_simple_program.cc
my_simple_program.cc:1:10: error: expected ';' after top level declarator
int error error;
^
;
1 error generated.
/usr/bin/ld: cannot find /tmp/--gWf0D2.o: No such file or directory
clang-3: error: linker command failed with exit code 1 (use -v to see
invocation)
> Chad
>
>
> Thanks,
> Matthew Curtis
>
> On 1/29/2013 2:15 PM, Chad Rosier wrote:
>
> Author: mcrosier
> Date: Tue Jan 29 14:15:05 2013
> New Revision: 173825
>
> URL: http://llvm.org/viewvc/llvm-project?rev=173825&view=rev
> Log:
> [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
>
> 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/test/Driver/output-file-cleanup.c
> cfe/trunk/tools/driver/driver.cpp
>
> Modified: cfe/trunk/include/clang/Driver/Compilation.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=173825&r1=173824&r2=173825&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Driver/Compilation.h (original)
> +++ cfe/trunk/include/clang/Driver/Compilation.h Tue Jan 29 14:15:05 2013
> @@ -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<int, const Command *> > &FailingCommands) const;
>
> /// initCompilationForDiagnostics - Remove stale state and suppress output
> /// so compilation can be reexecuted to generate additional diagnostic
>
> Modified: cfe/trunk/include/clang/Driver/Driver.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=173825&r1=173824&r2=173825&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Driver/Driver.h (original)
> +++ cfe/trunk/include/clang/Driver/Driver.h Tue Jan 29 14:15:05 2013
> @@ -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<int, const Command *> > &FailingCommands) const;
>
> /// generateCompilationDiagnostics - Generate diagnostics information
> /// including preprocessed source file(s).
>
> Modified: cfe/trunk/lib/Driver/Compilation.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=173825&r1=173824&r2=173825&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Driver/Compilation.cpp (original)
> +++ cfe/trunk/lib/Driver/Compilation.cpp Tue Jan 29 14:15:05 2013
> @@ -307,17 +307,17 @@ int Compilation::ExecuteCommand(const Co
> return Res;
> }
>
> -int Compilation::ExecuteJob(const Job &J,
> - const Command *&FailingCommand) const {
> +void Compilation::ExecuteJob(const Job &J,
> + SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands) const {
> if (const Command *C = dyn_cast<Command>(&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<JobList>(&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);
> }
> }
>
>
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=173825&r1=173824&r2=173825&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Tue Jan 29 14:15:05 2013
> @@ -440,11 +440,11 @@ void Driver::generateCompilationDiagnost
> }
>
> // Generate preprocessed output.
> - FailingCommand = 0;
> - int Res = C.ExecuteJob(C.getJobs(), FailingCommand);
> + SmallVector<std::pair<int, const Command *>, 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::generateCompilationDiagnost
> }
>
> int Driver::ExecuteCompilation(const Compilation &C,
> - const Command *&FailingCommand) const {
> + SmallVectorImpl< std::pair<int, const Command *> > &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 Com
> 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<JobAction>(&FailingCommand->getSource());
> - C.CleanupFileMap(C.getResultFiles(), JA, true);
> + // Otherwise, remove result files and print extra information about abnormal
> + // failures.
> + for (SmallVectorImpl< std::pair<int, const Command *> >::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<JobAction>(&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 {
>
> Modified: cfe/trunk/test/Driver/output-file-cleanup.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/output-file-cleanup.c?rev=173825&r1=173824&r2=173825&view=diff
> ==============================================================================
> --- cfe/trunk/test/Driver/output-file-cleanup.c (original)
> +++ cfe/trunk/test/Driver/output-file-cleanup.c Tue Jan 29 14:15:05 2013
> @@ -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
>
> Modified: cfe/trunk/tools/driver/driver.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=173825&r1=173824&r2=173825&view=diff
> ==============================================================================
> --- cfe/trunk/tools/driver/driver.cpp (original)
> +++ cfe/trunk/tools/driver/driver.cpp Tue Jan 29 14:15:05 2013
> @@ -466,21 +466,35 @@ int main(int argc_, const char **argv_)
>
> OwningPtr<Compilation> C(TheDriver.BuildCompilation(argv));
> int Res = 0;
> - const Command *FailingCommand = 0;
> + SmallVector<std::pair<int, const Command *>, 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<int, const Command *> >::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;
> }
>
>
> _______________________________________________
> cfe-commits mailing listcfe-commits at cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130226/91bcb236/attachment.html>
More information about the cfe-commits
mailing list