How about the attached patch as a possible solution?

It uses the dependency information in the Action associated with a Command to prevent the execution of any Commands downstream from a failing Command.

On 2/26/2013 3:27 PM, Chad Rosier wrote:

On Feb 26, 2013, at 1:12 PM, Richard Smith <[email protected] <mailto:[email protected]>> wrote:

On Tue, Feb 26, 2013 at 9:50 AM, Chad Rosier<[email protected] <mailto:[email protected]>>wrote:

    Matthew,

    On Feb 26, 2013, at 6:11 AM, Matthew Curtis
    <[email protected] <mailto:[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 ofrdar://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]  <mailto:[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] <mailto:[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

>From ab16be648f7f43e5aa5f128d349dec85b02afe80 Mon Sep 17 00:00:00 2001
From: Matthew Curtis <[email protected]>
Date: Tue, 26 Feb 2013 16:07:29 -0600
Subject: [PATCH] Inhibit downstream commands when a command fails.

---
 lib/Driver/Compilation.cpp                |   34 ++++++++++++++++++++++++++++-
 test/Driver/inhibit-downstream-commands.c |    4 +++
 2 files changed, 37 insertions(+), 1 deletions(-)
 create mode 100644 test/Driver/inhibit-downstream-commands.c

diff --git a/lib/Driver/Compilation.cpp b/lib/Driver/Compilation.cpp
index df904f0..929b54f 100644
--- a/lib/Driver/Compilation.cpp
+++ b/lib/Driver/Compilation.cpp
@@ -307,9 +307,41 @@ int Compilation::ExecuteCommand(const Command &C,
   return Res;
 }
 
+typedef SmallVectorImpl< std::pair<int, const Command *> > FailingCommandList;
+
+static bool ActionFailed(const Action *A,
+                         const FailingCommandList &FailingCommands) {
+
+  if (FailingCommands.empty())
+    return false;
+
+  FailingCommandList::const_iterator CI;
+  const FailingCommandList::const_iterator CE = FailingCommands.end();
+  for (CI = FailingCommands.begin(); CI != CE; ++CI) {
+    if (A == &(CI->second->getSource()))
+      return true;
+  }
+
+  Action::const_iterator AI;
+  const Action::const_iterator AE = A->end();
+  for (AI = A->begin(); AI != AE; ++AI) {
+    if (ActionFailed(*AI, FailingCommands))
+      return true;
+  }
+
+  return false;
+}
+
+static bool InputsOk(const Command &C,
+                     const FailingCommandList &FailingCommands) {
+  return !ActionFailed(&C.getSource(), FailingCommands);
+}
+
 void Compilation::ExecuteJob(const Job &J,
-    SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands) const 
{
+                             FailingCommandList &FailingCommands) const {
   if (const Command *C = dyn_cast<Command>(&J)) {
+    if (!InputsOk(*C, FailingCommands))
+      return;
     const Command *FailingCommand = 0;
     if (int Res = ExecuteCommand(*C, FailingCommand))
       FailingCommands.push_back(std::make_pair(Res, FailingCommand));
diff --git a/test/Driver/inhibit-downstream-commands.c 
b/test/Driver/inhibit-downstream-commands.c
new file mode 100644
index 0000000..522a4bc
--- /dev/null
+++ b/test/Driver/inhibit-downstream-commands.c
@@ -0,0 +1,4 @@
+// RUN: %clang %s 2>&1 | FileCheck %s
+// CHECK: error: unknown type name 'invalid'
+// CHECK-NOT: clang: error: linker command failed
+invalid C code!
-- 
1.7.8.3

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to