[Dependency Scanning] Refactor 'createScanQueryContext' to directly return an error

include/swift/DependencyScan/DependencyScanningTool.h

@@ -30,7 +30,7 @@ class DepScanInMemoryDiagnosticCollector;
 
 struct ScanQueryContext {
   /// Primary CompilerInstance configured for this scanning action
-  llvm::ErrorOr<std::unique_ptr<CompilerInstance>> ScanInstance;
+  std::unique_ptr<CompilerInstance> ScanInstance;
   /// An thread-safe diagnostic consumer which collects all emitted
   /// diagnostics in the scan to be reporte via libSwiftScan API
   std::unique_ptr<DepScanInMemoryDiagnosticCollector> InMemoryDiagnosticCollector;
@@ -39,6 +39,22 @@ struct ScanQueryContext {
   /// 'ThreadSafeSerializedDiagnosticConsumer'
   std::unique_ptr<DiagnosticConsumer> SerializedDiagnosticConsumer;
 
+  ScanQueryContext(
+      std::unique_ptr<CompilerInstance> ScanInstance,
+      std::unique_ptr<DepScanInMemoryDiagnosticCollector>
+          InMemoryDiagnosticCollector,
+      std::unique_ptr<DiagnosticConsumer> SerializedDiagnosticConsumer)
+      : ScanInstance(std::move(ScanInstance)),
+        InMemoryDiagnosticCollector(std::move(InMemoryDiagnosticCollector)),
+        SerializedDiagnosticConsumer(std::move(SerializedDiagnosticConsumer)) {}
+
+  ScanQueryContext(ScanQueryContext &&other)
+      : ScanInstance(std::move(other.ScanInstance)),
+        InMemoryDiagnosticCollector(
+            std::move(other.InMemoryDiagnosticCollector)),
+        SerializedDiagnosticConsumer(
+            std::move(other.SerializedDiagnosticConsumer)) {}
+
   ~ScanQueryContext() {
     if (SerializedDiagnosticConsumer)
       SerializedDiagnosticConsumer->finishProcessing();
@@ -65,7 +81,7 @@ public:
 /// which may have been used to emit the diagnostic
 class DepScanInMemoryDiagnosticCollector
     : public ThreadSafeDiagnosticCollector {
-private:
+public:
   struct ScannerDiagnosticInfo {
     std::string Message;
     llvm::SourceMgr::DiagKind Severity;
@@ -81,7 +97,10 @@ public:
   friend DependencyScanningTool;
   DepScanInMemoryDiagnosticCollector() {}
   void reset() { Diagnostics.clear(); }
-  const std::vector<ScannerDiagnosticInfo> &getDiagnostics() const {
+  std::vector<ScannerDiagnosticInfo> getDiagnostics() const {
+    return Diagnostics;
+  }
+  const std::vector<ScannerDiagnosticInfo> &getDiagnosticsRef() const {
     return Diagnostics;
   }
 };
@@ -115,9 +134,10 @@ public:
 
   /// Using the specified invocation command, instantiate a CompilerInstance
   /// that will be used for this scan.
-  ScanQueryContext
-  createScanQueryContext(ArrayRef<const char *> Command,
-                         StringRef WorkingDirectory);
+  llvm::ErrorOr<ScanQueryContext> createScanQueryContext(
+      ArrayRef<const char *> Command, StringRef WorkingDirectory,
+      std::vector<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+          &initializationDiagnostics);
 
 private:
   /// Shared cache of module dependencies, re-used by individual full-scan queries
@@ -130,7 +150,9 @@ private:
   llvm::StringSaver Saver;
 };
 
-swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(const DepScanInMemoryDiagnosticCollector *diagnosticCollector);
+swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(
+    ArrayRef<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+        diagnostics);
 
 } // end namespace dependencies
 } // end namespace swift

lib/DependencyScan/DependencyScanningTool.cpp

@@ -15,6 +15,7 @@
 #include "swift/Basic/LLVMInitialize.h"
 #include "swift/Basic/TargetInfo.h"
 #include "swift/Basic/ColorUtils.h"
+#include "swift/Basic/Defer.h"
 #include "swift/DependencyScan/DependencyScanningTool.h"
 #include "swift/DependencyScan/DependencyScanImpl.h"
 #include "swift/DependencyScan/SerializedModuleDependencyCacheFormat.h"
@@ -131,15 +132,15 @@ void DepScanInMemoryDiagnosticCollector::addDiagnostic(
 }
 
 swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(
-    const DepScanInMemoryDiagnosticCollector *diagnosticCollector) {
-  auto collectedDiagnostics = diagnosticCollector->getDiagnostics();
-  auto numDiagnostics = collectedDiagnostics.size();
+    ArrayRef<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+        diagnostics) {
+  auto numDiagnostics = diagnostics.size();
   swiftscan_diagnostic_set_t *diagnosticOutput = new swiftscan_diagnostic_set_t;
   diagnosticOutput->count = numDiagnostics;
   diagnosticOutput->diagnostics =
       new swiftscan_diagnostic_info_t[numDiagnostics];
   for (size_t i = 0; i < numDiagnostics; ++i) {
-    const auto &Diagnostic = collectedDiagnostics[i];
+    const auto &Diagnostic = diagnostics[i];
     swiftscan_diagnostic_info_s *diagnosticInfo =
         new swiftscan_diagnostic_info_s;
     diagnosticInfo->message =
@@ -183,7 +184,8 @@ swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(
 // module dependnecies but captures the diagnostics emitted during the attempted
 // scan query.
 static swiftscan_dependency_graph_t generateHollowDiagnosticOutput(
-    const DepScanInMemoryDiagnosticCollector &ScanDiagnosticConsumer) {
+    ArrayRef<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+        diagnostics) {
   // Create a dependency graph instance
   swiftscan_dependency_graph_t hollowResult = new swiftscan_dependency_graph_s;
 
@@ -246,7 +248,7 @@ static swiftscan_dependency_graph_t generateHollowDiagnosticOutput(
 
   // Populate the diagnostic info
   hollowResult->diagnostics =
-      mapCollectedDiagnosticsForOutput(&ScanDiagnosticConsumer);
+      mapCollectedDiagnosticsForOutput(diagnostics);
   return hollowResult;
 }
 
@@ -254,12 +256,13 @@ static swiftscan_dependency_graph_t generateHollowDiagnosticOutput(
 // imports but captures the diagnostics emitted during the attempted
 // scan query.
 static swiftscan_import_set_t generateHollowDiagnosticOutputImportSet(
-    const DepScanInMemoryDiagnosticCollector &ScanDiagnosticConsumer) {
+    ArrayRef<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+        diagnostics) {
   // Create an dependency graph instance
   swiftscan_import_set_t hollowResult = new swiftscan_import_set_s;
   hollowResult->imports = c_string_utils::create_empty_set();
   hollowResult->diagnostics =
-      mapCollectedDiagnosticsForOutput(&ScanDiagnosticConsumer);
+    mapCollectedDiagnosticsForOutput(diagnostics);
   return hollowResult;
 }
 
@@ -270,12 +273,17 @@ DependencyScanningTool::DependencyScanningTool()
 llvm::ErrorOr<swiftscan_dependency_graph_t>
 DependencyScanningTool::getDependencies(ArrayRef<const char *> Command,
                                         StringRef WorkingDirectory) {
+  // Diagnostics which may get collected during scanning instance
+  // initialization
+  std::vector<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+      InitializationDiagnostics;
+
   // The primary instance used to scan the query Swift source-code
-  auto QueryContext = createScanQueryContext(Command, WorkingDirectory);
-  if (QueryContext.ScanInstance.getError())
-    return generateHollowDiagnosticOutput(
-        *QueryContext.InMemoryDiagnosticCollector);
-  auto ScanInstance = QueryContext.ScanInstance->get();
+  auto QueryContext = createScanQueryContext(Command, WorkingDirectory,
+                                             InitializationDiagnostics);
+  if (QueryContext.getError())
+    return generateHollowDiagnosticOutput(InitializationDiagnostics);
+  auto ScanInstance = QueryContext->ScanInstance.get();
 
   // Local scan cache instance, wrapping the shared global cache.
   ModuleDependenciesCache cache(
@@ -283,11 +291,11 @@ DependencyScanningTool::getDependencies(ArrayRef<const char *> Command,
       ScanInstance->getInvocation().getModuleScanningHash());
   // Execute the scanning action, retrieving the in-memory result
   auto DependenciesOrErr =
-      performModuleScan(*ScanningService, cache, QueryContext);
+      performModuleScan(*ScanningService, cache, *QueryContext);
 
   if (DependenciesOrErr.getError())
     return generateHollowDiagnosticOutput(
-        *QueryContext.InMemoryDiagnosticCollector);
+        QueryContext->InMemoryDiagnosticCollector->getDiagnosticsRef());
 
   return std::move(*DependenciesOrErr);
 }
@@ -295,12 +303,17 @@ DependencyScanningTool::getDependencies(ArrayRef<const char *> Command,
 llvm::ErrorOr<swiftscan_import_set_t>
 DependencyScanningTool::getImports(ArrayRef<const char *> Command,
                                    StringRef WorkingDirectory) {
+  // Diagnostics which may get collected during scanning instance
+  // initialization
+  std::vector<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+      InitializationDiagnostics;
+
   // The primary instance used to scan the query Swift source-code
-  auto QueryContext = createScanQueryContext(Command, WorkingDirectory);
-  if (QueryContext.ScanInstance.getError())
-    return generateHollowDiagnosticOutputImportSet(
-        *QueryContext.InMemoryDiagnosticCollector);
-  auto ScanInstance = QueryContext.ScanInstance->get();
+  auto QueryContext = createScanQueryContext(Command, WorkingDirectory,
+                                             InitializationDiagnostics);
+  if (QueryContext.getError())
+    return generateHollowDiagnosticOutputImportSet(InitializationDiagnostics);
+  auto ScanInstance = QueryContext->ScanInstance.get();
 
   // Local scan cache instance, wrapping the shared global cache.
   ModuleDependenciesCache cache(
@@ -308,17 +321,19 @@ DependencyScanningTool::getImports(ArrayRef<const char *> Command,
       ScanInstance->getInvocation().getModuleScanningHash());
   // Execute the pre-scanning action, retrieving the in-memory result
   auto DependenciesOrErr =
-      performModulePrescan(*ScanningService, cache, QueryContext);
+      performModulePrescan(*ScanningService, cache, *QueryContext);
 
   if (DependenciesOrErr.getError())
     return generateHollowDiagnosticOutputImportSet(
-        *QueryContext.InMemoryDiagnosticCollector);
+        QueryContext->InMemoryDiagnosticCollector->getDiagnosticsRef());
 
   return std::move(*DependenciesOrErr);
 }
 
-ScanQueryContext DependencyScanningTool::createScanQueryContext(
-    ArrayRef<const char *> CommandArgs, StringRef WorkingDir) {
+llvm::ErrorOr<ScanQueryContext> DependencyScanningTool::createScanQueryContext(
+    ArrayRef<const char *> CommandArgs, StringRef WorkingDir,
+    std::vector<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+        &InitializationDiagnostics) {
   // The remainder of this method operates on shared state in the
   // scanning service
   llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
@@ -336,55 +351,59 @@ ScanQueryContext DependencyScanningTool::createScanQueryContext(
   auto Instance = std::make_unique<CompilerInstance>();
   Instance->addDiagnosticConsumer(ScannerDiagnosticsCollector.get());
 
-  // Basic error checking on the arguments
-  if (CommandArgs.empty()) {
-    Instance->getDiags().diagnose(SourceLoc(), diag::error_no_frontend_args);
-    return ScanQueryContext{std::make_error_code(std::errc::invalid_argument),
-                            std::move(ScannerDiagnosticsCollector), nullptr};
+  {
+    // In case we exit with an error, ensure the client gets the
+    // diagnostics collected so-far.
+    SWIFT_DEFER {
+      InitializationDiagnostics = ScannerDiagnosticsCollector->getDiagnostics();
+    };
+
+    // Basic error checking on the arguments
+    if (CommandArgs.empty()) {
+      Instance->getDiags().diagnose(SourceLoc(), diag::error_no_frontend_args);
+      return std::make_error_code(std::errc::invalid_argument);
+    }
+
+    CompilerInvocation Invocation;
+    SmallString<128> WorkingDirectory(WorkingDir);
+    if (WorkingDirectory.empty())
+      llvm::sys::fs::current_path(WorkingDirectory);
+
+    // Parse/tokenize arguments.
+    std::string CommandString;
+    for (const auto *c : CommandArgs) {
+      CommandString.append(c);
+      CommandString.append(" ");
+    }
+    SmallVector<const char *, 4> Args;
+    llvm::BumpPtrAllocator Alloc;
+    llvm::StringSaver Saver(Alloc);
+    // Ensure that we use the Windows command line parsing on Windows as we need
+    // to ensure that we properly handle paths.
+    if (llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows())
+      llvm::cl::TokenizeWindowsCommandLine(CommandString, Saver, Args);
+    else
+      llvm::cl::TokenizeGNUCommandLine(CommandString, Saver, Args);
+
+    if (Invocation.parseArgs(Args, Instance->getDiags(),
+                             nullptr, WorkingDirectory, "/tmp/foo")) {
+      return std::make_error_code(std::errc::invalid_argument);
+    }
+
+    // Setup the instance
+    std::string InstanceSetupError;
+    if (Instance->setup(Invocation, InstanceSetupError))
+      return std::make_error_code(std::errc::not_supported);
+
+    Invocation.getFrontendOptions().LLVMArgs.clear();
+
+    // Setup the caching service after the instance finishes setup.
+    if (ScanningService->setupCachingDependencyScanningService(*Instance))
+      return std::make_error_code(std::errc::invalid_argument);
+
+    (void)Instance->getMainModule();
   }
 
-  CompilerInvocation Invocation;
-  SmallString<128> WorkingDirectory(WorkingDir);
-  if (WorkingDirectory.empty())
-    llvm::sys::fs::current_path(WorkingDirectory);
-
-  // Parse/tokenize arguments.
-  std::string CommandString;
-  for (const auto *c : CommandArgs) {
-    CommandString.append(c);
-    CommandString.append(" ");
-  }
-  SmallVector<const char *, 4> Args;
-  llvm::BumpPtrAllocator Alloc;
-  llvm::StringSaver Saver(Alloc);
-  // Ensure that we use the Windows command line parsing on Windows as we need
-  // to ensure that we properly handle paths.
-  if (llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows())
-    llvm::cl::TokenizeWindowsCommandLine(CommandString, Saver, Args);
-  else
-    llvm::cl::TokenizeGNUCommandLine(CommandString, Saver, Args);
-
-  if (Invocation.parseArgs(Args, Instance->getDiags(),
-                           nullptr, WorkingDirectory, "/tmp/foo")) {
-    return ScanQueryContext{std::make_error_code(std::errc::invalid_argument),
-                            std::move(ScannerDiagnosticsCollector), nullptr};
-  }
-
-  // Setup the instance
-  std::string InstanceSetupError;
-  if (Instance->setup(Invocation, InstanceSetupError))
-    return ScanQueryContext{std::make_error_code(std::errc::not_supported),
-                            std::move(ScannerDiagnosticsCollector), nullptr};
-
-  Invocation.getFrontendOptions().LLVMArgs.clear();
-
-  // Setup the caching service after the instance finishes setup.
-  if (ScanningService->setupCachingDependencyScanningService(*Instance))
-    return ScanQueryContext{std::make_error_code(std::errc::invalid_argument),
-                            std::move(ScannerDiagnosticsCollector), nullptr};
-
-  (void)Instance->getMainModule();
-
   auto SerializedDiagnosticsOutputPath =
       Instance->getInvocation()
           .getSerializedDiagnosticsPathForAtMostOnePrimary();

lib/DependencyScan/ScanDependencies.cpp

@@ -966,10 +966,10 @@ static swiftscan_dependency_graph_t generateFullDependencyGraph(
   swiftscan_dependency_graph_t result = new swiftscan_dependency_graph_s;
   result->main_module_name = create_clone(mainModuleName.c_str());
   result->dependencies = dependencySet;
-  result->diagnostics =
-      diagnosticCollector
-          ? mapCollectedDiagnosticsForOutput(diagnosticCollector)
-          : nullptr;
+  result->diagnostics = diagnosticCollector
+                            ? mapCollectedDiagnosticsForOutput(
+                                  diagnosticCollector->getDiagnostics())
+                            : nullptr;
   return result;
 }
 
@@ -1452,14 +1452,14 @@ static llvm::ErrorOr<swiftscan_import_set_t> performModulePrescanImpl(
                     return importInfo.importIdentifier;
                   });
   importSet->imports = create_set(importIdentifiers);
-  importSet->diagnostics =
-      diagnosticCollector
-          ? mapCollectedDiagnosticsForOutput(diagnosticCollector)
-          : nullptr;
-  importSet->diagnostics =
-      diagnosticCollector
-          ? mapCollectedDiagnosticsForOutput(diagnosticCollector)
-          : nullptr;
+  importSet->diagnostics = diagnosticCollector
+                               ? mapCollectedDiagnosticsForOutput(
+                                     diagnosticCollector->getDiagnostics())
+                               : nullptr;
+  importSet->diagnostics = diagnosticCollector
+                               ? mapCollectedDiagnosticsForOutput(
+                                     diagnosticCollector->getDiagnostics())
+                               : nullptr;
   return importSet;
 }
 } // namespace
@@ -1551,7 +1551,7 @@ llvm::ErrorOr<swiftscan_dependency_graph_t>
 swift::dependencies::performModuleScan(SwiftDependencyScanningService &service,
                                        ModuleDependenciesCache &cache,
                                        ScanQueryContext &queryContext) {
-  return performModuleScanImpl(service, queryContext.ScanInstance->get(), cache,
+  return performModuleScanImpl(service, queryContext.ScanInstance.get(), cache,
                                queryContext.InMemoryDiagnosticCollector.get());
 }
 
@@ -1559,7 +1559,7 @@ llvm::ErrorOr<swiftscan_import_set_t> swift::dependencies::performModulePrescan(
     SwiftDependencyScanningService &service, ModuleDependenciesCache &cache,
     ScanQueryContext &queryContext) {
   return performModulePrescanImpl(
-      service, queryContext.ScanInstance->get(), cache,
+      service, queryContext.ScanInstance.get(), cache,
       queryContext.InMemoryDiagnosticCollector.get());
 }
 

unittests/DependencyScan/ModuleDeps.cpp

@@ -246,11 +246,15 @@ public func overlayFuncA() { }\n"));
     CommandB.push_back(command.c_str());
   }
 
-  auto queryAContext = ScannerTool.createScanQueryContext(CommandA, {});
-  auto queryBContext = ScannerTool.createScanQueryContext(CommandB, {});
+  std::vector<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+      InitializationDiagnostics;
+  auto queryAContext = ScannerTool.createScanQueryContext(CommandA, {},
+                                                          InitializationDiagnostics);
+  auto queryBContext = ScannerTool.createScanQueryContext(CommandB, {},
+                                                          InitializationDiagnostics);
   // Ensure that scans that only differ in module name have distinct scanning context hashes
-  ASSERT_NE(queryAContext.ScanInstance.get()->getInvocation().getModuleScanningHash(),
-            queryBContext.ScanInstance.get()->getInvocation().getModuleScanningHash());
+  ASSERT_NE(queryAContext->ScanInstance.get()->getInvocation().getModuleScanningHash(),
+            queryBContext->ScanInstance.get()->getInvocation().getModuleScanningHash());
 }
 
 namespace {
@@ -336,16 +340,19 @@ public func funcA() { }\n"));
 
   {
     auto ScanningService = std::make_unique<SwiftDependencyScanningService>();
-    auto QueryContext = ScannerTool.createScanQueryContext(Command, {});
-    ASSERT_FALSE(QueryContext.ScanInstance.getError());
+    std::vector<DepScanInMemoryDiagnosticCollector::ScannerDiagnosticInfo>
+        InitializationDiagnostics;
+    auto QueryContext = ScannerTool.createScanQueryContext(Command, {},
+                                                           InitializationDiagnostics);
+    ASSERT_FALSE(QueryContext.getError());
 
     ModuleDependenciesCache ScanCache(
-        QueryContext.ScanInstance.get()->getMainModule()->getNameStr().str(),
-        QueryContext.ScanInstance.get()
+        QueryContext->ScanInstance.get()->getMainModule()->getNameStr().str(),
+        QueryContext->ScanInstance.get()
             ->getInvocation()
             .getModuleScanningHash());
     auto DependenciesOrErr =
-        performModuleScan(*ScanningService, ScanCache, QueryContext);
+        performModuleScan(*ScanningService, ScanCache, *QueryContext);
 
     ASSERT_FALSE(DependenciesOrErr.getError());
   }