Improvements thanks to Jordan’s feedback.

Author: David Ungar

include/swift/Driver/DependencyGraph.h

@@ -127,10 +127,7 @@ class DependencyGraphImpl {
   /// In other words, this Job "cascades"; the need to recompile it causes other
   /// recompilations. It is possible that the current code marks things that do
   /// not need to be marked. Nothing would break if that were the case.
-  ///
-  /// "Marked" does NOT mean that any successors in the graph have been
-  /// processed. The traversal routines use "Visited" to avoid endless
-  /// "recursion".
+
   llvm::SmallPtrSet<const void *, 16> Marked;
 
   /// A list of all external dependencies that cannot be resolved from just this
@@ -163,11 +160,7 @@ class DependencyGraphImpl {
     (void)newlyInserted;
   }
 
-  /// Starting at \p node, visit the transitive closure of every dependent node.
-  /// Assume that the starting node is "cascading"; that all dependencies must
-  /// be dirty if the start is dirty. Therefore, mark the start. For each
-  /// visited node, add it to visited, and mark it if it cascades. The start
-  /// node is NOT added to visited.
+  /// See DependencyGraph::markTransitive.
 
   void markTransitive(SmallVectorImpl<const void *> &visited,
                       const void *node, MarkTracerImpl *tracer = nullptr);
@@ -280,6 +273,15 @@ class DependencyGraph : public DependencyGraphImpl {
   ///
   /// If you want to see how each node gets added to \p visited, pass a local
   /// MarkTracer instance to \p tracer.
+  ///
+  /// Assumes that the starting node is "cascading"; that all dependencies must
+  /// be dirty if the start is dirty. Therefore, mark the start. For each
+  /// visited node, add it to visited, and mark it if it cascades. The start
+  /// node is NOT added to visited.
+  ///
+  /// Do not confused "Marked" with "Visited".
+  /// "Marked" does NOT influence the traversal. The traversal routines use
+  /// "Visited" to avoid endless "recursion".
   template <unsigned N>
   void markTransitive(SmallVector<T, N> &visited, T node,
                       MarkTracer *tracer = nullptr) {

include/swift/Driver/Job.h

@@ -230,17 +230,20 @@ class CommandOutput {
 class Job {
 public:
   enum class Condition {
-    // There was no input map, or the map makred this Job as dirty
+    // There was no information about the previous build (i.e., an input map),
+    // or the map marked this Job as dirty
     // but the input didn't change.
     // Be maximally conservative with dependencies.
     Always,
-    // The input changed, or this job was private or [it was not dirty but
+    // The input changed, or this job was non-cascading or [it was not dirty but
     // primary output was missing].
+    // In other words, this job was scheduled as non-cascading in the last build
+    // but didn't get to run.
     RunWithoutCascading,
     // The best case: input didn't change, output exists.
     // Only run if it depends on some other thing that changed.
     CheckDependencies,
-    // Pessimal: run no matter what.
+    // Pessimal: Run no matter what (but may or may not cascade).
     NewlyAdded
   };
 

lib/Driver/Compilation.cpp

@@ -680,11 +680,6 @@ namespace driver {
     }
 
     /// Schedule all jobs we can from the initial list provided by Compilation.
-    ///
-    /// It would probably be safe and simpler to markTransitive on the start
-    /// nodes in the "Always" condition from the start instead of using
-    /// markIntransitive and having later functions call markTransitive. That
-    /// way markInstransitive would be an implemenation detail.
     void scheduleInitialJobs() {
       for (const Job *Cmd : Comp.getJobs()) {
         if (!Comp.getIncrementalBuildEnabled()) {
@@ -724,6 +719,12 @@ namespace driver {
             // Ensure dependents will get recompiled.
             InitialOutOfDateCommands.push_back(Cmd);
             // Mark this job as cascading.
+            //
+            // It would probably be safe and simpler to markTransitive on the
+            // start nodes in the "Always" condition from the start instead of
+            // using markIntransitive and having later functions call
+            // markTransitive. That way markInstransitive would be an
+            // implementation detail.
             DepGraph.markIntransitive(Cmd);
           }
           LLVM_FALLTHROUGH;