Move generation of a named import path 'Identifier' out of the individual scanning workers up into the parent scanner. This operation mutates the scanner ASTContext by potentially adding new identifiers to it and is therefore not thread-safe.

artemcm · artemcm · commit 87e7c3a9fc3e · 2023-12-04T14:29:12.000-08:00
diff --git a/include/swift/AST/ModuleLoader.h b/include/swift/AST/ModuleLoader.h
@@ -341,7 +341,8 @@ class ModuleLoader {
   /// Retrieve the dependencies for the given, named module, or \c None
   /// if no such module exists.
   virtual llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
-  getModuleDependencies(StringRef moduleName,
+  getModuleDependencies(ImportPath::Module namedImport,
+                        StringRef moduleName,
                         StringRef moduleOutputPath,
                         llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
                         const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h
@@ -440,7 +440,7 @@ class ClangImporter final : public ClangModuleLoader {
       StringRef moduleOutputPath, RemapPathCallback remapPath = nullptr);
 
   llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
-  getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
+  getModuleDependencies(ImportPath::Module namedImport, StringRef moduleName, StringRef moduleOutputPath,
                         llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
                         const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
                         clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,
diff --git a/include/swift/DependencyScan/ModuleDependencyScanner.h b/include/swift/DependencyScan/ModuleDependencyScanner.h
@@ -36,18 +36,21 @@ class ModuleDependencyScanningWorker {
 private:
   /// Retrieve the module dependencies for the module with the given name.
   ModuleDependencyVector
-  scanFilesystemForModuleDependency(StringRef moduleName,
+  scanFilesystemForModuleDependency(ImportPath::Module namedImport,
+                                    StringRef moduleName,
                                     const ModuleDependenciesCache &cache,
                                     bool isTestableImport = false);
 
   /// Retrieve the module dependencies for the Clang module with the given name.
   ModuleDependencyVector
-  scanFilesystemForClangModuleDependency(StringRef moduleName,
+  scanFilesystemForClangModuleDependency(ImportPath::Module namedImport,
+                                         StringRef moduleName,
                                          const ModuleDependenciesCache &cache);
 
   /// Retrieve the module dependencies for the Swift module with the given name.
   ModuleDependencyVector
-  scanFilesystemForSwiftModuleDependency(StringRef moduleName,
+  scanFilesystemForSwiftModuleDependency(ImportPath::Module namedImport,
+                                         StringRef moduleName,
                                          const ModuleDependenciesCache &cache);
 
   // An AST delegate for interface scanning.
@@ -135,6 +138,9 @@ class ModuleDependencyScanner {
   template <typename Function, typename... Args>
   auto withDependencyScanningWorker(Function &&F, Args &&...ArgList);
 
+  /// Generate a full named import path
+  ImportPath::Module getModuleImportIdentifier(StringRef moduleName);
+
 private:
   const CompilerInvocation &ScanCompilerInvocation;
   ASTContext &ScanASTContext;
diff --git a/include/swift/Sema/SourceLoader.h b/include/swift/Sema/SourceLoader.h
@@ -98,7 +98,7 @@ class SourceLoader : public ModuleLoader {
   }
 
   llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
-  getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
+  getModuleDependencies(ImportPath::Module namedImport, StringRef moduleName, StringRef moduleOutputPath,
                         llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
                         const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
                         clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,
diff --git a/include/swift/Serialization/SerializedModuleLoader.h b/include/swift/Serialization/SerializedModuleLoader.h
@@ -250,7 +250,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
   virtual void verifyAllModules() override;
 
   virtual llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
-  getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
+  getModuleDependencies(ImportPath::Module namedImport, StringRef moduleName, StringRef moduleOutputPath,
                         llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
                         const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
                         clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,
diff --git a/lib/ClangImporter/ClangModuleDependencyScanner.cpp b/lib/ClangImporter/ClangModuleDependencyScanner.cpp
@@ -407,7 +407,8 @@ computeClangWorkingDirectory(const std::vector<std::string> &commandLineArgs,
 }
 
 ModuleDependencyVector
-ClangImporter::getModuleDependencies(StringRef moduleName,
+ClangImporter::getModuleDependencies(ImportPath::Module namedImport,
+                                     StringRef moduleName,
                                      StringRef moduleOutputPath,
                                      llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
                                      const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
diff --git a/lib/DependencyScan/ModuleDependencyScanner.cpp b/lib/DependencyScan/ModuleDependencyScanner.cpp
@@ -179,20 +179,20 @@ ModuleDependencyScanningWorker::ModuleDependencyScanningWorker(
 
 ModuleDependencyVector
 ModuleDependencyScanningWorker::scanFilesystemForModuleDependency(
-    StringRef moduleName, const ModuleDependenciesCache &cache,
+    ImportPath::Module namedImport, StringRef moduleName, const ModuleDependenciesCache &cache,
     bool isTestableImport) {
   // First query a Swift module, otherwise lookup a Clang module
   ModuleDependencyVector moduleDependencies =
       swiftScannerModuleLoader->getModuleDependencies(
-          moduleName, cache.getModuleOutputPath(),
+          namedImport, moduleName, cache.getModuleOutputPath(),
           cache.getScanService().getCachingFS(),
           cache.getAlreadySeenClangModules(), clangScanningTool,
           *ScanningASTDelegate, cache.getScanService().getPrefixMapper(),
           isTestableImport);
 
   if (moduleDependencies.empty())
     moduleDependencies = clangScannerModuleLoader->getModuleDependencies(
-        moduleName, cache.getModuleOutputPath(),
+        namedImport, moduleName, cache.getModuleOutputPath(),
         cache.getScanService().getCachingFS(),
         cache.getAlreadySeenClangModules(), clangScanningTool,
         *ScanningASTDelegate, cache.getScanService().getPrefixMapper(),
@@ -203,19 +203,19 @@ ModuleDependencyScanningWorker::scanFilesystemForModuleDependency(
 
 ModuleDependencyVector
 ModuleDependencyScanningWorker::scanFilesystemForSwiftModuleDependency(
-    StringRef moduleName, const ModuleDependenciesCache &cache) {
+    ImportPath::Module namedImport, StringRef moduleName, const ModuleDependenciesCache &cache) {
   return swiftScannerModuleLoader->getModuleDependencies(
-      moduleName, cache.getModuleOutputPath(),
+      namedImport, moduleName, cache.getModuleOutputPath(),
       cache.getScanService().getCachingFS(), cache.getAlreadySeenClangModules(),
       clangScanningTool, *ScanningASTDelegate,
       cache.getScanService().getPrefixMapper(), false);
 }
 
 ModuleDependencyVector
 ModuleDependencyScanningWorker::scanFilesystemForClangModuleDependency(
-    StringRef moduleName, const ModuleDependenciesCache &cache) {
+    ImportPath::Module namedImport, StringRef moduleName, const ModuleDependenciesCache &cache) {
   return clangScannerModuleLoader->getModuleDependencies(
-      moduleName, cache.getModuleOutputPath(),
+      namedImport, moduleName, cache.getModuleOutputPath(),
       cache.getScanService().getCachingFS(), cache.getAlreadySeenClangModules(),
       clangScanningTool, *ScanningASTDelegate,
       cache.getScanService().getPrefixMapper(), false);
@@ -252,6 +252,11 @@ auto ModuleDependencyScanner::withDependencyScanningWorker(Function &&F,
   return result;
 }
 
+ImportPath::Module ModuleDependencyScanner::getModuleImportIdentifier(StringRef moduleName) {
+  ImportPath::Module::Builder builder(ScanASTContext, moduleName, /*separator=*/'.');
+  return builder.get();
+}
+
 ModuleDependencyScanner::ModuleDependencyScanner(
     SwiftDependencyScanningService &ScanningService,
     const CompilerInvocation &ScanCompilerInvocation,
@@ -443,10 +448,11 @@ ModuleDependencyScanner::getNamedClangModuleDependencyInfo(
     return found;
 
   // Otherwise perform filesystem scan
+  auto namedImportIdentifier = getModuleImportIdentifier(moduleName);
   auto moduleDependencies = withDependencyScanningWorker(
-      [&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
+      [&cache, namedImportIdentifier, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
         return ScanningWorker->scanFilesystemForClangModuleDependency(
-            moduleName, cache);
+            namedImportIdentifier, moduleName, cache);
       });
   if (moduleDependencies.empty())
     return llvm::None;
@@ -473,11 +479,14 @@ ModuleDependencyScanner::getNamedSwiftModuleDependencyInfo(
                                         ModuleDependencyKind::SwiftPlaceholder))
     return found;
 
+  ImportPath::Module::Builder builder(ScanASTContext, moduleName, /*separator=*/'.');
+  auto modulePath = builder.get();
   // Otherwise perform filesystem scan
+  auto namedImportIdentifier = getModuleImportIdentifier(moduleName);
   auto moduleDependencies = withDependencyScanningWorker(
-      [&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
+      [&cache, namedImportIdentifier, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
         return ScanningWorker->scanFilesystemForSwiftModuleDependency(
-            moduleName, cache);
+            namedImportIdentifier, moduleName, cache);
       });
   if (moduleDependencies.empty())
     return llvm::None;
@@ -552,6 +561,7 @@ void ModuleDependencyScanner::resolveImportDependencies(
   // A scanning task to query a module by-name. If the module already exists
   // in the cache, do nothing and return.
   auto scanForModuleDependency = [this, &cache, &moduleLookupResult](
+                                  ImportPath::Module namedImportIdentifier,
                                   StringRef moduleName, bool onlyClangModule,
                                   bool isTestable) {
     // If this is already in the cache, no work to do here
@@ -564,13 +574,13 @@ void ModuleDependencyScanner::resolveImportDependencies(
     }
 
     auto moduleDependencies = withDependencyScanningWorker(
-        [&cache, moduleName, onlyClangModule,
+        [&cache, namedImportIdentifier, moduleName, onlyClangModule,
          isTestable](ModuleDependencyScanningWorker *ScanningWorker) {
           return onlyClangModule
                      ? ScanningWorker->scanFilesystemForClangModuleDependency(
-                           moduleName, cache)
+                          namedImportIdentifier, moduleName, cache)
                      : ScanningWorker->scanFilesystemForModuleDependency(
-                           moduleName, cache, isTestable);
+                          namedImportIdentifier, moduleName, cache, isTestable);
         });
     moduleLookupResult.insert_or_assign(moduleName, moduleDependencies);
   };
@@ -579,14 +589,16 @@ void ModuleDependencyScanner::resolveImportDependencies(
   for (const auto &dependsOn : moduleDependencyInfo->getModuleImports()) {
     bool underlyingClangModuleLookup = moduleID.ModuleName == dependsOn;
     bool isTestable = moduleDependencyInfo->isTestableImport(dependsOn);
-    ScanningThreadPool.async(scanForModuleDependency, dependsOn,
+    ScanningThreadPool.async(scanForModuleDependency,
+                             getModuleImportIdentifier(dependsOn), dependsOn,
                              underlyingClangModuleLookup, isTestable);
   }
   for (const auto &dependsOn :
        moduleDependencyInfo->getOptionalModuleImports()) {
     bool underlyingClangModuleLookup = moduleID.ModuleName == dependsOn;
     bool isTestable = moduleDependencyInfo->isTestableImport(dependsOn);
-    ScanningThreadPool.async(scanForModuleDependency, dependsOn,
+    ScanningThreadPool.async(scanForModuleDependency,
+                             getModuleImportIdentifier(dependsOn), dependsOn,
                              underlyingClangModuleLookup, isTestable);
   }
   ScanningThreadPool.wait();
@@ -726,23 +738,28 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependencies(
   // A scanning task to query a Swift module by-name. If the module already
   // exists in the cache, do nothing and return.
   auto scanForSwiftDependency = [this, &cache, &swiftOverlayLookupResult](
+                                    ImportPath::Module namedImportIdentifier,
                                     StringRef moduleName) {
     if (cache.hasDependency(moduleName, ModuleDependencyKind::SwiftInterface) ||
         cache.hasDependency(moduleName, ModuleDependencyKind::SwiftBinary) ||
         cache.hasDependency(moduleName, ModuleDependencyKind::SwiftPlaceholder))
       return;
 
     auto moduleDependencies = withDependencyScanningWorker(
-        [&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
-          return ScanningWorker->scanFilesystemForSwiftModuleDependency(moduleName,
+        [&cache, namedImportIdentifier, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
+          return ScanningWorker->scanFilesystemForSwiftModuleDependency(namedImportIdentifier,
+                                                                        moduleName,
                                                                         cache);
         });
     swiftOverlayLookupResult.insert_or_assign(moduleName, moduleDependencies);
   };
 
   // Enque asynchronous lookup tasks
   for (const auto &clangDep : clangDependencies)
-    ScanningThreadPool.async(scanForSwiftDependency, clangDep);
+    ScanningThreadPool.async(scanForSwiftDependency,
+                             getModuleImportIdentifier(clangDep),
+                             clangDep);
+
   ScanningThreadPool.wait();
 
   // Aggregate both previously-cached and freshly-scanned module results
diff --git a/lib/Sema/SourceLoader.cpp b/lib/Sema/SourceLoader.cpp
@@ -155,7 +155,8 @@ void SourceLoader::loadExtensions(NominalTypeDecl *nominal,
 }
 
 ModuleDependencyVector
-SourceLoader::getModuleDependencies(StringRef moduleName,
+SourceLoader::getModuleDependencies(ImportPath::Module namedImport,
+                                    StringRef moduleName,
                                     StringRef moduleOutputPath,
                                     llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
                                     const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
diff --git a/lib/Serialization/ScanningLoaders.cpp b/lib/Serialization/ScanningLoaders.cpp
@@ -305,16 +305,14 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath,
 }
 
 ModuleDependencyVector SerializedModuleLoaderBase::getModuleDependencies(
-    StringRef moduleName, StringRef moduleOutputPath,
+    ImportPath::Module namedImport, StringRef moduleName, StringRef moduleOutputPath,
     llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
     const llvm::DenseSet<clang::tooling::dependencies::ModuleID>
         &alreadySeenClangModules,
     clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,
     InterfaceSubContextDelegate &delegate, llvm::TreePathPrefixMapper *mapper,
     bool isTestableDependencyLookup) {
-  ImportPath::Module::Builder builder(Ctx, moduleName, /*separator=*/'.');
-  auto modulePath = builder.get();
-  auto moduleId = modulePath.front().Item;
+  auto moduleId = namedImport.front().Item;
   llvm::Optional<SwiftDependencyTracker> tracker = llvm::None;
   if (CacheFS)
     tracker = SwiftDependencyTracker(*CacheFS, mapper);
@@ -339,7 +337,7 @@ ModuleDependencyVector SerializedModuleLoaderBase::getModuleDependencies(
          "Expected PlaceholderSwiftModuleScanner as the first dependency "
          "scanner loader.");
   for (auto &scanner : scanners) {
-    if (scanner->canImportModule(modulePath, nullptr,
+    if (scanner->canImportModule(namedImport, nullptr,
                                  isTestableDependencyLookup)) {
 
       ModuleDependencyVector moduleDependnecies;

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ class SourceLoader : public ModuleLoader {`
`98`	`98`	`}`
`99`	`99`
`100`	`100`	`llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>`
`101`		`- getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,`
	`101`	`+ getModuleDependencies(ImportPath::Module namedImport, StringRef moduleName, StringRef moduleOutputPath,`
`102`	`102`	`llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,`
`103`	`103`	`const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,`
`104`	`104`	`clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,`
Original file line number	Diff line number	Diff line change
`@@ -407,7 +407,8 @@ computeClangWorkingDirectory(const std::vector<std::string> &commandLineArgs,`
`407`	`407`	`}`
`408`	`408`
`409`	`409`	`ModuleDependencyVector`
`410`		`-ClangImporter::getModuleDependencies(StringRef moduleName,`
	`410`	`+ClangImporter::getModuleDependencies(ImportPath::Module namedImport,`
	`411`	`+ StringRef moduleName,`
`411`	`412`	`StringRef moduleOutputPath,`
`412`	`413`	`llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,`
`413`	`414`	`const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,`
Original file line number	Diff line number	Diff line change
`@@ -155,7 +155,8 @@ void SourceLoader::loadExtensions(NominalTypeDecl *nominal,`
`155`	`155`	`}`
`156`	`156`
`157`	`157`	`ModuleDependencyVector`
`158`		`-SourceLoader::getModuleDependencies(StringRef moduleName,`
	`158`	`+SourceLoader::getModuleDependencies(ImportPath::Module namedImport,`
	`159`	`+ StringRef moduleName,`
`159`	`160`	`StringRef moduleOutputPath,`
`160`	`161`	`llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,`
`161`	`162`	`const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,`