Merge pull request swiftlang#81262 from artemcm/DepScanParallelDiagnosticConsumer

[Dependency Scanning] Always use a locking diagnostic consumer

Author: artemcm

include/swift/DependencyScan/DependencyScanningTool.h

@@ -40,6 +40,7 @@ class DependencyScanDiagnosticCollector : public DiagnosticConsumer {
     std::optional<ScannerImportStatementInfo::ImportDiagnosticLocationInfo> ImportLocation;
   };
   std::vector<ScannerDiagnosticInfo> Diagnostics;
+  llvm::sys::SmartMutex<true> ScanningDiagnosticConsumerStateLock;
 
   void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
 
@@ -55,19 +56,6 @@ class DependencyScanDiagnosticCollector : public DiagnosticConsumer {
   }
 };
 
-/// 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.
 llvm::ErrorOr<swiftscan_string_ref_t> getTargetInfo(ArrayRef<const char *> Command,
@@ -97,11 +85,6 @@ class DependencyScanningTool {
   llvm::ErrorOr<swiftscan_import_set_t>
   getImports(ArrayRef<const char *> Command, StringRef WorkingDirectory);
 
-  /// Query diagnostics consumed so far.
-  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<ScanQueryInstance>
@@ -116,9 +99,6 @@ class DependencyScanningTool {
 
   /// Shared state mutual-exclusivity lock
   llvm::sys::SmartMutex<true> DependencyScanningToolStateLock;
-
-  /// A shared consumer that accumulates encountered diagnostics.
-  LockingDependencyScanDiagnosticCollector CDC;
   llvm::BumpPtrAllocator Alloc;
   llvm::StringSaver Saver;
 };

lib/DependencyScan/DependencyScanningTool.cpp

@@ -74,6 +74,8 @@ void DependencyScanDiagnosticCollector::handleDiagnostic(SourceManager &SM,
 
 void DependencyScanDiagnosticCollector::addDiagnostic(
     SourceManager &SM, const DiagnosticInfo &Info) {
+  llvm::sys::SmartScopedLock<true> Lock(ScanningDiagnosticConsumerStateLock);
+
   // Determine what kind of diagnostic we're emitting.
   llvm::SourceMgr::DiagKind SMKind;
   switch (Info.Kind) {
@@ -129,12 +131,6 @@ void DependencyScanDiagnosticCollector::addDiagnostic(
   }
 }
 
-void LockingDependencyScanDiagnosticCollector::addDiagnostic(
-    SourceManager &SM, const DiagnosticInfo &Info) {
-  llvm::sys::SmartScopedLock<true> Lock(ScanningDiagnosticConsumerStateLock);
-  DependencyScanDiagnosticCollector::addDiagnostic(SM, Info);
-}
-
 swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(
     const DependencyScanDiagnosticCollector *diagnosticCollector) {
   auto collectedDiagnostics = diagnosticCollector->getDiagnostics();
@@ -263,7 +259,7 @@ static swiftscan_import_set_t generateHollowDiagnosticOutputImportSet(
 
 DependencyScanningTool::DependencyScanningTool()
     : ScanningService(std::make_unique<SwiftDependencyScanningService>()),
-      CDC(), Alloc(), Saver(Alloc) {}
+      Alloc(), Saver(Alloc) {}
 
 llvm::ErrorOr<swiftscan_dependency_graph_t>
 DependencyScanningTool::getDependencies(
@@ -272,7 +268,8 @@ DependencyScanningTool::getDependencies(
     StringRef WorkingDirectory) {
   // There may be errors as early as in instance initialization, so we must ensure
   // we can catch those.
-  auto ScanDiagnosticConsumer = std::make_shared<DependencyScanDiagnosticCollector>();
+  auto ScanDiagnosticConsumer =
+    std::make_shared<DependencyScanDiagnosticCollector>();
 
   // The primary instance used to scan the query Swift source-code
   auto QueryContextOrErr = initCompilerInstanceForScan(Command,
@@ -329,18 +326,6 @@ DependencyScanningTool::getImports(ArrayRef<const char *> Command,
   return std::move(*DependenciesOrErr);
 }
 
-std::vector<
-    DependencyScanDiagnosticCollector::ScannerDiagnosticInfo>
-DependencyScanningTool::getDiagnostics() {
-  llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
-  return CDC.Diagnostics;
-}
-
-void DependencyScanningTool::resetDiagnostics() {
-  llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
-  CDC.reset();
-}
-
 llvm::ErrorOr<ScanQueryInstance>
 DependencyScanningTool::initCompilerInstanceForScan(
     ArrayRef<const char *> CommandArgs,
@@ -356,10 +341,6 @@ DependencyScanningTool::initCompilerInstanceForScan(
 
   // State unique to an individual scan
   auto Instance = std::make_unique<CompilerInstance>();
-
-  // 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(scannerDiagnosticsCollector.get());
 
   // Basic error checking on the arguments

lib/Tooling/libSwiftScan/libSwiftScan.cpp

@@ -589,45 +589,15 @@ swiftscan_compiler_supported_features_query() {
 //=== Scanner Diagnostics -------------------------------------------------===//
 swiftscan_diagnostic_set_t*
 swiftscan_scanner_diagnostics_query(swiftscan_scanner_t scanner) {
-  DependencyScanningTool *ScanningTool = unwrap(scanner);
-  auto Diagnostics = ScanningTool->getDiagnostics();
-  auto NumDiagnostics = Diagnostics.size();
-
-  swiftscan_diagnostic_set_t *Result = new swiftscan_diagnostic_set_t;
-  Result->count = NumDiagnostics;
-  Result->diagnostics = new swiftscan_diagnostic_info_t[NumDiagnostics];
-
-  for (size_t i = 0; i < NumDiagnostics; ++i) {
-    const auto &Diagnostic = Diagnostics[i];
-    swiftscan_diagnostic_info_s *DiagnosticInfo = new swiftscan_diagnostic_info_s;
-    DiagnosticInfo->message = swift::c_string_utils::create_clone(Diagnostic.Message.c_str());
-    switch (Diagnostic.Severity) {
-    case llvm::SourceMgr::DK_Error:
-      DiagnosticInfo->severity = SWIFTSCAN_DIAGNOSTIC_SEVERITY_ERROR;
-      break;
-    case llvm::SourceMgr::DK_Warning:
-      DiagnosticInfo->severity = SWIFTSCAN_DIAGNOSTIC_SEVERITY_WARNING;
-      break;
-    case llvm::SourceMgr::DK_Note:
-      DiagnosticInfo->severity = SWIFTSCAN_DIAGNOSTIC_SEVERITY_NOTE;
-      break;
-    case llvm::SourceMgr::DK_Remark:
-      DiagnosticInfo->severity = SWIFTSCAN_DIAGNOSTIC_SEVERITY_REMARK;
-      break;
-    }
-    // swiftscan_scanner_diagnostics_query is deprecated,
-    // so it does not support source locations.
-    DiagnosticInfo->source_location = nullptr;
-    Result->diagnostics[i] = DiagnosticInfo;
-  }
-
-  return Result;
+  // This method is deprecated
+  swiftscan_diagnostic_set_t *set = new swiftscan_diagnostic_set_t;
+  set->count = 0;
+  return set;
 }
 
 void
 swiftscan_scanner_diagnostics_reset(swiftscan_scanner_t scanner) {
-  DependencyScanningTool *ScanningTool = unwrap(scanner);
-  ScanningTool->resetDiagnostics();
+  // This method is deprecated
 }
 
 swiftscan_string_ref_t