Merge pull request #15531 from graydon/sort-jobs-dont-assert

Author: swift-ci

lib/Driver/ToolChain.cpp

@@ -280,14 +280,17 @@ mergeBatchInputs(ArrayRef<const Job *> jobs,
   return false;
 }
 
-/// Debugging only: return whether the set of \p jobs is an ordered subsequence
-/// of the sequence of top-level input files in the \c Compilation \p C.
-static bool
-jobsAreSubsequenceOfCompilationInputs(ArrayRef<const Job *> jobs,
-                                      Compilation &C) {
-  llvm::SmallVector<const Job *, 16> sortedJobs;
+/// Unfortunately the success or failure of a Swift compilation is currently
+/// sensitive to the order in which files are processed, at least in terms of
+/// the order of processing extensions (and likely other ways we haven't
+/// discovered yet). So long as this is true, we need to make sure any batch job
+/// we build names its inputs in an order that's a subsequence of the sequence
+/// of inputs the driver was initially invoked with.
+static void sortJobsToMatchCompilationInputs(ArrayRef<const Job *> unsortedJobs,
+                                             SmallVectorImpl<const Job *> &sortedJobs,
+                                             Compilation &C) {
   llvm::StringMap<const Job *> jobsByInput;
-  for (const Job *J : jobs) {
+  for (const Job *J : unsortedJobs) {
     const CompileJobAction *CJA = cast<CompileJobAction>(&J->getSource());
     const InputAction* IA = findSingleSwiftInput(CJA);
     auto R = jobsByInput.insert(std::make_pair(IA->getInputArg().getValue(),
@@ -300,40 +303,34 @@ jobsAreSubsequenceOfCompilationInputs(ArrayRef<const Job *> jobs,
       sortedJobs.push_back(I->second);
     }
   }
-  if (sortedJobs.size() != jobs.size())
-    return false;
-  for (size_t i = 0; i < sortedJobs.size(); ++i) {
-    if (sortedJobs[i] != jobs[i])
-      return false;
-  }
-  return true;
 }
 
 /// Construct a \c BatchJob by merging the constituent \p jobs' CommandOutput,
 /// input \c Job and \c Action members. Call through to \c constructInvocation
 /// on \p BatchJob, to build the \c InvocationInfo.
 std::unique_ptr<Job>
-ToolChain::constructBatchJob(ArrayRef<const Job *> jobs,
+ToolChain::constructBatchJob(ArrayRef<const Job *> unsortedJobs,
                              Compilation &C) const
 {
-  if (jobs.empty())
+  if (unsortedJobs.empty())
     return nullptr;
 
-  assert(jobsAreSubsequenceOfCompilationInputs(jobs, C));
+  llvm::SmallVector<const Job *, 16> sortedJobs;
+  sortJobsToMatchCompilationInputs(unsortedJobs, sortedJobs, C);
 
   // Synthetic OutputInfo is a slightly-modified version of the initial
   // compilation's OI.
   auto OI = C.getOutputInfo();
   OI.CompilerMode = OutputInfo::Mode::BatchModeCompile;
 
-  auto const *executablePath = jobs[0]->getExecutable();
-  auto outputType = jobs[0]->getOutput().getPrimaryOutputType();
-  auto output = makeBatchCommandOutput(jobs, C, outputType);
+  auto const *executablePath = sortedJobs[0]->getExecutable();
+  auto outputType = sortedJobs[0]->getOutput().getPrimaryOutputType();
+  auto output = makeBatchCommandOutput(sortedJobs, C, outputType);
 
   llvm::SmallSetVector<const Job *, 16> inputJobs;
   llvm::SmallSetVector<const Action *, 16> inputActions;
   auto *batchCJA = C.createAction<CompileJobAction>(outputType);
-  if (mergeBatchInputs(jobs, inputJobs, inputActions, batchCJA))
+  if (mergeBatchInputs(sortedJobs, inputJobs, inputActions, batchCJA))
     return nullptr;
 
   JobContext context{C, inputJobs.getArrayRef(), inputActions.getArrayRef(),
@@ -346,7 +343,7 @@ ToolChain::constructBatchJob(ArrayRef<const Job *> jobs,
                                      std::move(invocationInfo.Arguments),
                                      std::move(invocationInfo.ExtraEnvironment),
                                      std::move(invocationInfo.FilelistInfos),
-                                     jobs);
+                                     sortedJobs);
 }
 
 bool

test/Driver/Inputs/abcd_filemap.yaml

@@ -0,0 +1,27 @@
+"":
+  swift-dependencies: "master.yaml"
+"a.swift":
+  object: "a.o"
+  dependencies: "a.d"
+  diagnostics: "a.dia"
+  swift-dependencies: "a.swiftdeps"
+"b.swift":
+  object: "b.o"
+  dependencies: "b.d"
+  diagnostics: "b.dia"
+  swift-dependencies: "b.swiftdeps"
+"c.swift":
+  object: "c.o"
+  dependencies: "c.d"
+  diagnostics: "c.dia"
+  swift-dependencies: "c.swiftdeps"
+"d.swift":
+  object: "d.o"
+  dependencies: "d.d"
+  diagnostics: "d.dia"
+  swift-dependencies: "d.swiftdeps"
+"main.swift":
+  object: "main.o"
+  dependencies: "main.d"
+  diagnostics: "main.dia"
+  swift-dependencies: "main.swiftdeps"

test/Driver/batch_mode_dependencies_make_wrong_order.swift

@@ -0,0 +1,28 @@
+// RUN: %empty-directory(%t)
+// RUN: echo 'class a {}' >%t/a.swift
+// RUN: echo 'class b : a {}' >%t/b.swift
+// RUN: echo 'class c : b {}' >%t/c.swift
+// RUN: echo 'class d : c {}' >%t/d.swift
+// RUN: echo 'public func main() {}' >%t/main.swift
+//
+// First prime the incremental state, but note that we're building in the d c b a (reverse-alphabetical) order.
+// RUN: (cd %t && %swiftc_driver -enable-batch-mode -incremental -output-file-map %S/Inputs/abcd_filemap.yaml -module-name main -j 1 d.swift c.swift b.swift a.swift main.swift)
+//
+// Now perturb the interface of a.swift and delete its output
+// RUN: echo 'class a { var x : Int = 10 }' >%t/a.swift
+// RUN: rm %t/a.o
+//
+// Now rebuild, which will rebuild a.swift then do a cascading dep-graph invalidation
+// RUN: (cd %t && %swiftc_driver -enable-batch-mode -incremental -output-file-map %S/Inputs/abcd_filemap.yaml -module-name main -j 1 d.swift c.swift b.swift a.swift main.swift -driver-show-incremental -driver-show-job-lifecycle) >%t/out.txt 2>&1
+// RUN: %FileCheck %s <%t/out.txt
+//
+// Check that we saw invalidation happen in alphabetic order
+// CHECK: Queuing because of dependencies discovered later: {compile: b.o <= b.swift}
+// CHECK: Queuing because of dependencies discovered later: {compile: c.o <= c.swift}
+// CHECK: Queuing because of dependencies discovered later: {compile: d.o <= d.swift}
+// CHECK: Batchable: {compile: b.o <= b.swift}
+// CHECK: Batchable: {compile: c.o <= c.swift}
+// CHECK: Batchable: {compile: d.o <= d.swift}
+//
+// But check that we still issued the job in reverse-alphabetic order
+// CHECK: Adding batch job to task queue: {compile: d.o c.o b.o <= d.swift c.swift b.swift}