[Driver] Handle dependencies we learn about after a job is run.

Specifically, we care about the case where a job is run because of a private
dependency, and then a non-private dependency turns out to be dirty. In
this case, we still need to make sure to build all downstream files.

With this the driver support for private dependencies should be complete
and correct.

Swift SVN r23853

include/swift/Driver/DependencyGraph.h

@@ -48,11 +48,11 @@ public:
 
     /// The file was loaded successfully; with current information the node
     /// does not need to be rebuilt.
-    Valid,
+    UpToDate,
 
-    /// The file was loaded successfully; with current information the node
-    /// should be marked and rebuilt.
-    NeedsRebuilding
+    /// The file was loaded successfully; anything that depends on the node
+    /// should be considered out of date.
+    AffectsDownstream
   };
 
 private:
@@ -83,16 +83,18 @@ private:
   llvm::DenseMap<const void *, std::vector<ProvidesEntryTy>> Provides;
 
   /// The "incoming" edge map. Semantically this maps incoming (kind, string)
-  /// edges representing dependencies to the nodes that depend on them.
+  /// edges representing dependencies to the nodes that depend on them, as
+  /// well as a flag marking whether that (kind, string) pair has been marked
+  /// dirty.
   ///
-  /// The representation is a map from strings to kind mask / node pairs. This
-  /// is because it is unusual (though not impossible) for dependencies of
-  /// different kinds to have the same strings. In the case of multiple
-  /// incoming edges with the same string, the kinds are combined into the one
-  /// field.
+  /// The representation is a map from strings to kind mask / node pairs, plus
+  /// a mask of kinds that have been marked dirty. This is because it is
+  /// unusual (though not impossible) for dependencies of different kinds to
+  /// have the same strings. In the case of multiple incoming edges with the
+  /// same string, the kinds are combined into the one field.
   ///
   /// \sa DependencyMaskTy
-  llvm::StringMap<std::vector<DependencyEntryTy>> Dependencies;
+  llvm::StringMap<std::pair<std::vector<DependencyEntryTy>, DependencyMaskTy>> Dependencies;
 
   /// The set of marked nodes.
   llvm::SmallPtrSet<const void *, 16> Marked;
@@ -124,6 +126,9 @@ protected:
 /// with other nodes' "depends" sets to form a traversable directed graph.
 /// Information on a particular node can be updated at any time, which will
 /// affect any following operations. The "depends" entries can be "private".
+///
+/// The graph also supports a "mark" operation, which is intended to track
+/// nodes that have been not just visited but transitively marked through.
 template <typename T>
 class DependencyGraph : public DependencyGraphImpl {
   using Traits = llvm::PointerLikeTypeTraits<T>;

lib/Driver/Compilation.cpp

@@ -14,6 +14,7 @@
 
 #include "swift/AST/DiagnosticEngine.h"
 #include "swift/AST/DiagnosticsDriver.h"
+#include "swift/Basic/Fallthrough.h"
 #include "swift/Basic/Program.h"
 #include "swift/Basic/TaskQueue.h"
 #include "swift/Driver/Action.h"
@@ -135,10 +136,10 @@ int Compilation::performJobsInList(const JobList &JL, PerformJobsState &State) {
       case DependencyGraphImpl::LoadResult::HadError:
         NeedToRunEverything = true;
         break;
-      case DependencyGraphImpl::LoadResult::Valid:
+      case DependencyGraphImpl::LoadResult::UpToDate:
         Condition = Cmd->getCondition();
         break;
-      case DependencyGraphImpl::LoadResult::NeedsRebuilding:
+      case DependencyGraphImpl::LoadResult::AffectsDownstream:
         llvm_unreachable("we haven't marked anything in this graph yet");
       }
     }
