Merge pull request #3957 from jrose-apple/swift-3.0-we-can-rebuild-them

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

Author: tkremenek

lib/Driver/Compilation.cpp

@@ -456,6 +456,69 @@ 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:
+          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 +544,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"