[Dependency Scanning] Collect emitted diagnostics per-scan-query

Author: artemcm

include/swift-c/DependencyScan/DependencyScan.h

@@ -61,6 +61,18 @@ typedef struct {
   size_t count;
 } swiftscan_dependency_set_t;
 
+typedef enum {
+  SWIFTSCAN_DIAGNOSTIC_SEVERITY_ERROR = 0,
+  SWIFTSCAN_DIAGNOSTIC_SEVERITY_WARNING = 1,
+  SWIFTSCAN_DIAGNOSTIC_SEVERITY_NOTE = 2,
+  SWIFTSCAN_DIAGNOSTIC_SEVERITY_REMARK = 3
+} swiftscan_diagnostic_severity_t;
+
+typedef struct {
+  swiftscan_diagnostic_info_t *diagnostics;
+  size_t count;
+} swiftscan_diagnostic_set_t;
+
 //=== Batch Scan Input Specification --------------------------------------===//
 
 /// Opaque container to a container of batch scan entry information.
@@ -92,6 +104,12 @@ SWIFTSCAN_PUBLIC swiftscan_dependency_set_t *
 swiftscan_dependency_graph_get_dependencies(
     swiftscan_dependency_graph_t result);
 
+// Return value disposed of together with the dependency_graph
+// using `swiftscan_dependency_graph_dispose`
+SWIFTSCAN_PUBLIC swiftscan_diagnostic_set_t *
+swiftscan_dependency_graph_get_diagnostics(
+    swiftscan_dependency_graph_t result);
+
 //=== Dependency Module Info Functions ------------------------------------===//
 
 SWIFTSCAN_PUBLIC swiftscan_string_ref_t
@@ -277,6 +295,11 @@ swiftscan_batch_scan_entry_get_is_swift(swiftscan_batch_scan_entry_t entry);
 SWIFTSCAN_PUBLIC swiftscan_string_set_t *
 swiftscan_import_set_get_imports(swiftscan_import_set_t result);
 
+// Return value disposed of together with the dependency_graph
+// using `swiftscan_import_set_dispose`
+SWIFTSCAN_PUBLIC swiftscan_diagnostic_set_t *
+swiftscan_import_set_get_diagnostics(swiftscan_import_set_t result);
+
 //=== Scanner Invocation Functions ----------------------------------------===//
 
 /// Create an \c swiftscan_scan_invocation_t instance.
