[Dependency Scanning] Refine cross-import overlay detection algorithm

The algorithm already performs pairwise checks on module dependencies brought into compilation per-source-file. Previously, the algorithm considered the entire sub-graph of a given source file. Actual source compiles do not consider the full transitive module dependency set for cross-import-overlay lookup, but rather only directly-imported modules in a given source file, and '@_exported import' Swift transitive dependencies.

This change adds tracking of whether a given import statement is 'exported' to the dependency scanner and then refines the cross-import overlay lookup logic to only consider transitive modules that are exported by directly-imported dependencies.

Author: artemcm

include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

@@ -56,11 +56,11 @@ 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 taht indicates whether or not a link library is a force-load one
+/// A bit that indicates whether or not a link library is a force-load one
 using IsForceLoadField = BCFixed<1>;
-/// A bit taht indicates whether or not an import statement is optional
+/// A bit that indicates whether or not an import statement is optional
 using IsOptionalImport = BCFixed<1>;
-/// A bit taht indicates whether or not an import statement is @_exported
+/// A bit that indicates whether or not an import statement is @_exported
 using IsExportedImport = BCFixed<1>;
 
 /// Source location fields

lib/AST/ModuleDependencies.cpp

@@ -126,7 +126,7 @@ void ModuleDependencyInfo::addModuleImport(
     StringRef module, bool isExported, llvm::StringSet<> *alreadyAddedModules,
     const SourceManager *sourceManager, SourceLoc sourceLocation) {
   auto scannerImportLocToDiagnosticLocInfo =
-      [&sourceManager, isExported](SourceLoc sourceLocation) {
+      [&sourceManager](SourceLoc sourceLocation) {
         auto lineAndColumnNumbers =
             sourceManager->getLineAndColumnInBuffer(sourceLocation);
         return ScannerImportStatementInfo::ImportDiagnosticLocationInfo(

lib/ClangImporter/ClangModuleDependencyScanner.cpp

@@ -281,7 +281,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/ModuleDependencyScanner.cpp

@@ -590,18 +590,19 @@ static void discoverCrossImportOverlayFiles(
       mainModuleName.str(), ModuleDependencyKind::SwiftSource});
 
   llvm::StringMap<ModuleDependencyIDSet> perSourceFileDependencies;
-  const ModuleDependencyIDSet directSwiftDepsSet{
+  const ModuleDependencyIDSet mainModuleDirectSwiftDepsSet{
       mainModuleInfo.getImportedSwiftDependencies().begin(),
       mainModuleInfo.getImportedSwiftDependencies().end()};
-  const ModuleDependencyIDSet directClangDepsSet{
+  const ModuleDependencyIDSet mainModuleDirectClangDepsSet{
       mainModuleInfo.getImportedClangDependencies().begin(),
       mainModuleInfo.getImportedClangDependencies().end()};
 
   // A utility to map an import identifier to one of the
   // known resolved module dependencies
   auto getModuleIDForImportIdentifier =
-      [directSwiftDepsSet, directClangDepsSet](
-          const std::string &importIdentifierStr) -> ModuleDependencyID {
+      [](const std::string &importIdentifierStr,
+         const ModuleDependencyIDSet &directSwiftDepsSet,
+         const ModuleDependencyIDSet &directClangDepsSet) -> ModuleDependencyID {
     if (auto textualDepIt = directSwiftDepsSet.find(
             {importIdentifierStr, ModuleDependencyKind::SwiftInterface});
         textualDepIt != directSwiftDepsSet.end())
@@ -621,8 +622,9 @@ static void discoverCrossImportOverlayFiles(
   // Collect the set of directly-imported module dependencies
   // for each source file in the source module under scan.
   for (const auto &import : mainModuleInfo.getModuleImports()) {
-    auto importResolvedModuleID =
-        getModuleIDForImportIdentifier(import.importIdentifier);
+    auto importResolvedModuleID = getModuleIDForImportIdentifier(
+        import.importIdentifier, mainModuleDirectSwiftDepsSet,
+        mainModuleDirectClangDepsSet);
     for (const auto &importLocation : import.importLocations)
       perSourceFileDependencies[importLocation.bufferIdentifier].insert(
           importResolvedModuleID);
@@ -640,19 +642,29 @@ static void discoverCrossImportOverlayFiles(
       auto moduleID = worklist.pop_back_val();
       perSourceFileDependencies[bufferIdentifier].insert(moduleID);
       if (isSwiftDependencyKind(moduleID.Kind)) {
-        for (const auto &directSwiftDepID :
-             cache.getImportedSwiftDependencies(moduleID)) {
-          if (perSourceFileDependencies[bufferIdentifier].count(directSwiftDepID))
-            continue;
-          worklist.push_back(directSwiftDepID);
+        auto moduleInfo = cache.findKnownDependency(moduleID);
+        if (llvm::any_of(moduleInfo.getModuleImports(),
+                         [](const ScannerImportStatementInfo &importInfo) {
+                           return importInfo.isExported;
+                         })) {
+          const ModuleDependencyIDSet directSwiftDepsSet{
+              moduleInfo.getImportedSwiftDependencies().begin(),
+              moduleInfo.getImportedSwiftDependencies().end()};
+          const ModuleDependencyIDSet directClangDepsSet{
+              moduleInfo.getImportedClangDependencies().begin(),
+              moduleInfo.getImportedClangDependencies().end()};
+          for (const auto &import : moduleInfo.getModuleImports()) {
+            if (import.isExported) {
+              auto importResolvedDepID = getModuleIDForImportIdentifier(
+                  import.importIdentifier, directSwiftDepsSet,
+                  directClangDepsSet);
+              if (!perSourceFileDependencies[bufferIdentifier].count(
+                      importResolvedDepID))
+                worklist.push_back(importResolvedDepID);
+            }
+          }
         }
       }
-      for (const auto &directClangDepID :
-           cache.getImportedClangDependencies(moduleID)) {
-        if (perSourceFileDependencies[bufferIdentifier].count(directClangDepID))
-          continue;
-        worklist.push_back(directClangDepID);
-      }
     }
   }
 

test/ScanDependencies/Inputs/Swift/EWrapper.swiftinterface

@@ -1,5 +1,5 @@
 // swift-interface-format-version: 1.0
 // swift-module-flags: -module-name EWrapper
 import Swift
-import E
+@_exported import E
 public func funcEWrapper() { }

test/ScanDependencies/Inputs/Swift/EWrapperNonExported.swiftinterface

@@ -0,0 +1,5 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -module-name EWrapperNonExported
+import Swift
+import E
+public func funcEWrapperNonExported() { }

test/ScanDependencies/Inputs/Swift/SubEWrapper.swiftinterface

@@ -1,5 +1,5 @@
 // swift-interface-format-version: 1.0
 // swift-module-flags: -module-name SubEWrapper
 import Swift
-import SubE
+@_exported import SubE
 public func funcSubEWrapper() { }

test/ScanDependencies/module_deps_cross_import_overlay.swift

@@ -1,11 +1,11 @@
 // RUN: %empty-directory(%t)
 // RUN: mkdir -p %t/clang-module-cache
-// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -emit-dependencies -emit-dependencies-path %t/deps.d -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -module-name CrossImportTestModule -enable-cross-import-overlays
+// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -module-name CrossImportTestModule -enable-cross-import-overlays
 // Check the contents of the JSON output
 // RUN: %validate-json %t/deps.json | %FileCheck %s
 
 // Ensure that round-trip serialization does not affect result
-// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -test-dependency-scan-cache-serialization -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -emit-dependencies -emit-dependencies-path %t/deps.d -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -module-name CrossImportTestModule -enable-cross-import-overlays
+// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -test-dependency-scan-cache-serialization -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -module-name CrossImportTestModule -enable-cross-import-overlays
 // RUN: %validate-json %t/deps.json | %FileCheck %s
 
 // REQUIRES: executable_test

test/ScanDependencies/module_deps_no_cross_import_overlay_transitive.swift

@@ -0,0 +1,16 @@
+// REQUIRES: objc_interop
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/clang-module-cache)
+// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -module-name CrossImportTestModule -enable-cross-import-overlays
+// Check the contents of the JSON output
+// RUN: %validate-json %t/deps.json | %FileCheck %s
+
+// Ensure that round-trip serialization does not affect result
+// RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -test-dependency-scan-cache-serialization -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -I %S/Inputs/CHeaders/ExtraCModules -module-name CrossImportTestModule -enable-cross-import-overlays
+// RUN: %validate-json %t/deps.json | %FileCheck %s
+
+// Ensure that if one of the two components of a cross-import overlay is a transitive (non-exported) Swift dependency, then no overlay is required to be brought in.
+import EWrapperNonExported
+import SubEWrapper
+
+// CHECK-NOT: "swift": "_cross_import_E"