[Dependency Scanning] Refactor Clang dependency bridging into a 'ModuleDependencyScanner' utility

This moves the functionality of 'bridgeClangModuleDependency' into a utility in the main scanner class because it relies on various objects whose lifetime is already tied to the scanner itself.

Author: artemcm

include/swift/AST/ModuleDependencies.h

@@ -23,7 +23,6 @@
 #include "swift/Basic/Assertions.h"
 #include "swift/Basic/CXXStdlibKind.h"
 #include "swift/Basic/LLVM.h"
-#include "swift/ClangImporter/ClangImporter.h"
 #include "swift/Serialization/Validation.h"
 #include "clang/CAS/CASOptions.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
@@ -165,6 +164,12 @@ using ModuleDependencyIDSetVector =
     llvm::SetVector<ModuleDependencyID, std::vector<ModuleDependencyID>,
                     std::set<ModuleDependencyID>>;
 
+/// A callback to lookup module outputs for "-fmodule-file=", "-o" etc.
+using LookupModuleOutputCallback = llvm::function_ref<std::string(
+    const clang::tooling::dependencies::ModuleDeps &,
+    clang::tooling::dependencies::ModuleOutputKind)>;
+using RemapPathCallback = llvm::function_ref<std::string(StringRef)>;
+
 namespace dependencies {
 std::string createEncodedModuleKindAndName(ModuleDependencyID id);
 bool checkImportNotTautological(const ImportPath::Module, const SourceLoc,
@@ -176,18 +181,6 @@ void registerCxxInteropLibraries(
     const llvm::Triple &Target, StringRef mainModuleName, bool hasStaticCxx,
     bool hasStaticCxxStdlib, CXXStdlibKind cxxStdlibKind,
     std::function<void(const LinkLibrary &)> RegistrationCallback);
-
-using RemapPathCallback = llvm::function_ref<std::string(StringRef)>;
-using LookupModuleOutputCallback =
-    llvm::function_ref<std::string(const clang::tooling::dependencies::ModuleDeps &,
-                                   clang::tooling::dependencies::ModuleOutputKind)>;
-
-ModuleDependencyInfo
-bridgeClangModuleDependency(
-    const ASTContext &ctx,
-    const clang::tooling::dependencies::ModuleDeps &clangDependency,
-    LookupModuleOutputCallback LookupModuleOutput,
-    RemapPathCallback remapPath = nullptr);
 } // namespace dependencies
 
 struct ScannerImportStatementInfo {
@@ -1042,6 +1035,8 @@ using ModuleNameToDependencyMap = llvm::StringMap<ModuleDependencyInfo>;
 using ModuleDependenciesKindMap =
     std::unordered_map<ModuleDependencyKind, ModuleNameToDependencyMap,
                        ModuleDependencyKindHash>;
+using BridgeClangDependencyCallback = llvm::function_ref<ModuleDependencyInfo(
+    const clang::tooling::dependencies::ModuleDeps &clangModuleDep)>;
 
 // MARK: SwiftDependencyScanningService
 /// A carrier of state shared among possibly multiple invocations of the
@@ -1194,9 +1189,8 @@ class ModuleDependenciesCache {
   /// Record dependencies for the given collection of Clang modules.
   void recordClangDependencies(
       const clang::tooling::dependencies::ModuleDepsGraph &dependencies,
-      const ASTContext &ctx,
-      dependencies::LookupModuleOutputCallback LookupModuleOutput,
-      dependencies::RemapPathCallback remapPath = nullptr);
+      DiagnosticEngine &diags,
+      BridgeClangDependencyCallback bridgeClangModule);
 
   /// Update stored dependencies for the given module.
   void updateDependency(ModuleDependencyID moduleID,

include/swift/DependencyScan/ModuleDependencyScanner.h

@@ -43,39 +43,23 @@ class ModuleDependencyScanningWorker {
   std::optional<clang::tooling::dependencies::TranslationUnitDeps>
   scanFilesystemForClangModuleDependency(
       Identifier moduleName,
+      LookupModuleOutputCallback lookupModuleCallback,
       const llvm::DenseSet<clang::tooling::dependencies::ModuleID>
           &alreadySeenModules);
 
-  /// Retrieve the module dependencies for the Swift module with the given name.
-  SwiftModuleScannerQueryResult scanFilesystemForSwiftModuleDependency(
-      Identifier moduleName, bool isTestableImport = false);
-
   /// Query dependency information for header dependencies
-  /// of a binary Swift module.
-  ///
-  /// \param moduleID the name of the Swift module whose dependency
-  /// information will be augmented with information about the given
-  /// textual header inputs.
-  ///
-  /// \param headerPath the path to the header to be scanned.
-  ///
-  /// \param clangScanningTool The clang dependency scanner.
-  ///
-  /// \param cache The module dependencies cache to update, with information
-  /// about new Clang modules discovered along the way.
-  ///
-  /// \returns \c true if an error occurred, \c false otherwise
-  bool scanHeaderDependenciesOfSwiftModule(
-      const ASTContext &ctx,
+  /// of a Swift module.
+  std::optional<clang::tooling::dependencies::TranslationUnitDeps>
+  scanHeaderDependenciesOfSwiftModule(
       ModuleDependencyID moduleID, std::optional<StringRef> headerPath,
       std::optional<llvm::MemoryBufferRef> sourceBuffer,
-      ModuleDependenciesCache &cache,
-      ModuleDependencyIDSetVector &headerClangModuleDependencies,
-      std::vector<std::string> &headerFileInputs,
-      std::vector<std::string> &bridgingHeaderCommandLine,
-      std::vector<std::string> &visibleClangModules,
-      std::optional<std::string> &includeTreeID);
+      LookupModuleOutputCallback lookupModuleCallback,
+      const llvm::DenseSet<clang::tooling::dependencies::ModuleID>
+         &alreadySeenModules);
 
+  /// Retrieve the module dependencies for the Swift module with the given name.
+  SwiftModuleScannerQueryResult scanFilesystemForSwiftModuleDependency(
+      Identifier moduleName, bool isTestableImport = false);
 
   /// Store cache entry for include tree.
   llvm::Error
@@ -93,16 +77,9 @@ class ModuleDependencyScanningWorker {
   // Swift and Clang module loaders acting as scanners.
   std::unique_ptr<SwiftModuleScanner> swiftModuleScannerLoader;
 
-  /// The location of where the explicitly-built modules will be output to
-  std::string moduleOutputPath;
-  /// The location of where the explicitly-built SDK modules will be output to
-  std::string sdkModuleOutputPath;
-
   // CAS instance.
   std::shared_ptr<llvm::cas::ObjectStore> CAS;
   std::shared_ptr<llvm::cas::ActionCache> ActionCache;
-  /// File prefix mapper.
-  llvm::PrefixMapper *PrefixMapper;
 
   // Base command line invocation for clang scanner queries (both module and header)
   std::vector<std::string> clangScanningBaseCommandLineArgs;
@@ -223,11 +200,7 @@ class ModuleDependencyScanner {
     return PrefixMapper && !PrefixMapper->getMappings().empty();
   }
   llvm::PrefixMapper *getPrefixMapper() const { return PrefixMapper.get(); }
-  std::string remapPath(StringRef Path) const {
-    if (!PrefixMapper)
-      return Path.str();
-    return PrefixMapper->mapToString(Path);
-  }
+  std::string remapPath(StringRef Path) const;
 
   /// CAS options.
   llvm::cas::ObjectStore &getCAS() const {
@@ -294,11 +267,22 @@ class ModuleDependencyScanner {
                                 ModuleDependenciesCache &cache,
                                 ModuleDependencyIDSetVector &allModules);
 
+  /// Bridge Clang dependency scanner's dependency node
+  /// to the Swift scanner's `ModuleDependencyInfo`.
+  ModuleDependencyInfo
+  bridgeClangModuleDependency(
+      const clang::tooling::dependencies::ModuleDeps &clangDependency);
+
   /// Perform an operation utilizing one of the Scanning workers
   /// available to this scanner.
   template <typename Function, typename... Args>
   auto withDependencyScanningWorker(Function &&F, Args &&...ArgList);
 
+  /// Determine cache-relative output path for a given Clang module
+  std::string clangModuleOutputPathLookup(
+      const clang::tooling::dependencies::ModuleDeps &clangDep,
+      clang::tooling::dependencies::ModuleOutputKind moduleOutputKind) const;
+
   /// Use the scanner's ASTContext to construct an `Identifier`
   /// for a given module name.
   Identifier getModuleImportIdentifier(StringRef moduleName);
@@ -318,6 +302,11 @@ class ModuleDependencyScanner {
   ASTContext &ScanASTContext;
   ModuleDependencyIssueReporter IssueReporter;
 
+  /// The location of where the explicitly-built modules will be output to
+  std::string ModuleOutputPath;
+  /// The location of where the explicitly-built SDK modules will be output to
+  std::string SDKModuleOutputPath;
+
   /// The available pool of workers for filesystem module search
   unsigned NumThreads;
   std::list<std::unique_ptr<ModuleDependencyScanningWorker>> Workers;

include/swift/Serialization/ScanningLoaders.h

@@ -77,10 +77,6 @@ class SwiftModuleScanner : public SerializedModuleLoaderBase {
 
   /// AST delegate to be used for textual interface scanning
   InterfaceSubContextDelegate &astDelegate;
-  /// Location where pre-built modules are to be built into.
-  std::string moduleOutputPath;
-  /// Location where pre-built SDK modules are to be built into.
-  std::string sdkModuleOutputPath;
   /// Clang-specific (-Xcc) command-line flags to include on
   /// Swift module compilation commands
   std::vector<std::string> swiftModuleClangCC1CommandLineArgs;
@@ -96,14 +92,12 @@ class SwiftModuleScanner : public SerializedModuleLoaderBase {
 public:
   SwiftModuleScanner(
       ASTContext &ctx, ModuleLoadingMode LoadMode,
-      InterfaceSubContextDelegate &astDelegate, StringRef moduleOutputPath,
-      StringRef sdkModuleOutputPath,
+      InterfaceSubContextDelegate &astDelegate,
       std::vector<std::string> swiftModuleClangCC1CommandLineArgs,
       llvm::StringMap<std::string> &explicitSwiftModuleInputs)
       : SerializedModuleLoaderBase(ctx, nullptr, LoadMode,
                                    /*IgnoreSwiftSourceInfoFile=*/true),
-        astDelegate(astDelegate), moduleOutputPath(moduleOutputPath),
-        sdkModuleOutputPath(sdkModuleOutputPath),
+        astDelegate(astDelegate),
         swiftModuleClangCC1CommandLineArgs(swiftModuleClangCC1CommandLineArgs),
         explicitSwiftModuleInputs(explicitSwiftModuleInputs) {
   }

lib/AST/ModuleDependencies.cpp

@@ -599,120 +599,6 @@ void swift::dependencies::registerCxxInteropLibraries(
   }
 }
 
-ModuleDependencyInfo swift::dependencies::bridgeClangModuleDependency(
-    const ASTContext &ctx,
-    const clang::tooling::dependencies::ModuleDeps &clangModuleDep,
-    LookupModuleOutputCallback lookupModuleOutput, RemapPathCallback callback) {
-  auto remapPath = [&](StringRef path) {
-    if (callback)
-      return callback(path);
-    return path.str();
-  };
-
-  // File dependencies for this module.
-  std::vector<std::string> fileDeps;
-  clangModuleDep.forEachFileDep(
-      [&fileDeps](StringRef fileDep) { fileDeps.emplace_back(fileDep); });
-
-  std::vector<std::string> swiftArgs;
-  auto addClangArg = [&](Twine arg) {
-    swiftArgs.push_back("-Xcc");
-    swiftArgs.push_back(arg.str());
-  };
-
-  // We are using Swift frontend mode.
-  swiftArgs.push_back("-frontend");
-
-  // Swift frontend action: -emit-pcm
-  swiftArgs.push_back("-emit-pcm");
-  swiftArgs.push_back("-module-name");
-  swiftArgs.push_back(clangModuleDep.ID.ModuleName);
-
-  auto pcmPath = lookupModuleOutput(
-      clangModuleDep,
-      clang::tooling::dependencies::ModuleOutputKind::ModuleFile);
-  swiftArgs.push_back("-o");
-  swiftArgs.push_back(pcmPath);
-
-  // Ensure that the resulting PCM build invocation uses Clang frontend
-  // directly
-  swiftArgs.push_back("-direct-clang-cc1-module-build");
-
-  // Swift frontend option for input file path (Foo.modulemap).
-  swiftArgs.push_back(remapPath(clangModuleDep.ClangModuleMapFile));
-
-  auto invocation = clangModuleDep.getUnderlyingCompilerInvocation();
-  // Clear some options from clang scanner.
-  invocation.getMutFrontendOpts().ModuleCacheKeys.clear();
-  invocation.getMutFrontendOpts().PathPrefixMappings.clear();
-  invocation.getMutFrontendOpts().OutputFile.clear();
-
-  // Reset CASOptions since that should be coming from swift.
-  invocation.getMutCASOpts() = clang::CASOptions();
-  invocation.getMutFrontendOpts().CASIncludeTreeID.clear();
-
-  // FIXME: workaround for rdar://105684525: find the -ivfsoverlay option
-  // from clang scanner and pass to swift.
-  if (!ctx.CASOpts.EnableCaching) {
-    auto &overlayFiles = invocation.getMutHeaderSearchOpts().VFSOverlayFiles;
-    for (auto overlay : overlayFiles) {
-      swiftArgs.push_back("-vfsoverlay");
-      swiftArgs.push_back(overlay);
-    }
-  }
-
-  // Add args reported by the scanner.
-  auto clangArgs = invocation.getCC1CommandLine();
-  llvm::for_each(clangArgs, addClangArg);
-
-  // CASFileSystemRootID.
-  std::string RootID = clangModuleDep.CASFileSystemRootID
-                           ? clangModuleDep.CASFileSystemRootID->toString()
-                           : "";
-
-  std::string IncludeTree =
-      clangModuleDep.IncludeTreeID ? *clangModuleDep.IncludeTreeID : "";
-
-  ctx.CASOpts.enumerateCASConfigurationFlags(
-      [&](StringRef Arg) { swiftArgs.push_back(Arg.str()); });
-
-  if (!IncludeTree.empty()) {
-    swiftArgs.push_back("-clang-include-tree-root");
-    swiftArgs.push_back(IncludeTree);
-  }
-  std::string mappedPCMPath = remapPath(pcmPath);
-
-  std::vector<LinkLibrary> LinkLibraries;
-  for (const auto &ll : clangModuleDep.LinkLibraries)
-    LinkLibraries.emplace_back(ll.Library,
-                               ll.IsFramework ? LibraryKind::Framework
-                                              : LibraryKind::Library,
-                               /*static=*/false);
-
-  // Module-level dependencies.
-  llvm::StringSet<> alreadyAddedModules;
-  auto bridgedDependencyInfo = ModuleDependencyInfo::forClangModule(
-      pcmPath, mappedPCMPath, clangModuleDep.ClangModuleMapFile,
-      clangModuleDep.ID.ContextHash, swiftArgs, fileDeps, LinkLibraries, RootID,
-      IncludeTree, /*module-cache-key*/ "", clangModuleDep.IsSystem);
-
-  std::vector<ModuleDependencyID> directDependencyIDs;
-  for (const auto &moduleName : clangModuleDep.ClangModuleDeps) {
-    // FIXME: This assumes, conservatively, that all Clang module imports
-    // are exported. We need to fix this once the clang scanner gains the
-    // appropriate API to query this.
-    bridgedDependencyInfo.addModuleImport(
-        moduleName.ModuleName, /* isExported */ true, AccessLevel::Public,
-        &alreadyAddedModules);
-    // It is safe to assume that all dependencies of a Clang module are Clang
-    // modules.
-    directDependencyIDs.push_back(
-        {moduleName.ModuleName, ModuleDependencyKind::Clang});
-  }
-  bridgedDependencyInfo.setImportedClangDependencies(directDependencyIDs);
-  return bridgedDependencyInfo;
-}
-
 void
 swift::dependencies::registerBackDeployLibraries(
     const IRGenOptions &IRGenOpts,
@@ -895,9 +781,8 @@ void ModuleDependenciesCache::recordDependency(
 
 void ModuleDependenciesCache::recordClangDependencies(
     const clang::tooling::dependencies::ModuleDepsGraph &dependencies,
-    const ASTContext &ctx,
-    dependencies::LookupModuleOutputCallback lookupModuleOutput,
-    dependencies::RemapPathCallback remapPath) {
+    DiagnosticEngine &diags,
+    BridgeClangDependencyCallback bridgeClangModule) {
   for (const auto &dep : dependencies) {
     auto depID =
         ModuleDependencyID{dep.ID.ModuleName, ModuleDependencyKind::Clang};
@@ -917,33 +802,29 @@ void ModuleDependenciesCache::recordClangDependencies(
         //
         // Emit a failure diagnostic here that is hopefully more actionable
         // for the time being.
-        ctx.Diags.diagnose(SourceLoc(),
+        diags.diagnose(SourceLoc(),
                            diag::dependency_scan_unexpected_variant,
                            dep.ID.ModuleName);
-        ctx.Diags.diagnose(
+        diags.diagnose(
             SourceLoc(),
             diag::dependency_scan_unexpected_variant_context_hash_note,
             priorContextHash, newContextHash);
-        ctx.Diags.diagnose(
+        diags.diagnose(
             SourceLoc(),
             diag::dependency_scan_unexpected_variant_module_map_note,
             priorClangModuleDetails->moduleMapFile, dep.ClangModuleMapFile);
 
-        auto newClangModuleDetails =
-            dependencies::bridgeClangModuleDependency(
-                ctx, dep, lookupModuleOutput, remapPath)
-                .getAsClangModule();
-
+        auto newClangModuleDetails = bridgeClangModule(dep).getAsClangModule();
         auto diagnoseExtraCommandLineFlags =
-            [&ctx](const ClangModuleDependencyStorage *checkModuleDetails,
+            [&diags](const ClangModuleDependencyStorage *checkModuleDetails,
                    const ClangModuleDependencyStorage *baseModuleDetails,
                    bool isNewlyDiscovered) -> void {
           std::unordered_set<std::string> baseCommandLineSet(
               baseModuleDetails->buildCommandLine.begin(),
               baseModuleDetails->buildCommandLine.end());
           for (const auto &checkArg : checkModuleDetails->buildCommandLine)
             if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
-              ctx.Diags.diagnose(
+              diags.diagnose(
                   SourceLoc(),
                   diag::dependency_scan_unexpected_variant_extra_arg_note,
                   isNewlyDiscovered, checkArg);
@@ -954,9 +835,7 @@ void ModuleDependenciesCache::recordClangDependencies(
                                       priorClangModuleDetails, false);
       }
     } else {
-      recordDependency(dep.ID.ModuleName,
-                       dependencies::bridgeClangModuleDependency(
-                           ctx, dep, lookupModuleOutput, remapPath));
+      recordDependency(dep.ID.ModuleName, bridgeClangModule(dep));
       addSeenClangModule(dep.ID);
     }
   }