On Tue, Feb 26, 2013 at 9:50 AM, Chad Rosier <span dir="ltr"><<a href="mailto:mcrosier@apple.com" target="_blank">mcrosier@apple.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div style="word-wrap:break-word">Matthew,<div><br><div><div><div>On Feb 26, 2013, at 6:11 AM, Matthew Curtis <<a href="mailto:mcurtis@codeaurora.org" target="_blank">mcurtis@codeaurora.org</a>> wrote:</div>
<br><blockquote type="cite"><div text="#000000" bgcolor="#FFFFFF" style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div>Hello Chad,<br><br>Could you provide a little more background on why it's important to execute subsequent commands after one fails?<br>

</div></div></blockquote><div><br></div></div><div>It is required for Unix conformance testing. :)</div><div><br><blockquote type="cite"><div text="#000000" bgcolor="#FFFFFF" style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">

<div><br>This change is currently causing some noise for failed compilations on hexagon:<br><blockquote><tt>hello-world.c:17:26: error: expected ';' after expression</tt><br><tt>  printf("hello world\n")</tt><br>

<tt>                         ^</tt><br><tt>                         ;</tt><br><tt>1 error generated.</tt><br><font color="#990000"><tt>Assembler messages:</tt><br><tt>Error: can't open /tmp/hello-world-ZhHzJ1.s for reading: No such file or directory</tt><br>

<tt><...>/qc/../gnu/bin/hexagon-ld: cannot find /tmp/hello-world-KKCeEQ.o: No such file or directory</tt><br><tt>clang: error: hexagon-as command failed with exit code 1 (use -v to see invocation)</tt><br><tt>clang: error: hexagon-ld command failed with exit code 1 (use -v to see invocation)</tt><br>

</font></blockquote><br>Should it only apply to commands that do not depend on the output of a failing command?<br></div></div></blockquote><div><br></div></div><div>It would be nice to detect such dependencies, but I don't plan on implementing it any time soon.</div>

<div><br><blockquote type="cite"><div text="#000000" bgcolor="#FFFFFF" style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div><br>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.<br>

</div></div></blockquote><div><br></div></div><div>If it's a huge distraction, we might consider adding a driver flag that would cause the previous behavior.</div></div></div></div></blockquote><div><br></div><div>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:</div>

<div><br></div><div>$ clang++ my_simple_program.cc</div><div><div>my_simple_program.cc:1:10: error: expected ';' after top level declarator</div><div>int error error;</div><div>         ^</div><div>         ;</div>
<div>1 error generated.</div><div>/usr/bin/ld: cannot find /tmp/--gWf0D2.o: No such file or directory</div><div>clang-3: error: linker command failed with exit code 1 (use -v to see invocation)</div></div><div><br></div><div>
 </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><span><font color="#888888"><div>
 Chad</div></font></span><div><div><br><blockquote type="cite"><div text="#000000" bgcolor="#FFFFFF" style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">

<div><br>Thanks,<br>Matthew Curtis<br><br>On 1/29/2013 2:15 PM, Chad Rosier wrote:<br></div><blockquote type="cite"><pre>Author: mcrosier
Date: Tue Jan 29 14:15:05 2013
New Revision: 173825

URL: <a href="http://llvm.org/viewvc/llvm-project?rev=173825&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=173825&view=rev</a>
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 <a>rdar://12984531</a>

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: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=173825&r1=173824&r2=173825&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=173825&r1=173824&r2=173825&view=diff</a>
==============================================================================
--- 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: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=173825&r1=173824&r2=173825&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=173825&r1=173824&r2=173825&view=diff</a>
==============================================================================
--- 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: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=173825&r1=173824&r2=173825&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=173825&r1=173824&r2=173825&view=diff</a>
==============================================================================
--- 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: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=173825&r1=173824&r2=173825&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=173825&r1=173824&r2=173825&view=diff</a>
==============================================================================
--- 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: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/output-file-cleanup.c?rev=173825&r1=173824&r2=173825&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/output-file-cleanup.c?rev=173825&r1=173824&r2=173825&view=diff</a>
==============================================================================
--- 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: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=173825&r1=173824&r2=173825&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=173825&r1=173824&r2=173825&view=diff</a>
==============================================================================
--- 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
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a>
</pre></blockquote><br><br><pre cols="72">-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation</pre></div></blockquote></div></div></div><br></div></div><br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br>