@@ -244,10 +245,11 @@ int Compilation::performJobsInList(const JobList &JL, PerformJobsState &State) {
           DeferredCommands.clear();
           Dependents.clear();
           break;
-        case DependencyGraphImpl::LoadResult::NeedsRebuilding:
-          llvm_unreachable("currently unused");
-        case DependencyGraphImpl::LoadResult::Valid:
-          if (wasNonPrivate)
+        case DependencyGraphImpl::LoadResult::UpToDate:
+          if (!wasNonPrivate)
+            break;
+          SWIFT_FALLTHROUGH;
+        case DependencyGraphImpl::LoadResult::AffectsDownstream:
           DepGraph.markTransitive(Dependents, FinishedCmd);
           break;
         }

lib/Driver/DependencyGraph.cpp

@@ -49,11 +49,11 @@ parseDependencyFile(llvm::MemoryBuffer &buffer,
   auto *topLevelMap = dyn_cast<yaml::MappingNode>(I->getRoot());
   if (!topLevelMap) {
     if (isa<yaml::NullNode>(I->getRoot()))
-      return LoadResult::Valid;
+      return LoadResult::UpToDate;
     return LoadResult::HadError;
   }
 
-  LoadResult result = LoadResult::Valid;
+  LoadResult result = LoadResult::UpToDate;
   SmallString<64> scratch;
   // FIXME: LLVM's YAML support does incremental parsing in such a way that
   // for-range loops break.
@@ -91,10 +91,10 @@ parseDependencyFile(llvm::MemoryBuffer &buffer,
                        entry->getRawTag() == "!private")) {
       case LoadResult::HadError:
         return LoadResult::HadError;
-      case LoadResult::Valid:
+      case LoadResult::UpToDate:
         break;
-      case LoadResult::NeedsRebuilding:
-        result = LoadResult::NeedsRebuilding;
+      case LoadResult::AffectsDownstream:
+        result = LoadResult::AffectsDownstream;
         break;
       }
     }
