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. > > > 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 > [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