@@ -378,18 +401,6 @@ SWIFTSCAN_PUBLIC swiftscan_import_set_t swiftscan_import_set_create(
 
 
 //=== Scanner Diagnostics -------------------------------------------------===//
-typedef enum {
-  SWIFTSCAN_DIAGNOSTIC_SEVERITY_ERROR = 0,
-  SWIFTSCAN_DIAGNOSTIC_SEVERITY_WARNING = 1,
-  SWIFTSCAN_DIAGNOSTIC_SEVERITY_NOTE = 2,
-  SWIFTSCAN_DIAGNOSTIC_SEVERITY_REMARK = 3
-} swiftscan_diagnostic_severity_t;
-
-typedef struct {
-  swiftscan_diagnostic_info_t *diagnostics;
-  size_t count;
-} swiftscan_diagnostic_set_t;
-
 /// For the specified \c scanner instance, query all insofar emitted diagnostics
 SWIFTSCAN_PUBLIC swiftscan_diagnostic_set_t*
 swiftscan_scanner_diagnostics_query(swiftscan_scanner_t scanner);

include/swift/DependencyScan/DependencyScanImpl.h

@@ -30,6 +30,9 @@ struct swiftscan_dependency_graph_s {
 
   /// The complete list of modules discovered
   swiftscan_dependency_set_t *dependencies;
+
+  /// Diagnostics produced during this scan
+  swiftscan_diagnostic_set_t *diagnostics;
 };
 
 struct swiftscan_dependency_info_s {
@@ -192,6 +195,8 @@ struct swiftscan_batch_scan_entry_s {
 struct swiftscan_import_set_s {
   /// The complete list of imports discovered
   swiftscan_string_set_t *imports;
+  /// Diagnostics produced during this import scan
+  swiftscan_diagnostic_set_t *diagnostics;
 };
 
 struct swiftscan_scan_invocation_s {

include/swift/DependencyScan/DependencyScanningTool.h

@@ -24,28 +24,48 @@
 namespace swift {
 namespace dependencies {
 class DependencyScanningTool;
+class DependencyScanDiagnosticCollector;
+
+struct ScanQueryInstance {
+  std::unique_ptr<CompilerInstance> ScanInstance;
+  std::unique_ptr<DependencyScanDiagnosticCollector> ScanDiagnostics;
+};
 
 /// Diagnostic consumer that simply collects the diagnostics emitted so-far
-class DependencyScannerDiagnosticCollectingConsumer : public DiagnosticConsumer {
-public:
-  friend DependencyScanningTool;
-  DependencyScannerDiagnosticCollectingConsumer() {}
-  void reset() { Diagnostics.clear(); }
+class DependencyScanDiagnosticCollector : public DiagnosticConsumer {
 private:
   struct ScannerDiagnosticInfo {
     std::string Message;
     llvm::SourceMgr::DiagKind Severity;
   };
-  
-  void handleDiagnostic(SourceManager &SM,
-                        const DiagnosticInfo &Info) override;
-  void addDiagnostic(SourceManager &SM, const DiagnosticInfo &Info);
+
+  void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
   std::vector<ScannerDiagnosticInfo> Diagnostics;
-  // FIXME: For now, we isolate access to shared state of this object
-  // but we really should make sure that it doesn't get shared.
-  llvm::sys::SmartMutex<true> ScanningDiagnosticConsumerStateLock;
+
+protected:
+  virtual void addDiagnostic(SourceManager &SM, const DiagnosticInfo &Info);
+
+public:
+  friend DependencyScanningTool;
+  DependencyScanDiagnosticCollector() {}
+  void reset() { Diagnostics.clear(); }
+  const std::vector<ScannerDiagnosticInfo> &getDiagnostics() const {
+    return Diagnostics;
+  }
 };
 
+/// Locking variant of the above diagnostic collector that guards accesses to
+/// its state with a lock.
+class LockingDependencyScanDiagnosticCollector
+    : public DependencyScanDiagnosticCollector {
+private:
+  void addDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
+  llvm::sys::SmartMutex<true> ScanningDiagnosticConsumerStateLock;
+
+public:
+  friend DependencyScanningTool;
+  LockingDependencyScanDiagnosticCollector() {}
+};
 
 /// Given a set of arguments to a print-target-info frontend tool query, produce the
 /// JSON target info.
@@ -93,19 +113,19 @@ class DependencyScanningTool {
   /// Discard the tool's current `SharedCache` and start anew.
   void resetCache();
   /// Query diagnostics consumed so far.
-  std::vector<DependencyScannerDiagnosticCollectingConsumer::ScannerDiagnosticInfo> getDiagnostics();
+  std::vector<DependencyScanDiagnosticCollector::ScannerDiagnosticInfo> getDiagnostics();
   /// Discared the collection of diagnostics encountered so far.
   void resetDiagnostics();
 
   /// Using the specified invocation command, instantiate a CompilerInstance
   /// that will be used for this scan.
-  llvm::ErrorOr<std::unique_ptr<CompilerInstance>>
+  llvm::ErrorOr<ScanQueryInstance>
   initCompilerInstanceForScan(ArrayRef<const char *> Command);
 
 private:
   /// Using the specified invocation command, initialize the scanner instance
   /// for this scan. Returns the `CompilerInstance` that will be used.
-  llvm::ErrorOr<std::unique_ptr<CompilerInstance>>
+  llvm::ErrorOr<ScanQueryInstance>
   initScannerForAction(ArrayRef<const char *> Command);
 
   /// Shared cache of module dependencies, re-used by individual full-scan queries
@@ -120,7 +140,7 @@ class DependencyScanningTool {
   llvm::sys::SmartMutex<true> DependencyScanningToolStateLock;
 
   /// A shared consumer that accumulates encountered diagnostics.
-  DependencyScannerDiagnosticCollectingConsumer CDC;
+  LockingDependencyScanDiagnosticCollector CDC;
   llvm::BumpPtrAllocator Alloc;
   llvm::StringSaver Saver;
 };

include/swift/DependencyScan/ScanDependencies.h

@@ -30,6 +30,7 @@ class ModuleDependenciesCache;
 class SwiftDependencyScanningService;
 
 namespace dependencies {
+class DependencyScanDiagnosticCollector;
 
 using CompilerArgInstanceCacheMap =
     llvm::StringMap<std::tuple<std::unique_ptr<CompilerInstance>,
@@ -59,16 +60,19 @@ bool batchScanDependencies(CompilerInstance &instance,
 /// Scans the dependencies of the main module of \c instance.
 llvm::ErrorOr<swiftscan_dependency_graph_t>
 performModuleScan(CompilerInstance &instance,
+                  DependencyScanDiagnosticCollector *diagnostics,
                   ModuleDependenciesCache &cache);
 
 /// Scans the main module of \c instance for all direct module imports
 llvm::ErrorOr<swiftscan_import_set_t>
 performModulePrescan(CompilerInstance &instance,
+                     DependencyScanDiagnosticCollector *diagnostics,
                      ModuleDependenciesCache &cache);
 
 /// Batch scan the dependencies for modules specified in \c batchInputFile.
 std::vector<llvm::ErrorOr<swiftscan_dependency_graph_t>>
 performBatchModuleScan(CompilerInstance &invocationInstance,
+                       DependencyScanDiagnosticCollector *diagnostics,
                        ModuleDependenciesCache &invocationCache,
                        CompilerArgInstanceCacheMap *versionedPCMInstanceCache,
                        llvm::StringSaver &saver,

lib/DependencyScan/DependencyScanningTool.cpp

@@ -68,16 +68,16 @@ llvm::ErrorOr<swiftscan_string_ref_t> getTargetInfo(ArrayRef<const char *> Comma
   return c_string_utils::create_clone(ResultStr.c_str());
 }
 
-void DependencyScannerDiagnosticCollectingConsumer::handleDiagnostic(SourceManager &SM,
+void DependencyScanDiagnosticCollector::handleDiagnostic(SourceManager &SM,
                       const DiagnosticInfo &Info) {
   addDiagnostic(SM, Info);
   for (auto ChildInfo : Info.ChildDiagnosticInfo) {
     addDiagnostic(SM, *ChildInfo);
   }
 }
 
-void DependencyScannerDiagnosticCollectingConsumer::addDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) {
-  llvm::sys::SmartScopedLock<true> Lock(ScanningDiagnosticConsumerStateLock);
+void DependencyScanDiagnosticCollector::addDiagnostic(
+    SourceManager &SM, const DiagnosticInfo &Info) {
   // Determine what kind of diagnostic we're emitting.
   llvm::SourceMgr::DiagKind SMKind;
   switch (Info.Kind) {
@@ -115,6 +115,12 @@ void DependencyScannerDiagnosticCollectingConsumer::addDiagnostic(SourceManager
   Diagnostics.push_back(ScannerDiagnosticInfo{Msg.getMessage().str(), SMKind});
 }
 
+void LockingDependencyScanDiagnosticCollector::addDiagnostic(
+    SourceManager &SM, const DiagnosticInfo &Info) {
+  llvm::sys::SmartScopedLock<true> Lock(ScanningDiagnosticConsumerStateLock);
+  DependencyScanDiagnosticCollector::addDiagnostic(SM, Info);
+}
+
 DependencyScanningTool::DependencyScanningTool()
     : ScanningService(std::make_unique<SwiftDependencyScanningService>()),
       VersionedPCMInstanceCacheCache(
@@ -126,18 +132,20 @@ DependencyScanningTool::getDependencies(
     ArrayRef<const char *> Command,
     const llvm::StringSet<> &PlaceholderModules) {
   // The primary instance used to scan the query Swift source-code
-  auto InstanceOrErr = initScannerForAction(Command);
-  if (std::error_code EC = InstanceOrErr.getError())
+  auto QueryContextOrErr = initScannerForAction(Command);
+  if (std::error_code EC = QueryContextOrErr.getError())
     return EC;
-  auto Instance = std::move(*InstanceOrErr);
+  auto QueryContext = std::move(*QueryContextOrErr);
 
   // Local scan cache instance, wrapping the shared global cache.
   ModuleDependenciesCache cache(
-      *ScanningService, Instance->getMainModule()->getNameStr().str(),
-      Instance->getInvocation().getFrontendOptions().ExplicitModulesOutputPath,
-      Instance->getInvocation().getModuleScanningHash());
+      *ScanningService, QueryContext.ScanInstance->getMainModule()->getNameStr().str(),
+      QueryContext.ScanInstance->getInvocation().getFrontendOptions().ExplicitModulesOutputPath,
+      QueryContext.ScanInstance->getInvocation().getModuleScanningHash());
   // Execute the scanning action, retrieving the in-memory result
-  auto DependenciesOrErr = performModuleScan(*Instance.get(), cache);
+  auto DependenciesOrErr = performModuleScan(*QueryContext.ScanInstance.get(), 
+                                             QueryContext.ScanDiagnostics.get(),
+                                             cache);
   if (DependenciesOrErr.getError())
     return std::make_error_code(std::errc::not_supported);
   auto Dependencies = std::move(*DependenciesOrErr);
@@ -148,17 +156,19 @@ DependencyScanningTool::getDependencies(
 llvm::ErrorOr<swiftscan_import_set_t>
 DependencyScanningTool::getImports(ArrayRef<const char *> Command) {
   // The primary instance used to scan the query Swift source-code
-  auto InstanceOrErr = initScannerForAction(Command);
-  if (std::error_code EC = InstanceOrErr.getError())
+  auto QueryContextOrErr = initScannerForAction(Command);
+  if (std::error_code EC = QueryContextOrErr.getError())
     return EC;
-  auto Instance = std::move(*InstanceOrErr);
+  auto QueryContext = std::move(*QueryContextOrErr);
 
   // Local scan cache instance, wrapping the shared global cache.
   ModuleDependenciesCache cache(
-      *ScanningService, Instance->getMainModule()->getNameStr().str(),
-      Instance->getInvocation().getFrontendOptions().ExplicitModulesOutputPath,
-      Instance->getInvocation().getModuleScanningHash());
-  auto DependenciesOrErr = performModulePrescan(*Instance.get(), cache);
+      *ScanningService, QueryContext.ScanInstance->getMainModule()->getNameStr().str(),
+      QueryContext.ScanInstance->getInvocation().getFrontendOptions().ExplicitModulesOutputPath,
+      QueryContext.ScanInstance->getInvocation().getModuleScanningHash());
+  auto DependenciesOrErr = performModulePrescan(*QueryContext.ScanInstance.get(), 
+                                                QueryContext.ScanDiagnostics.get(),
+                                                cache);
   if (DependenciesOrErr.getError())
     return std::make_error_code(std::errc::not_supported);
   auto Dependencies = std::move(*DependenciesOrErr);
@@ -172,19 +182,20 @@ DependencyScanningTool::getDependencies(
     const std::vector<BatchScanInput> &BatchInput,
     const llvm::StringSet<> &PlaceholderModules) {
   // The primary instance used to scan Swift modules
-  auto InstanceOrErr = initScannerForAction(Command);
-  if (std::error_code EC = InstanceOrErr.getError())
+  auto QueryContextOrErr = initScannerForAction(Command);
+  if (std::error_code EC = QueryContextOrErr.getError())
     return std::vector<llvm::ErrorOr<swiftscan_dependency_graph_t>>(
         BatchInput.size(), std::make_error_code(std::errc::invalid_argument));
-  auto Instance = std::move(*InstanceOrErr);
+  auto QueryContext = std::move(*QueryContextOrErr);
 
   // Local scan cache instance, wrapping the shared global cache.
   ModuleDependenciesCache cache(
-      *ScanningService, Instance->getMainModule()->getNameStr().str(),
-      Instance->getInvocation().getFrontendOptions().ExplicitModulesOutputPath,
-      Instance->getInvocation().getModuleScanningHash());
+      *ScanningService, QueryContext.ScanInstance->getMainModule()->getNameStr().str(),
+      QueryContext.ScanInstance->getInvocation().getFrontendOptions().ExplicitModulesOutputPath,
+      QueryContext.ScanInstance->getInvocation().getModuleScanningHash());
   auto BatchScanResults = performBatchModuleScan(
-      *Instance.get(), cache, VersionedPCMInstanceCacheCache.get(),
+      *QueryContext.ScanInstance.get(), QueryContext.ScanDiagnostics.get(),
+      cache, VersionedPCMInstanceCacheCache.get(),
       Saver, BatchInput);
 
   return BatchScanResults;
@@ -221,7 +232,7 @@ void DependencyScanningTool::resetCache() {
 }
 
 std::vector<
-    DependencyScannerDiagnosticCollectingConsumer::ScannerDiagnosticInfo>
+    DependencyScanDiagnosticCollector::ScannerDiagnosticInfo>
 DependencyScanningTool::getDiagnostics() {
   llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
   return CDC.Diagnostics;
@@ -232,25 +243,27 @@ void DependencyScanningTool::resetDiagnostics() {
   CDC.reset();
 }
 
-llvm::ErrorOr<std::unique_ptr<CompilerInstance>>
+llvm::ErrorOr<ScanQueryInstance>
 DependencyScanningTool::initScannerForAction(
     ArrayRef<const char *> Command) {
   // The remainder of this method operates on shared state in the
   // scanning service and global LLVM state with:
   // llvm::cl::ResetAllOptionOccurrences
   llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
-  auto instanceOrErr = initCompilerInstanceForScan(Command);
-  if (instanceOrErr.getError())
-    return instanceOrErr;
-  return instanceOrErr;
+  return initCompilerInstanceForScan(Command);
 }
 
-llvm::ErrorOr<std::unique_ptr<CompilerInstance>>
+llvm::ErrorOr<ScanQueryInstance>
 DependencyScanningTool::initCompilerInstanceForScan(
     ArrayRef<const char *> CommandArgs) {
   // State unique to an individual scan
   auto Instance = std::make_unique<CompilerInstance>();
+  auto ScanDiagnosticConsumer = std::make_unique<DependencyScanDiagnosticCollector>();
+
+  // FIXME: The shared CDC must be deprecated once all clients have switched
+  // to using per-scan diagnostic output embedded in the `swiftscan_dependency_graph_s`
   Instance->addDiagnosticConsumer(&CDC);
+  Instance->addDiagnosticConsumer(ScanDiagnosticConsumer.get());
 
   // Basic error checking on the arguments
   if (CommandArgs.empty()) {
@@ -300,7 +313,8 @@ DependencyScanningTool::initCompilerInstanceForScan(
 
   (void)Instance->getMainModule();
 
-  return Instance;
+  return ScanQueryInstance{std::move(Instance), 
+                           std::move(ScanDiagnosticConsumer)};
 }
 
 } // namespace dependencies