@@ -124,7 +124,7 @@ LoadResult DependencyGraphImpl::loadFromBuffer(const void *node,
                                       bool isPrivate) -> LoadResult {
 
     auto &entries = Dependencies[name];
-    auto iter = std::find_if(entries.begin(), entries.end(),
+    auto iter = std::find_if(entries.first.begin(), entries.first.end(),
                              [node](const DependencyEntryTy &entry) -> bool {
       return node == entry.node;
     });
@@ -133,16 +133,16 @@ LoadResult DependencyGraphImpl::loadFromBuffer(const void *node,
     if (!isPrivate)
       flags |= DependencyFlags::IsNonPrivate;
 
-    if (iter == entries.end()) {
-      entries.push_back({node, kind, flags});
+    if (iter == entries.first.end()) {
+      entries.first.push_back({node, kind, flags});
     } else {
       iter->kindMask |= kind;
       iter->flags |= flags;
     }
 
-    // FIXME: This should return NeedsRebuilding if the dependency has already
-    // been marked.
-    return LoadResult::Valid;
+    if (!isPrivate && (entries.second & kind))
+      return LoadResult::AffectsDownstream;
+    return LoadResult::UpToDate;
   };
 
   auto providesCallback =
@@ -159,7 +159,7 @@ LoadResult DependencyGraphImpl::loadFromBuffer(const void *node,
     else
       iter->kindMask |= kind;
 
-    return LoadResult::Valid;
+    return LoadResult::UpToDate;
   };
 
   return parseDependencyFile(buffer, providesCallback, dependsCallback);
@@ -181,7 +181,10 @@ DependencyGraphImpl::markTransitive(SmallVectorImpl<const void *> &visited,
       if (allDependents == Dependencies.end())
         continue;
 
-      for (const auto &dependent : allDependents->second) {
+      // Record that we've traversed this dependency.
+      allDependents->second.second |= provided.kindMask;
+
+      for (const auto &dependent : allDependents->second.first) {
         if (!(provided.kindMask & dependent.kindMask))
           continue;
         if (isMarked(dependent.node))

test/Driver/Dependencies/Inputs/chained-private-after/main.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+top-level: [a]
+nominals: [b]

test/Driver/Dependencies/Inputs/chained-private-after/main.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+top-level: [!private a]
+nominals: [b]

test/Driver/Dependencies/Inputs/chained-private-after/other.swift

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

test/Driver/Dependencies/Inputs/chained-private-after/other.swiftdeps

@@ -0,0 +1,2 @@
+# Dependencies before compilation:
+provides: [a]

test/Driver/Dependencies/Inputs/chained-private-after/output.json

@@ -0,0 +1,14 @@
+{
+  "./main.swift": {
+    "object": "./main.o",
+    "swift-dependencies": "./main.swiftdeps"
+  },
+  "./other.swift": {
+    "object": "./other.o",
+    "swift-dependencies": "./other.swiftdeps"
+  },
+  "./yet-another.swift": {
+    "object": "./yet-another.o",
+    "swift-dependencies": "./yet-another.swiftdeps"
+  }
+}

test/Driver/Dependencies/Inputs/chained-private-after/yet-another.swift

@@ -0,0 +1,2 @@
+# Dependencies after compilation:
+member-access: [b]

test/Driver/Dependencies/Inputs/chained-private-after/yet-another.swiftdeps

@@ -0,0 +1,2 @@
+# Dependencies before compilation:
+member-access: [b]

test/Driver/Dependencies/Inputs/private-after/a.swift

@@ -0,0 +1,2 @@
+# Dependencies after compilation:
+nominals: [a]

test/Driver/Dependencies/Inputs/private-after/a.swiftdeps

@@ -0,0 +1,2 @@
+# Dependencies before compilation:
+nominals: [a]

test/Driver/Dependencies/Inputs/private-after/b.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+nominals: [b]
+member-access: [!private a, e]

test/Driver/Dependencies/Inputs/private-after/b.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+nominals: [b]
+member-access: [!private a]

test/Driver/Dependencies/Inputs/private-after/c.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+nominals: [c]
+member-access: [b]

test/Driver/Dependencies/Inputs/private-after/c.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+nominals: [c]
+member-access: [b]

test/Driver/Dependencies/Inputs/private-after/d.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+nominals: [d]
+member-access: [a]

test/Driver/Dependencies/Inputs/private-after/d.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+nominals: []
+member-access: [a]

test/Driver/Dependencies/Inputs/private-after/e.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+nominals: [e]
+member-access: [d]

test/Driver/Dependencies/Inputs/private-after/e.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+nominals: []
+member-access: [d]

test/Driver/Dependencies/Inputs/private-after/f.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+nominals: [f]
+member-access: [!private e]

test/Driver/Dependencies/Inputs/private-after/f.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+nominals: [f]
+member-access: [!private e]

test/Driver/Dependencies/Inputs/private-after/g.swift

@@ -0,0 +1,3 @@
+# Dependencies after compilation:
+nominals: [g]
+member-access: [f]

test/Driver/Dependencies/Inputs/private-after/g.swiftdeps

@@ -0,0 +1,3 @@
+# Dependencies before compilation:
+nominals: [g]
+member-access: [f]

test/Driver/Dependencies/Inputs/private-after/output.json

@@ -0,0 +1,30 @@
+{
+  "./a.swift": {
+    "object": "./a.o",
+    "swift-dependencies": "./a.swiftdeps"
+  },
+  "./b.swift": {
+    "object": "./b.o",
+    "swift-dependencies": "./b.swiftdeps"
+  },
+  "./c.swift": {
+    "object": "./c.o",
+    "swift-dependencies": "./c.swiftdeps"
+  },
+  "./d.swift": {
+    "object": "./d.o",
+    "swift-dependencies": "./d.swiftdeps"
+  },
+  "./e.swift": {
+    "object": "./e.o",
+    "swift-dependencies": "./e.swiftdeps"
+  },
+  "./f.swift": {
+    "object": "./f.o",
+    "swift-dependencies": "./f.swiftdeps"
+  },
+  "./g.swift": {
+    "object": "./g.o",
+    "swift-dependencies": "./g.swiftdeps"
+  },
+}

test/Driver/Dependencies/chained-private-after.swift

@@ -0,0 +1,14 @@
+// RUN: rm -rf %t && cp -r %S/Inputs/chained-private-after/ %t
+// RUN: touch -t 201401240005 %t/*.swift
+// RUN: touch -t 201401240006 %t/*.o
+
+// RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path %S/Inputs/update-dependencies.py -output-file-map %t/output.json -incremental ./main.swift ./other.swift ./yet-another.swift -module-name main -j1 -v 2>&1 | FileCheck -check-prefix=CHECK-FIRST %s
+
+// CHECK-FIRST-NOT: Handled
+
+// RUN: rm %t/other.o
+// RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path %S/Inputs/update-dependencies.py -output-file-map %t/output.json -incremental ./yet-another.swift ./main.swift ./other.swift -module-name main -j1 -v 2>&1 | FileCheck -check-prefix=CHECK-SECOND %s
+
+// CHECK-SECOND: Handled other.swift
+// CHECK-SECOND: Handled main.swift
+// CHECK-SECOND: Handled yet-another.swift

test/Driver/Dependencies/private-after.swift

@@ -0,0 +1,38 @@
+// RUN: rm -rf %t && cp -r %S/Inputs/private-after/ %t
+// RUN: touch -t 201401240005 %t/*.swift
+// RUN: touch -t 201401240006 %t/*.o
+
+// RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path %S/Inputs/update-dependencies.py -output-file-map %t/output.json -incremental ./a.swift ./b.swift ./c.swift ./d.swift ./e.swift ./f.swift ./g.swift -module-name main -j1 -v 2>&1 | FileCheck -check-prefix=CHECK-INITIAL %s
+
+// CHECK-INITIAL-NOT: Handled
+
+// RUN: rm %t/a.o
+// RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path %S/Inputs/update-dependencies.py -output-file-map %t/output.json -incremental ./a.swift ./b.swift ./c.swift ./d.swift ./e.swift ./f.swift ./g.swift -module-name main -j1 -v > %t/a.txt 2>&1
+// RUN: FileCheck -check-prefix=CHECK-A %s < %t/a.txt
+// RUN: FileCheck -check-prefix=CHECK-A-NEG %s < %t/a.txt
+
+// CHECK-A: Handled a.swift
+// CHECK-A-DAG: Handled b.swift
+// CHECK-A-DAG: Handled d.swift
+// CHECK-A: Handled e.swift
+// CHECK-A-DAG: Handled c.swift
+// CHECK-A-DAG: Handled f.swift
+// CHECK-A-NEG-NOT: Handled g.swift
+
+
+// RUN: rm -rf %t && cp -r %S/Inputs/private-after/ %t
+// RUN: touch -t 201401240005 %t/*.swift
+// RUN: touch -t 201401240006 %t/*.o
+
+// RUN: rm %t/f.o
+// RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path %S/Inputs/update-dependencies.py -output-file-map %t/output.json -incremental ./a.swift ./b.swift ./c.swift ./d.swift ./e.swift ./f.swift ./g.swift -module-name main -j1 -v > %t/f.txt 2>&1
+// RUN: FileCheck -check-prefix=CHECK-F %s < %t/f.txt
+// RUN: FileCheck -check-prefix=CHECK-F-NEG %s < %t/f.txt
+
+// CHECK-F: Handled f.swift
+// CHECK-F: Handled g.swift
+// CHECK-F-NEG-NOT: Handled a.swift
+// CHECK-F-NEG-NOT: Handled b.swift
+// CHECK-F-NEG-NOT: Handled c.swift
+// CHECK-F-NEG-NOT: Handled d.swift
+// CHECK-F-NEG-NOT: Handled e.swift