Merge pull request #61059 from artemcm/ClangScannerGardening

[Dependency Scanner][NFC] Clean up/Gardening on 'ClangModuleDependencyScanner'

Author: artemcm

include/swift/AST/ModuleDependencies.h

@@ -20,6 +20,8 @@
 
 #include "swift/Basic/LLVM.h"
 #include "swift/AST/Import.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
+#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringSet.h"
@@ -305,6 +307,7 @@ class SwiftPlaceholderModuleDependencyStorage : public ModuleDependenciesStorage
   }
 };
 
+// MARK: Module Dependencies
 /// Describes the dependencies of a given module.
 ///
 /// The dependencies of a module include all of the source files that go
@@ -475,6 +478,7 @@ using ModuleDependenciesKindRefMap =
                        llvm::StringMap<const ModuleDependencies *>,
                        ModuleDependenciesKindHash>;
 
+// MARK: GlobalModuleDependenciesCache
 /// A cache describing the set of module dependencies that has been queried
 /// thus far. This cache records/stores the actual Dependency values and can be
 /// preserved across different scanning actions (e.g. in
@@ -523,7 +527,7 @@ class GlobalModuleDependenciesCache {
   getDependenciesMap(ModuleDependenciesKind kind) const;
 
 public:
-  GlobalModuleDependenciesCache() {};
+  GlobalModuleDependenciesCache();
   GlobalModuleDependenciesCache(const GlobalModuleDependenciesCache &) = delete;
   GlobalModuleDependenciesCache &
   operator=(const GlobalModuleDependenciesCache &) = delete;
@@ -592,6 +596,7 @@ class GlobalModuleDependenciesCache {
   }
 };
 
+// MARK: ModuleDependenciesCache
 /// This "local" dependencies cache persists only for the duration of a given
 /// scanning action, and wraps an instance of a `GlobalModuleDependenciesCache`
 /// which may carry cached scanning information from prior scanning actions.
@@ -609,21 +614,17 @@ class ModuleDependenciesCache {
 
   /// Name of the module under scan
   StringRef mainScanModuleName;
+  /// Set containing all of the Clang modules that have already been seen.
+  llvm::StringSet<> alreadySeenClangModules;
+  /// The 'persistent' Clang dependency scanner service
+  /// TODO: Share this service among common scanner invocations
+  clang::tooling::dependencies::DependencyScanningService clangScanningService;
+  /// The Clang dependency scanner tool
+  clang::tooling::dependencies::DependencyScanningTool clangScanningTool;
 
   /// Discovered Clang modules are only cached locally.
   llvm::StringMap<ModuleDependenciesVector> clangModuleDependencies;
 
-  /// Function that will delete \c clangImpl properly.
-  void (*clangImplDeleter)(ClangModuleDependenciesCacheImpl *) = nullptr;
-  /// Additional information needed for Clang dependency scanning.
-  ClangModuleDependenciesCacheImpl *clangImpl = nullptr;
-
-  /// Free up the storage associated with the Clang implementation.
-  void destroyClangImpl() {
-    if (this->clangImplDeleter)
-      this->clangImplDeleter(this->clangImpl);
-  }
-
   /// Retrieve the dependencies map that corresponds to the given dependency
   /// kind.
   llvm::StringMap<const ModuleDependencies *> &
@@ -647,28 +648,20 @@ class ModuleDependenciesCache {
                           StringRef mainScanModuleName);
   ModuleDependenciesCache(const ModuleDependenciesCache &) = delete;
   ModuleDependenciesCache &operator=(const ModuleDependenciesCache &) = delete;
-  virtual ~ModuleDependenciesCache() { destroyClangImpl(); }
 
 public:
-  /// Set the Clang-specific implementation data.
-  void
-  setClangImpl(ClangModuleDependenciesCacheImpl *clangImpl,
-               void (*clangImplDeleter)(ClangModuleDependenciesCacheImpl *)) {
-    destroyClangImpl();
-
-    this->clangImpl = clangImpl;
-    this->clangImplDeleter = clangImplDeleter;
-  }
-
-  /// Retrieve the Clang-specific implementation data;
-  ClangModuleDependenciesCacheImpl *getClangImpl() const {
-    return clangImpl;
-  }
-
   /// Whether we have cached dependency information for the given module.
   bool hasDependencies(StringRef moduleName,
                        ModuleLookupSpecifics details) const;
 
+  /// Produce a reference to the Clang scanner tool associated with this cache
+  clang::tooling::dependencies::DependencyScanningTool& getClangScannerTool() {
+    return clangScanningTool;
+  }
+  llvm::StringSet<>& getAlreadySeenClangModules() {
+    return alreadySeenClangModules;
+  }
+  
   /// Look for module dependencies for a module with the given name given
   /// current search paths.
   ///

lib/AST/CMakeLists.txt

@@ -137,6 +137,7 @@ if(NOT SWIFT_BUILD_ONLY_SYNTAXPARSERLIB)
     clangFormat
     clangToolingCore
     clangFrontendTool
+    clangDependencyScanning
     clangFrontend
     clangDriver
     clangSerialization

lib/AST/ModuleDependencies.cpp

@@ -225,6 +225,7 @@ void ModuleDependencies::addBridgingModuleDependency(
   }
 }
 
