[Driver] Make sure to rebuild dependents when a dirty file fails.

lib/Driver/Compilation.cpp

@@ -456,6 +456,70 @@ int Compilation::performJobsImpl() {
         llvm::errs() << Output;
     }
 
+    // In order to handle both old dependencies that have disappeared and new
+    // dependencies that have arisen, we need to reload the dependency file.
+    // Do this whether or not the build succeeded.
+    SmallVector<const Job *, 16> Dependents;
+    if (getIncrementalBuildEnabled()) {
+      const CommandOutput &Output = FinishedCmd->getOutput();
+      StringRef DependenciesFile =
+        Output.getAdditionalOutputForType(types::TY_SwiftDeps);
+      if (!DependenciesFile.empty() &&
+          (ReturnCode == EXIT_SUCCESS || ReturnCode == EXIT_FAILURE)) {
+        bool wasCascading = DepGraph.isMarked(FinishedCmd);
+
+        switch (DepGraph.loadFromPath(FinishedCmd, DependenciesFile)) {
+        case DependencyGraphImpl::LoadResult::HadError:
+          if (ReturnCode == EXIT_SUCCESS) {
+            disableIncrementalBuild();
+            for (const Job *Cmd : DeferredCommands)
+              scheduleCommandIfNecessaryAndPossible(Cmd);
+            DeferredCommands.clear();
+            Dependents.clear();
+          } // else, let the next build handle it.
+          break;
+        case DependencyGraphImpl::LoadResult::UpToDate:
+          if (!wasCascading)
+            break;
+          SWIFT_FALLTHROUGH;
+        case DependencyGraphImpl::LoadResult::AffectsDownstream:
+            llvm::errs() << "DOWNSTREAM " << ReturnCode << "\n";
+          DepGraph.markTransitive(Dependents, FinishedCmd);
+          break;
+        }
+      } else {
+        // If there's a crash, assume the worst.
+        switch (FinishedCmd->getCondition()) {
+        case Job::Condition::NewlyAdded:
+          // The job won't be treated as newly added next time. Conservatively
+          // mark it as affecting other jobs, because some of them may have
+          // completed already.
+          DepGraph.markTransitive(Dependents, FinishedCmd);
+          break;
+        case Job::Condition::Always:
+          // This applies to non-incremental tasks as well, but any incremental
+          // task that shows up here has already been marked.
+          break;
+        case Job::Condition::RunWithoutCascading:
+          // If this file changed, it might have been a non-cascading change and
+          // it might not. Unfortunately, the interface hash has been updated or
+          // compromised, so we don't actually know anymore; we have to
+          // conservatively assume the changes could affect other files.
+          DepGraph.markTransitive(Dependents, FinishedCmd);
+          break;
+        case Job::Condition::CheckDependencies:
+          // If the only reason we're running this is because something else
+          // changed, then we can trust the dependency graph as to whether it's
+          // a cascading or non-cascading change. That is, if whatever /caused/
+          // the error isn't supposed to affect other files, and whatever
+          // /fixes/ the error isn't supposed to affect other files, then
+          // there's no need to recompile any other inputs. If either of those
+          // are false, we /do/ need to recompile other inputs.
+          break;
+        }
+      }
+    }
+
     if (ReturnCode != EXIT_SUCCESS) {
       // The task failed, so return true without performing any further
       // dependency analysis.
@@ -481,39 +545,10 @@ int Compilation::performJobsImpl() {
     // might have been blocked.
     markFinished(FinishedCmd);
 
-    // In order to handle both old dependencies that have disappeared and new
-    // dependencies that have arisen, we need to reload the dependency file.
-    if (getIncrementalBuildEnabled()) {
-      const CommandOutput &Output = FinishedCmd->getOutput();
-      StringRef DependenciesFile =
-        Output.getAdditionalOutputForType(types::TY_SwiftDeps);
-      if (!DependenciesFile.empty()) {
-        SmallVector<const Job *, 16> Dependents;
-        bool wasCascading = DepGraph.isMarked(FinishedCmd);
-
-        switch (DepGraph.loadFromPath(FinishedCmd, DependenciesFile)) {
-        case DependencyGraphImpl::LoadResult::HadError:
-          disableIncrementalBuild();
-          for (const Job *Cmd : DeferredCommands)
-            scheduleCommandIfNecessaryAndPossible(Cmd);
-          DeferredCommands.clear();
-          Dependents.clear();
-          break;
-        case DependencyGraphImpl::LoadResult::UpToDate:
-          if (!wasCascading)
-            break;
-          SWIFT_FALLTHROUGH;
-        case DependencyGraphImpl::LoadResult::AffectsDownstream:
-          DepGraph.markTransitive(Dependents, FinishedCmd);
-          break;
-        }
-
-        for (const Job *Cmd : Dependents) {
-          DeferredCommands.erase(Cmd);
-          noteBuilding(Cmd, "because of dependencies discovered later");
-          scheduleCommandIfNecessaryAndPossible(Cmd);
-        }
-      }
+    for (const Job *Cmd : Dependents) {
+      DeferredCommands.erase(Cmd);
+      noteBuilding(Cmd, "because of dependencies discovered later");
+      scheduleCommandIfNecessaryAndPossible(Cmd);
     }
 
     return TaskFinishedResponse::ContinueExecution;

lib/Driver/Driver.cpp

@@ -1604,36 +1604,40 @@ handleCompileJobCondition(Job *J, CompileJobAction::InputInfo inputInfo,
     return;
   }
 
-  if (!alwaysRebuildDependents) {
-    // Default all non-newly added files to being rebuilt without cascading.
-    J->setCondition(Job::Condition::RunWithoutCascading);
-  }
-
+  bool hasValidModTime = false;
   llvm::sys::fs::file_status inputStatus;
-  if (llvm::sys::fs::status(input, inputStatus))
-    return;
-
-  J->setInputModTime(inputStatus.getLastModificationTime());
-  if (J->getInputModTime() != inputInfo.previousModTime)
-    return;
+  if (!llvm::sys::fs::status(input, inputStatus)) {
+    J->setInputModTime(inputStatus.getLastModificationTime());
+    hasValidModTime = true;
+  }
 
   Job::Condition condition;
-  switch (inputInfo.status) {
-  case CompileJobAction::InputInfo::UpToDate:
-    if (!llvm::sys::fs::exists(J->getOutput().getPrimaryOutputFilename()))
+  if (!hasValidModTime || J->getInputModTime() != inputInfo.previousModTime) {
+    if (alwaysRebuildDependents ||
+        inputInfo.status == CompileJobAction::InputInfo::NeedsCascadingBuild) {
+      condition = Job::Condition::Always;
+    } else {
       condition = Job::Condition::RunWithoutCascading;
-    else
-      condition = Job::Condition::CheckDependencies;
-    break;
-  case CompileJobAction::InputInfo::NeedsCascadingBuild:
-    condition = Job::Condition::Always;
-    break;
-  case CompileJobAction::InputInfo::NeedsNonCascadingBuild:
-    condition = Job::Condition::RunWithoutCascading;
-    break;
-  case CompileJobAction::InputInfo::NewlyAdded:
-    llvm_unreachable("handled above");
+    }
+  } else {
+    switch (inputInfo.status) {
+    case CompileJobAction::InputInfo::UpToDate:
+      if (!llvm::sys::fs::exists(J->getOutput().getPrimaryOutputFilename()))
+        condition = Job::Condition::RunWithoutCascading;
+      else
+        condition = Job::Condition::CheckDependencies;
+      break;
+    case CompileJobAction::InputInfo::NeedsCascadingBuild:
+      condition = Job::Condition::Always;
+      break;
+    case CompileJobAction::InputInfo::NeedsNonCascadingBuild:
+      condition = Job::Condition::RunWithoutCascading;
+      break;
+    case CompileJobAction::InputInfo::NewlyAdded:
+      llvm_unreachable("handled above");
+    }
   }
+
   J->setCondition(condition);
 }
 

test/Driver/Dependencies/Inputs/crash-simple/crash.swift

@@ -0,0 +1,2 @@
+# Dependencies after compilation:
+provides-top-level: [a]

test/Driver/Dependencies/Inputs/crash-simple/main.swift

@@ -0,0 +1,2 @@
+# Dependencies after compilation:
+depends-top-level: [a]

test/Driver/Dependencies/Inputs/crash-simple/other.swift

@@ -0,0 +1,2 @@
+# Dependencies after compilation:
+depends-top-level: [!private a]

test/Driver/Dependencies/Inputs/crash-simple/output.json

@@ -0,0 +1,17 @@
+{
+  "./main.swift": {
+    "object": "./main.o",
+    "swift-dependencies": "./main.swiftdeps"
+  },
+  "./crash.swift": {
+    "object": "./crash.o",
+    "swift-dependencies": "./crash.swiftdeps"
+  },
+  "./other.swift": {
+    "object": "./other.o",
+    "swift-dependencies": "./other.swiftdeps"
+  },
+  "": {
+    "swift-dependencies": "./main~buildrecord.swiftdeps"
+  }  
+}

test/Driver/Dependencies/Inputs/fail-with-bad-deps/bad.swift

@@ -0,0 +1,4 @@
+# Dependencies after compilation:
+provides-top-level: [bad]
+interface-hash: "after"
+garbage: ""

test/Driver/Dependencies/Inputs/fail-with-bad-deps/bad.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+provides-top-level: [bad]
+interface-hash: "before"

test/Driver/Dependencies/Inputs/fail-with-bad-deps/depends-on-bad.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+depends-top-level: [bad]
+interface-hash: "after"

test/Driver/Dependencies/Inputs/fail-with-bad-deps/depends-on-bad.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+depends-top-level: [bad]
+interface-hash: "before"

test/Driver/Dependencies/Inputs/fail-with-bad-deps/depends-on-main.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+depends-top-level: [main]
+interface-hash: "after"

test/Driver/Dependencies/Inputs/fail-with-bad-deps/depends-on-main.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+depends-top-level: [main]
+interface-hash: "before"

test/Driver/Dependencies/Inputs/fail-with-bad-deps/main.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+provides-top-level: [main]
+interface-hash: "after"

test/Driver/Dependencies/Inputs/fail-with-bad-deps/main.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+provides-top-level: [main]
+interface-hash: "before"

test/Driver/Dependencies/Inputs/fail-with-bad-deps/output.json

@@ -0,0 +1,21 @@
+{
+  "./main.swift": {
+    "object": "./main.o",
+    "swift-dependencies": "./main.swiftdeps"
+  },
+  "./bad.swift": {
+    "object": "./bad.o",
+    "swift-dependencies": "./bad.swiftdeps"
+  },
+  "./depends-on-main.swift": {
+    "object": "./depends-on-main.o",
+    "swift-dependencies": "./depends-on-main.swiftdeps"
+  },
+  "./depends-on-bad.swift": {
+    "object": "./depends-on-bad.o",
+    "swift-dependencies": "./depends-on-bad.swiftdeps"
+  },
+  "": {
+    "swift-dependencies": "./main~buildrecord.swiftdeps"
+  }  
+}

test/Driver/Dependencies/Inputs/update-dependencies-bad.py

@@ -11,23 +11,38 @@
 #
 # ----------------------------------------------------------------------------
 #
-# Fails if the input file is named "bad.swift"; otherwise dispatches to
-# update-dependencies.py.
+# Fails if the input file is named "bad.swift" or "crash.swift"; otherwise
+# dispatches to update-dependencies.py. "crash.swift" gives an exit code
+# other than 1.
 #
 # ----------------------------------------------------------------------------
 
 from __future__ import print_function
 
 import os
+import shutil
 import sys
 
 assert sys.argv[1] == '-frontend'
 
 primaryFile = sys.argv[sys.argv.index('-primary-file') + 1]
 
-if os.path.basename(primaryFile) == 'bad.swift':
+if (os.path.basename(primaryFile) == 'bad.swift' or
+        os.path.basename(primaryFile) == 'crash.swift'):
     print("Handled", os.path.basename(primaryFile))
-    sys.exit(1)
+
+    # Replace the dependencies file with the input file.
+    try:
+        depsFile = sys.argv[sys.argv.index(
+            '-emit-reference-dependencies-path') + 1]
+        shutil.copyfile(primaryFile, depsFile)
+    except ValueError:
+        pass
+
+    if os.path.basename(primaryFile) == 'bad.swift':
+        sys.exit(1)
+    else:
+        sys.exit(129)
 
 dir = os.path.dirname(os.path.abspath(__file__))
 execfile(os.path.join(dir, "update-dependencies.py"))

test/Driver/Dependencies/bindings-build-record-options.swift

@@ -1,12 +1,17 @@
 // RUN: rm -rf %t && cp -r %S/Inputs/bindings-build-record/ %t
 // RUN: touch -t 201401240005 %t/*
 
-// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC
+// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC-INITIAL
 
-// MUST-EXEC-NOT: warning
-// MUST-EXEC: inputs: ["./main.swift"], output: {{[{].*[}]}}, condition: run-without-cascading
-// MUST-EXEC: inputs: ["./other.swift"], output: {{[{].*[}]}}, condition: run-without-cascading
-// MUST-EXEC: inputs: ["./yet-another.swift"], output: {{[{].*[}]}}, condition: run-without-cascading
+// MUST-EXEC-INITIAL-NOT: warning
+// MUST-EXEC-INITIAL: inputs: ["./main.swift"], output: {{[{].*[}]}}, condition: run-without-cascading
+// MUST-EXEC-INITIAL: inputs: ["./other.swift"], output: {{[{].*[}]}}, condition: run-without-cascading
+// MUST-EXEC-INITIAL: inputs: ["./yet-another.swift"], output: {{[{].*[}]}}, condition: run-without-cascading
+
+// MUST-EXEC-ALL-NOT: warning
+// MUST-EXEC-ALL: inputs: ["./main.swift"], output: {{[{].*[}]$}}
+// MUST-EXEC-ALL: inputs: ["./other.swift"], output: {{[{].*[}]$}}
+// MUST-EXEC-ALL: inputs: ["./yet-another.swift"], output: {{[{].*[}]$}}
 
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-use-frontend-path %S/Inputs/update-dependencies.py ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
@@ -19,28 +24,28 @@
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings -serialize-diagnostics ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 
-// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -O -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC
+// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -O -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC-ALL
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-use-frontend-path %S/Inputs/update-dependencies.py ./main.swift ./other.swift ./yet-another.swift -incremental -O -output-file-map %t/output.json
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -O -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -O -serialize-diagnostics -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 
-// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -Onone -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC
+// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -Onone -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC-ALL
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-use-frontend-path %S/Inputs/update-dependencies.py ./main.swift ./other.swift ./yet-another.swift -incremental -Onone -output-file-map %t/output.json
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -Onone -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 
-// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC
+// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC-ALL
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-use-frontend-path %S/Inputs/update-dependencies.py ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 
-// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC
+// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC-ALL
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-use-frontend-path %S/Inputs/update-dependencies.py ./main.swift ./other.swift ./yet-another.swift -incremental -I. -output-file-map %t/output.json
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 
-// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -I/ -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC
+// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -I/ -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC-ALL
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-use-frontend-path %S/Inputs/update-dependencies.py ./main.swift ./other.swift ./yet-another.swift -incremental -I. -I/ -output-file-map %t/output.json
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -I/ -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 
-// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -DDEBUG -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC
+// RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -DDEBUG -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC-ALL
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-use-frontend-path %S/Inputs/update-dependencies.py ./main.swift ./other.swift ./yet-another.swift -incremental -I. -DDEBUG -output-file-map %t/output.json
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -I. -DDEBUG -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC
 // RUN: cd %t && %swiftc_driver -c -module-name main -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -DDEBUG -I. -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=NO-EXEC

test/Driver/Dependencies/bindings-build-record.swift

@@ -37,10 +37,15 @@
 
 // RUN: %S/Inputs/touch.py 443865900 %t/*
 // RUN: cd %t && %swiftc_driver -driver-print-bindings ./main.swift ./other.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=FILE-REMOVED
-// FILE-REMOVED: inputs: ["./main.swift"], output: {{[{].*[}]}}, condition: run-without-cascading
-// FILE-REMOVED: inputs: ["./other.swift"], output: {{[{].*[}]}}, condition: run-without-cascading
+// FILE-REMOVED: inputs: ["./main.swift"], output: {{[{].*[}]$}}
+// FILE-REMOVED: inputs: ["./other.swift"], output: {{[{].*[}]$}}
 // FILE-REMOVED-NOT: yet-another.swift
 
 
 // RUN: echo '{version: "bogus", inputs: {"./main.swift": [443865900, 0], "./other.swift": !private [443865900, 0], "./yet-another.swift": !dirty [443865900, 0]}}' > %t/main~buildrecord.swiftdeps
-// RUN: cd %t && %swiftc_driver -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=MUST-EXEC
+// RUN: cd %t && %swiftc_driver -driver-print-bindings ./main.swift ./other.swift ./yet-another.swift -incremental -output-file-map %t/output.json 2>&1 | FileCheck %s -check-prefix=INVALID-RECORD
+
+// INVALID-RECORD-NOT: warning
+// INVALID-RECORD: inputs: ["./main.swift"], output: {{[{].*[}]$}}
+// INVALID-RECORD: inputs: ["./other.swift"], output: {{[{].*[}]$}}
+// INVALID-RECORD: inputs: ["./yet-another.swift"], output: {{[{].*[}]$}}