On Feb 26, 2013, at 1:12 PM, Richard Smith <[email protected]> wrote:
> On Tue, Feb 26, 2013 at 9:50 AM, Chad Rosier <[email protected]> wrote: > Matthew, > > On Feb 26, 2013, at 6:11 AM, Matthew Curtis <[email protected]> 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. Change of plan; I'll see if I can't get this fixed in a day or two. > >> >> 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)FWIW, the original idea was to prevent the failure of one > translation unit not effect FWIW, the original idea was to prevent the failure of one translation unit from preventing the compilation of another. Unfortunately, I missed the fact that the failure of any phase (e.g., preprocess, compile, assemble) for a single translation unit should prevent later phases from executing. Chad > > 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 list >>> [email protected] >>> http://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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