+GlobalModuleDependenciesCache::GlobalModuleDependenciesCache() {}
 GlobalModuleDependenciesCache::TargetSpecificGlobalCacheState *
 GlobalModuleDependenciesCache::getCurrentCache() const {
   assert(CurrentTriple.hasValue() &&
@@ -533,7 +534,17 @@ ModuleDependenciesCache::getDependencyReferencesMap(
 ModuleDependenciesCache::ModuleDependenciesCache(
     GlobalModuleDependenciesCache &globalCache,
     StringRef mainScanModuleName)
-    : globalCache(globalCache), mainScanModuleName(mainScanModuleName) {
+    : globalCache(globalCache),
+      mainScanModuleName(mainScanModuleName),
+      clangScanningService(
+          clang::tooling::dependencies::ScanningMode::DependencyDirectivesScan,
+          clang::tooling::dependencies::ScanningOutputFormat::Full,
+          clang::CASOptions(),
+          /* Cache */ nullptr,
+          /* SharedFS */ nullptr,
+          /* ReuseFileManager */ false,
+          /* OptimizeArgs */ false),
+      clangScanningTool(clangScanningService) {
   for (auto kind = ModuleDependenciesKind::FirstKind;
        kind != ModuleDependenciesKind::LastKind; ++kind) {
     ModuleDependenciesMap.insert(

lib/ClangImporter/ClangModuleDependencyScanner.cpp

@@ -28,21 +28,6 @@ using namespace swift;
 using namespace clang::tooling;
 using namespace clang::tooling::dependencies;
 
-class swift::ClangModuleDependenciesCacheImpl {
-public:
-  /// Set containing all of the Clang modules that have already been seen.
-  llvm::StringSet<> alreadySeen;
-
-  DependencyScanningService service;
-
-  DependencyScanningTool tool;
-
-  ClangModuleDependenciesCacheImpl()
-      : service(ScanningMode::DependencyDirectivesScan,
-                ScanningOutputFormat::Full, clang::CASOptions(), nullptr, nullptr),
-        tool(service) {}
-};
-
 // Add search paths.
 // Note: This is handled differently for the Clang importer itself, which
 // adds search paths to Clang's data structures rather than to its
@@ -107,21 +92,6 @@ static std::vector<std::string> getClangDepScanningInvocationArguments(
   return commandLineArgs;
 }
 
-/// Get or create the Clang-specific
-static ClangModuleDependenciesCacheImpl *getOrCreateClangImpl(
-    ModuleDependenciesCache &cache) {
-  auto clangImpl = cache.getClangImpl();
-  if (!clangImpl) {
-    clangImpl = new ClangModuleDependenciesCacheImpl();
-    cache.setClangImpl(clangImpl,
-                       [](ClangModuleDependenciesCacheImpl *ptr) {
-      delete ptr;
-    });
-  }
-
-  return clangImpl;
-}
-
 /// Record the module dependencies we found by scanning Clang modules into
 /// the module dependencies cache.
 void ClangImporter::recordModuleDependencies(
@@ -272,9 +242,6 @@ Optional<ModuleDependencies> ClangImporter::getModuleDependencies(
           {ModuleDependenciesKind::Clang, currentSwiftSearchPathSet}))
     return found;
 
-  // Retrieve or create the shared state.
-  auto clangImpl = getOrCreateClangImpl(cache);
-
   // Determine the command-line arguments for dependency scanning.
   std::vector<std::string> commandLineArgs =
     getClangDepScanningInvocationArguments(ctx);
@@ -298,16 +265,17 @@ Optional<ModuleDependencies> ClangImporter::getModuleDependencies(
     workingDir = *(clangWorkingDirPos - 1);
   }
 
-  auto clangDependencies = clangImpl->tool.getFullDependencies(
-      commandLineArgs, workingDir, clangImpl->alreadySeen, moduleName);
+  auto clangDependencies = cache.getClangScannerTool().getFullDependencies(
+      commandLineArgs, workingDir, cache.getAlreadySeenClangModules(),
+      moduleName);
   if (!clangDependencies) {
     auto errorStr = toString(clangDependencies.takeError());
     // We ignore the "module 'foo' not found" error, the Swift dependency
     // scanner will report such an error only if all of the module loaders
     // fail as well.
-    if (errorStr.find("fatal error: module '" + moduleName.str() + "' not found") == std::string::npos)
-      ctx.Diags.diagnose(SourceLoc(),
-                         diag::clang_dependency_scan_error,
+    if (errorStr.find("fatal error: module '" + moduleName.str() +
+                      "' not found") == std::string::npos)
+      ctx.Diags.diagnose(SourceLoc(), diag::clang_dependency_scan_error,
                          errorStr);
     return None;
   }
@@ -344,9 +312,6 @@ bool ClangImporter::addBridgingHeaderDependencies(
     llvm_unreachable("Unexpected module dependency kind");
   }
 
-  // Retrieve or create the shared state.
-  auto clangImpl = getOrCreateClangImpl(cache);
-
   // Retrieve the bridging header.
   std::string bridgingHeader = *targetModule.getBridgingHeader();
 
@@ -356,9 +321,8 @@ bool ClangImporter::addBridgingHeaderDependencies(
   std::string workingDir =
       ctx.SourceMgr.getFileSystem()->getCurrentWorkingDirectory().get();
 
-  auto clangDependencies = clangImpl->tool.getFullDependencies(
-      commandLineArgs, workingDir, clangImpl->alreadySeen);
-
+  auto clangDependencies = cache.getClangScannerTool().getFullDependencies(
+      commandLineArgs, workingDir, cache.getAlreadySeenClangModules());
   if (!clangDependencies) {
     // FIXME: Route this to a normal diagnostic.
     llvm::logAllUnhandledErrors(clangDependencies.takeError(), llvm::errs());