[6.1 🍒][Dependency Scanning] Refine cross-import overlay detection algorithm

include/swift/AST/ModuleDependencies.h

@@ -152,12 +152,14 @@ struct ScannerImportStatementInfo {
     uint32_t columnNumber;
   };
 
-  ScannerImportStatementInfo(std::string importIdentifier)
-      : importLocations(), importIdentifier(importIdentifier) {}
+  ScannerImportStatementInfo(std::string importIdentifier, bool isExported)
+      : importLocations(), importIdentifier(importIdentifier),
+        isExported(isExported) {}
 
-  ScannerImportStatementInfo(std::string importIdentifier,
+  ScannerImportStatementInfo(std::string importIdentifier, bool isExported,
                              ImportDiagnosticLocationInfo location)
-      : importLocations({location}), importIdentifier(importIdentifier) {}
+      : importLocations({location}), importIdentifier(importIdentifier),
+        isExported(isExported) {}
 
   void addImportLocation(ImportDiagnosticLocationInfo location) {
     importLocations.push_back(location);
@@ -167,6 +169,8 @@ struct ScannerImportStatementInfo {
   SmallVector<ImportDiagnosticLocationInfo, 4> importLocations;
   /// Imported module string. e.g. "Foo.Bar" in 'import Foo.Bar'
   std::string importIdentifier;
+  /// Is this an @_exported import
+  bool isExported;
 };
 
 /// Base class for the variant storage of ModuleDependencyInfo.
@@ -909,7 +913,7 @@ class ModuleDependencyInfo {
 
   /// Add a dependency on the given module, if it was not already in the set.
   void
-  addOptionalModuleImport(StringRef module,
+  addOptionalModuleImport(StringRef module, bool isExported,
                           llvm::StringSet<> *alreadyAddedModules = nullptr);
 
   /// Add all of the module imports in the given source
@@ -919,13 +923,13 @@ class ModuleDependencyInfo {
                         const SourceManager *sourceManager);
 
   /// Add a dependency on the given module, if it was not already in the set.
-  void addModuleImport(ImportPath::Module module,
+  void addModuleImport(ImportPath::Module module, bool isExported,
                        llvm::StringSet<> *alreadyAddedModules = nullptr,
                        const SourceManager *sourceManager = nullptr,
                        SourceLoc sourceLocation = SourceLoc());
 
   /// Add a dependency on the given module, if it was not already in the set.
-  void addModuleImport(StringRef module,
+  void addModuleImport(StringRef module, bool isExported,
                        llvm::StringSet<> *alreadyAddedModules = nullptr,
                        const SourceManager *sourceManager = nullptr,
                        SourceLoc sourceLocation = SourceLoc());
@@ -962,7 +966,7 @@ class ModuleDependencyInfo {
   llvm::StringMap<llvm::SmallSetVector<Identifier, 4>>
   collectCrossImportOverlayNames(
       ASTContext &ctx, StringRef moduleName,
-      std::vector<std::pair<std::string, std::string>> &overlayFiles) const;
+      std::set<std::pair<std::string, std::string>> &overlayFiles) const;
 };
 
 using ModuleDependencyVector =

include/swift/DependencyScan/ModuleDependencyScanner.h

@@ -100,52 +100,47 @@ class ModuleDependencyScanner {
   /// closure for the given module.
   /// 1. Swift modules imported directly or via another Swift dependency
   /// 2. Clang modules imported directly or via a Swift dependency
-  /// 3. Clang modules imported via textual header inputs to Swift modules (bridging headers)
-  /// 4. Swift overlay modules of all of the transitively imported Clang modules that have one
+  /// 3. Clang modules imported via textual header inputs to Swift modules
+  /// (bridging headers)
+  /// 4. Swift overlay modules of all of the transitively imported Clang modules
+  /// that have one
   ModuleDependencyIDSetVector
   resolveImportedModuleDependencies(const ModuleDependencyID &rootModuleID,
                                     ModuleDependenciesCache &cache);
-  void
-  resolveSwiftModuleDependencies(const ModuleDependencyID &rootModuleID,
-                                 ModuleDependenciesCache &cache,
-                                 ModuleDependencyIDSetVector &discoveredSwiftModules);
-  void
-  resolveAllClangModuleDependencies(ArrayRef<ModuleDependencyID> swiftModules,
-                                    ModuleDependenciesCache &cache,
-                                    ModuleDependencyIDSetVector &discoveredClangModules);
-  void
-  resolveHeaderDependencies(ArrayRef<ModuleDependencyID> swiftModules,
-                            ModuleDependenciesCache &cache,
-                            ModuleDependencyIDSetVector &discoveredHeaderDependencyClangModules);
-  void
-  resolveSwiftOverlayDependencies(ArrayRef<ModuleDependencyID> swiftModules,
-                                  ModuleDependenciesCache &cache,
-                                  ModuleDependencyIDSetVector &discoveredDependencies);
+  void resolveSwiftModuleDependencies(
+      const ModuleDependencyID &rootModuleID, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &discoveredSwiftModules);
+  void resolveAllClangModuleDependencies(
+      ArrayRef<ModuleDependencyID> swiftModules, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &discoveredClangModules);
+  void resolveHeaderDependencies(
+      ArrayRef<ModuleDependencyID> swiftModules, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &discoveredHeaderDependencyClangModules);
+  void resolveSwiftOverlayDependencies(
+      ArrayRef<ModuleDependencyID> swiftModules, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &discoveredDependencies);
 
   /// Resolve all of a given module's imports to a Swift module, if one exists.
-  void
-  resolveSwiftImportsForModule(const ModuleDependencyID &moduleID,
-                               ModuleDependenciesCache &cache,
-                               ModuleDependencyIDSetVector &importedSwiftDependencies);
+  void resolveSwiftImportsForModule(
+      const ModuleDependencyID &moduleID, ModuleDependenciesCache &cache,
+      ModuleDependencyIDSetVector &importedSwiftDependencies);
 
-  /// If a module has a bridging header or other header inputs, execute a dependency scan
-  /// on it and record the dependencies.
+  /// If a module has a bridging header or other header inputs, execute a
+  /// dependency scan on it and record the dependencies.
   void resolveHeaderDependenciesForModule(
       const ModuleDependencyID &moduleID, ModuleDependenciesCache &cache,
       ModuleDependencyIDSetVector &headerClangModuleDependencies);
 
   /// Resolve all module dependencies comprised of Swift overlays
   /// of this module's Clang module dependencies.
   void resolveSwiftOverlayDependenciesForModule(
-      const ModuleDependencyID &moduleID,
-      ModuleDependenciesCache &cache,
+      const ModuleDependencyID &moduleID, ModuleDependenciesCache &cache,
       ModuleDependencyIDSetVector &swiftOverlayDependencies);
 
-  /// Identify all cross-import overlay modules of the specified
-  /// dependency set and apply an action for each.
-  void discoverCrossImportOverlayDependencies(
-      StringRef mainModuleName, ArrayRef<ModuleDependencyID> allDependencies,
-      ModuleDependenciesCache &cache,
+  /// Identify all cross-import overlay module dependencies of the
+  /// source module under scan and apply an action for each.
+  void resolveCrossImportOverlayDependencies(
+      StringRef mainModuleName, ModuleDependenciesCache &cache,
       llvm::function_ref<void(ModuleDependencyID)> action);
 
   /// Perform an operation utilizing one of the Scanning workers

include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

@@ -57,6 +57,13 @@ using IsFrameworkField = BCFixed<1>;
 using IsSystemField = BCFixed<1>;
 /// A bit that indicates whether or not a module is that of a static archive
 using IsStaticField = BCFixed<1>;
+/// A bit that indicates whether or not an import statement is @_exported
+using IsExportedImport = BCFixed<1>;
+
+/// Source location fields
+using LineNumberField = BCFixed<32>;
+using ColumnNumberField = BCFixed<32>;
+
 
 /// Arrays of various identifiers, distinguished for readability
 using IdentifierIDArryField = llvm::BCArray<IdentifierIDField>;

include/swift/Serialization/SerializedModuleLoader.h

@@ -169,6 +169,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
 
   struct BinaryModuleImports {
     llvm::StringSet<> moduleImports;
+    llvm::StringSet<> exportedModules;
     std::string headerImport;
   };
 
@@ -183,7 +184,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
 
   /// If the module has a package name matching the one
   /// specified, return a set of package-only imports for this module.
-  static llvm::ErrorOr<llvm::StringSet<>>
+  static llvm::ErrorOr<std::vector<ScannerImportStatementInfo>>
   getMatchingPackageOnlyImportsOfModule(Twine modulePath,
                                         bool isFramework,
                                         bool isRequiredOSSAModules,

lib/AST/ModuleDependencies.cpp

@@ -115,13 +115,13 @@ bool ModuleDependencyInfo::isTestableImport(StringRef moduleName) const {
 }
 
 void ModuleDependencyInfo::addOptionalModuleImport(
-    StringRef module, llvm::StringSet<> *alreadyAddedModules) {
+    StringRef module, bool isExported, llvm::StringSet<> *alreadyAddedModules) {
   if (!alreadyAddedModules || alreadyAddedModules->insert(module).second)
-    storage->optionalModuleImports.push_back(module.str());
+    storage->optionalModuleImports.push_back({module.str(), isExported});
 }
 
 void ModuleDependencyInfo::addModuleImport(
-    StringRef module, llvm::StringSet<> *alreadyAddedModules,
+    StringRef module, bool isExported, llvm::StringSet<> *alreadyAddedModules,
     const SourceManager *sourceManager, SourceLoc sourceLocation) {
   auto scannerImportLocToDiagnosticLocInfo =
       [&sourceManager](SourceLoc sourceLocation) {
@@ -135,14 +135,16 @@ void ModuleDependencyInfo::addModuleImport(
                              sourceManager->isOwning(sourceLocation);
 
   if (alreadyAddedModules && alreadyAddedModules->contains(module)) {
-    if (validSourceLocation) {
-      // Find a prior import of this module and add import location
-      for (auto &existingImport : storage->moduleImports) {
-        if (existingImport.importIdentifier == module) {
+    // Find a prior import of this module and add import location
+    // and adjust whether or not this module is ever imported as exported
+    for (auto &existingImport : storage->moduleImports) {
+      if (existingImport.importIdentifier == module) {
+        if (validSourceLocation) {
           existingImport.addImportLocation(
-              scannerImportLocToDiagnosticLocInfo(sourceLocation));
-          break;
+            scannerImportLocToDiagnosticLocInfo(sourceLocation));
         }
+        existingImport.isExported |= isExported;
+        break;
       }
     }
   } else {
@@ -151,15 +153,15 @@ void ModuleDependencyInfo::addModuleImport(
 
     if (validSourceLocation)
       storage->moduleImports.push_back(ScannerImportStatementInfo(
-          module.str(), scannerImportLocToDiagnosticLocInfo(sourceLocation)));
+          module.str(), isExported, scannerImportLocToDiagnosticLocInfo(sourceLocation)));
     else
       storage->moduleImports.push_back(
-          ScannerImportStatementInfo(module.str()));
+         ScannerImportStatementInfo(module.str(), isExported));
   }
 }
 
 void ModuleDependencyInfo::addModuleImport(
-    ImportPath::Module module, llvm::StringSet<> *alreadyAddedModules,
+    ImportPath::Module module, bool isExported, llvm::StringSet<> *alreadyAddedModules,
     const SourceManager *sourceManager, SourceLoc sourceLocation) {
   std::string ImportedModuleName = module.front().Item.str().str();
   auto submodulePath = module.getSubmodulePath();
@@ -172,7 +174,7 @@ void ModuleDependencyInfo::addModuleImport(
                               alreadyAddedModules);
   }
 
-  addModuleImport(ImportedModuleName, alreadyAddedModules,
+  addModuleImport(ImportedModuleName, isExported, alreadyAddedModules,
                   sourceManager, sourceLocation);
 }
 
@@ -201,7 +203,8 @@ void ModuleDependencyInfo::addModuleImports(
               importDecl->isExported()))
         continue;
 
-      addModuleImport(realPath, &alreadyAddedModules, sourceManager,
+      addModuleImport(realPath, importDecl->isExported(),
+                      &alreadyAddedModules, sourceManager,
                       importDecl->getLoc());
 
       // Additionally, keep track of which dependencies of a Source

lib/AST/ModuleLoader.cpp

@@ -208,7 +208,7 @@ void ModuleLoader::findOverlayFiles(SourceLoc diagLoc, ModuleDecl *module,
 llvm::StringMap<llvm::SmallSetVector<Identifier, 4>>
 ModuleDependencyInfo::collectCrossImportOverlayNames(
     ASTContext &ctx, StringRef moduleName,
-    std::vector<std::pair<std::string, std::string>> &overlayFiles) const {
+    std::set<std::pair<std::string, std::string>> &overlayFiles) const {
   using namespace llvm::sys;
   using namespace file_types;
   std::optional<std::string> modulePath;
@@ -260,7 +260,7 @@ ModuleDependencyInfo::collectCrossImportOverlayNames(
       ModuleDecl::collectCrossImportOverlay(ctx, file, moduleName,
                                             bystandingModule);
     result[bystandingModule] = std::move(overlayNames);
-    overlayFiles.push_back({moduleName.str(), file.str()});
+    overlayFiles.insert({moduleName.str(), file.str()});
   });
   return result;
 }

lib/ClangImporter/ClangModuleDependencyScanner.cpp

@@ -302,7 +302,10 @@ ModuleDependencyVector ClangImporter::bridgeClangModuleDependencies(
 
     std::vector<ModuleDependencyID> directDependencyIDs;
     for (const auto &moduleName : clangModuleDep.ClangModuleDeps) {
-      dependencies.addModuleImport(moduleName.ModuleName, &alreadyAddedModules);
+      // 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.
+      dependencies.addModuleImport(moduleName.ModuleName, /* isExported */ true, &alreadyAddedModules);
       // It is safe to assume that all dependencies of a Clang module are Clang modules.
       directDependencyIDs.push_back({moduleName.ModuleName, ModuleDependencyKind::Clang});
     }

lib/DependencyScan/ModuleDependencyCacheSerialization.cpp

@@ -228,16 +228,6 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
       currentContextHashID = contextHashID;
       auto importStrings = getStringArray(moduleImportsArrayID);
       auto optionalImportStrings = getStringArray(optionalModuleImportsArrayID);
-      if (importStrings.has_value()) {
-        for (const auto &is : importStrings.value())
-          currentModuleImports.push_back(is);
-      }
-
-      if (optionalImportStrings.has_value()) {
-        for (const auto &ois : optionalImportStrings.value())
-          currentOptionalModuleImports.push_back(ois);
-      }
-
       auto optionalCurrentModuleDependencyIDs = getModuleDependencyIDArray(moduleDependencyIDArrayID);
       if (!optionalCurrentModuleDependencyIDs)
         llvm::report_fatal_error("Bad direct dependencies: no qualified dependencies");
@@ -319,10 +309,10 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
 
       // Add imports of this module
       for (const auto &moduleName : currentModuleImports)
-        moduleDep.addModuleImport(moduleName.importIdentifier);
+        moduleDep.addModuleImport(moduleName.importIdentifier, false);
       // Add optional imports of this module
       for (const auto &moduleName : currentOptionalModuleImports)
-        moduleDep.addOptionalModuleImport(moduleName.importIdentifier);
+        moduleDep.addOptionalModuleImport(moduleName.importIdentifier, false);
 
       // Add qualified dependencies of this module
       std::vector<ModuleDependencyID> swiftDeps;
@@ -442,10 +432,10 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
 
       // Add dependencies of this module
       for (const auto &moduleName : currentModuleImports)
-        moduleDep.addModuleImport(moduleName.importIdentifier);
+        moduleDep.addModuleImport(moduleName.importIdentifier, false);
       // Add optional imports of this module
       for (const auto &moduleName : currentOptionalModuleImports)
-        moduleDep.addOptionalModuleImport(moduleName.importIdentifier);
+        moduleDep.addOptionalModuleImport(moduleName.importIdentifier, false);
 
       // Add bridging header file path
       if (bridgingHeaderFileID != 0) {
@@ -600,10 +590,10 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
 
       // Add dependencies of this module
       for (const auto &moduleName : currentModuleImports)
-        moduleDep.addModuleImport(moduleName.importIdentifier);
+        moduleDep.addModuleImport(moduleName.importIdentifier, false);
       // Add optional imports of this module
       for (const auto &moduleName : currentOptionalModuleImports)
-        moduleDep.addOptionalModuleImport(moduleName.importIdentifier);
+        moduleDep.addOptionalModuleImport(moduleName.importIdentifier, false);
 
       cache.recordDependency(currentModuleName, std::move(moduleDep),
                              getContextHash());
@@ -663,10 +653,10 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
 
       // Add dependencies of this module
       for (const auto &moduleName : currentModuleImports)
-        moduleDep.addModuleImport(moduleName.importIdentifier);
+        moduleDep.addModuleImport(moduleName.importIdentifier, false);
       // Add optional imports of this module
       for (const auto &moduleName : currentOptionalModuleImports)
-        moduleDep.addOptionalModuleImport(moduleName.importIdentifier);
+        moduleDep.addOptionalModuleImport(moduleName.importIdentifier, false);
 
       cache.recordDependency(currentModuleName, std::move(moduleDep),
                              getContextHash());