DependenciesScanner: refactor cross import overlay resolver to a standalone function. NFC

This change facilitates resolving cross-import overlays from transitive dependencies.

include/swift/AST/ModuleDependencies.h

@@ -255,7 +255,7 @@ public:
 
   /// Add a dependency on the given module, if it was not already in the set.
   void addModuleDependency(StringRef module,
-                           llvm::StringSet<> &alreadyAddedModules);
+                           llvm::StringSet<> *alreadyAddedModules = nullptr);
 
   /// Add all of the module dependencies for the imports in the given source
   /// file to the set of module dependencies.

lib/AST/ModuleDependencies.cpp

@@ -37,8 +37,8 @@ ModuleDependencies::getAsClangModule() const {
 }
 
 void ModuleDependencies::addModuleDependency(
-    StringRef module, llvm::StringSet<> &alreadyAddedModules) {
+    StringRef module, llvm::StringSet<> *alreadyAddedModules) {
-  if (alreadyAddedModules.insert(module).second)
+  if (!alreadyAddedModules || alreadyAddedModules->insert(module).second)
     storage->moduleDependencies.push_back(module.str());
 }
 
@@ -53,7 +53,7 @@ void ModuleDependencies::addModuleDependencies(
       continue;
 
     addModuleDependency(importDecl->getModulePath().front().Item.str(),
-                        alreadyAddedModules);
+                        &alreadyAddedModules);
   }
 
   auto fileName = sf.getFilename();

lib/ClangImporter/ClangModuleDependencyScanner.cpp

@@ -265,7 +265,7 @@ void ClangImporter::recordModuleDependencies(
         swiftArgs,
         fileDeps);
     for (const auto &moduleName : clangModuleDep.ClangModuleDeps) {
-      dependencies.addModuleDependency(moduleName.ModuleName, alreadyAddedModules);
+      dependencies.addModuleDependency(moduleName.ModuleName, &alreadyAddedModules);
     }
 
     cache.recordDependencies(clangModuleDep.ModuleName,

lib/FrontendTool/ScanDependencies.cpp

@@ -62,23 +62,12 @@ static void findAllImportedClangModules(ASTContext &ctx, StringRef moduleName,
 /// Resolve the direct dependencies of the given module.
 static std::vector<ModuleDependencyID> resolveDirectDependencies(
     CompilerInstance &instance, ModuleDependencyID module,
-    ModuleDependenciesCache &cache) {
+    ModuleDependenciesCache &cache,
+    InterfaceSubContextDelegate &ASTDelegate) {
   auto &ctx = instance.getASTContext();
   auto knownDependencies = *cache.findDependencies(module.first, module.second);
   auto isSwift = knownDependencies.isSwiftModule();
-  auto ModuleCachePath = getModuleCachePathFromClang(ctx
+
-    .getClangModuleLoader()->getClangInstance());
-  auto &FEOpts = instance.getInvocation().getFrontendOptions();
-  ModuleInterfaceLoaderOptions LoaderOpts(FEOpts);
-  InterfaceSubContextDelegateImpl ASTDelegate(ctx.SourceMgr, ctx.Diags,
-                                              ctx.SearchPathOpts, ctx.LangOpts,
-                                              LoaderOpts,
-                                              ctx.getClangModuleLoader(),
-                                              /*buildModuleCacheDirIfAbsent*/false,
-                                              ModuleCachePath,
-                                              FEOpts.PrebuiltModuleCachePath,
-                                              FEOpts.SerializeModuleInterfaceDependencyHashes,
-                                              FEOpts.shouldTrackSystemDependencies());
   // Find the dependencies of every module this module directly depends on.
   std::vector<ModuleDependencyID> result;
   for (auto dependsOn : knownDependencies.getModuleDependencies()) {
@@ -138,16 +127,17 @@ static std::vector<ModuleDependencyID> resolveDirectDependencies(
       }
     }
   }
-  // Only resolve cross-import overlays when this is the main module.
+  return result;
-  // For other modules, these overlays are explicitly written.
+}
-  bool isMainModule =
+
-    instance.getMainModule()->getName().str() == module.first &&
+static void discoverCrosssImportOverlayDependencies(
-    module.second == ModuleDependenciesKind::Swift;
+    CompilerInstance &instance, StringRef mainModuleName,
-  if (isMainModule) {
+    ArrayRef<ModuleDependencyID> allDependencies,
+    ModuleDependenciesCache &cache, InterfaceSubContextDelegate &ASTDelegate,
+    llvm::function_ref<void(ModuleDependencyID)> action) {
   // Modules explicitly imported. Only these can be secondary module.
-    std::vector<ModuleDependencyID> explicitImports = result;
+  llvm::SetVector<Identifier> newOverlays;
-    for (unsigned I = 0; I != result.size(); ++I) {
+  for (auto dep: allDependencies) {
-      auto dep = result[I];
     auto moduleName = dep.first;
     auto dependencies = *cache.findDependencies(moduleName, dep.second);
     // Collect a map from secondary module name to cross-import overlay names.
@@ -155,22 +145,57 @@ static std::vector<ModuleDependencyID> resolveDirectDependencies(
       instance.getASTContext(), moduleName);
     if (overlayMap.empty())
       continue;
-      std::for_each(explicitImports.begin(), explicitImports.end(),
+    std::for_each(allDependencies.begin(), allDependencies.end(),
                   [&](ModuleDependencyID Id) {
       // check if any explicitly imported modules can serve as a secondary
       // module, and add the overlay names to the dependencies list.
       for (auto overlayName: overlayMap[Id.first]) {
-          if (auto found = ctx.getModuleDependencies(overlayName.str(),
+        if (std::find_if(allDependencies.begin(), allDependencies.end(),
-                                                     /*onlyClangModule=*/false,
+            [&](ModuleDependencyID Id) { return Id.first == overlayName.str(); })
-                                                     cache,
+                == allDependencies.end()) {
-                                                     ASTDelegate)) {
+          newOverlays.insert(overlayName);
-            result.emplace_back(overlayName.str(), found->getKind());
         }
       }
     });
   }
+  // No new cross-import overlays are found, return.
+  if (newOverlays.empty())
+    return;
+  // Construct a dummy main to resolve the newly discovered cross import overlays.
+  StringRef dummyMainName = "DummyMainModuleForResolvingCrossImportOverlays";
+  auto dummyMainDependencies = ModuleDependencies::forMainSwiftModule({});
+
+  // Update main module's dependencies to include these new overlays.
+  auto mainDep = *cache.findDependencies(mainModuleName, ModuleDependenciesKind::Swift);
+  std::for_each(newOverlays.begin(), newOverlays.end(), [&](Identifier modName) {
+    dummyMainDependencies.addModuleDependency(modName.str());
+    mainDep.addModuleDependency(modName.str());
+  });
+  cache.updateDependencies({mainModuleName, ModuleDependenciesKind::Swift}, mainDep);
+
+  // Record the dummy main module's direct dependencies. The dummy main module
+  // only directly depend on these newly discovered overlay modules.
+  cache.recordDependencies(dummyMainName, dummyMainDependencies,
+                           ModuleDependenciesKind::Swift);
+  llvm::SetVector<ModuleDependencyID, std::vector<ModuleDependencyID>,
+                  std::set<ModuleDependencyID>> allModules;
+
+  // Seed the all module list from the dummpy main module.
+  allModules.insert({dummyMainName.str(), dummyMainDependencies.getKind()});
+
+  // Explore the dependencies of every module.
+  for (unsigned currentModuleIdx = 0;
+       currentModuleIdx < allModules.size();
+       ++currentModuleIdx) {
+    auto module = allModules[currentModuleIdx];
+    auto discoveredModules = resolveDirectDependencies(instance, module,
+                                                       cache, ASTDelegate);
+    allModules.insert(discoveredModules.begin(), discoveredModules.end());
   }
-  return result;
+  // Report any discovered modules to the clients, which include all overlays
+  // and their dependencies.
+  std::for_each(/* +1 to exclude dummy main*/allModules.begin() + 1,
+                allModules.end(), action);
 }
 
 /// Write a single JSON field.
@@ -259,6 +284,7 @@ namespace {
 static void writeJSON(llvm::raw_ostream &out,
                       CompilerInstance &instance,
                       ModuleDependenciesCache &cache,
+                      InterfaceSubContextDelegate &ASTDelegate,
                       ArrayRef<ModuleDependencyID> allModules) {
   // Write out a JSON description of all of the dependencies.
   out << "{\n";
@@ -277,7 +303,8 @@ static void writeJSON(llvm::raw_ostream &out,
   };
   for (const auto &module : allModules) {
     auto directDependencies = resolveDirectDependencies(
-        instance, ModuleDependencyID(module.first, module.second), cache);
+      instance, ModuleDependencyID(module.first, module.second), cache,
+      ASTDelegate);
 
     // Grab the completed module dependencies.
     auto moduleDeps = *cache.findDependencies(module.first, module.second);
@@ -469,18 +496,18 @@ bool swift::scanDependencies(CompilerInstance &instance) {
       break;
 
     case ImplicitStdlibKind::Stdlib:
-      mainDependencies.addModuleDependency("Swift", alreadyAddedModules);
+      mainDependencies.addModuleDependency("Swift", &alreadyAddedModules);
       break;
     }
 
     // Add any implicit module names.
     for (const auto &moduleName : importInfo.ModuleNames) {
-      mainDependencies.addModuleDependency(moduleName.str(), alreadyAddedModules);
+      mainDependencies.addModuleDependency(moduleName.str(), &alreadyAddedModules);
     }
 
     // Already-loaded, implicitly imported module names.
     for (const auto &module : importInfo.AdditionalModules) {
-      mainDependencies.addModuleDependency(module.first->getNameStr(), alreadyAddedModules);
+      mainDependencies.addModuleDependency(module.first->getNameStr(), &alreadyAddedModules);
     }
 
     // Add the bridging header.
@@ -492,7 +519,7 @@ bool swift::scanDependencies(CompilerInstance &instance) {
     // add a dependency with the same name to trigger the search.
     if (importInfo.ShouldImportUnderlyingModule) {
       mainDependencies.addModuleDependency(mainModule->getName().str(),
-                                           alreadyAddedModules);
+                                           &alreadyAddedModules);
     }
   }
 
@@ -508,18 +535,40 @@ bool swift::scanDependencies(CompilerInstance &instance) {
   cache.recordDependencies(mainModuleName, std::move(mainDependencies),
                            ModuleDependenciesKind::Swift);
 
+  auto &ctx = instance.getASTContext();
+  auto ModuleCachePath = getModuleCachePathFromClang(ctx
+    .getClangModuleLoader()->getClangInstance());
+  auto &FEOpts = instance.getInvocation().getFrontendOptions();
+  ModuleInterfaceLoaderOptions LoaderOpts(FEOpts);
+  InterfaceSubContextDelegateImpl ASTDelegate(ctx.SourceMgr, ctx.Diags,
+                                              ctx.SearchPathOpts, ctx.LangOpts,
+                                              LoaderOpts,
+                                              ctx.getClangModuleLoader(),
+                                              /*buildModuleCacheDirIfAbsent*/false,
+                                              ModuleCachePath,
+                                              FEOpts.PrebuiltModuleCachePath,
+                                              FEOpts.SerializeModuleInterfaceDependencyHashes,
+                                              FEOpts.shouldTrackSystemDependencies());
+
   // Explore the dependencies of every module.
   for (unsigned currentModuleIdx = 0;
        currentModuleIdx < allModules.size();
        ++currentModuleIdx) {
     auto module = allModules[currentModuleIdx];
     auto discoveredModules =
-        resolveDirectDependencies(instance, module, cache);
+        resolveDirectDependencies(instance, module, cache, ASTDelegate);
     allModules.insert(discoveredModules.begin(), discoveredModules.end());
   }
 
+  // We have all explicit imports now, resolve cross import overlays.
+  discoverCrosssImportOverlayDependencies(instance, mainModuleName,
+      /*All transitive dependencies*/allModules.getArrayRef().slice(1), cache,
+      ASTDelegate, [&](ModuleDependencyID id) {
+    allModules.insert(id);
+  });
+
   // Write out the JSON description.
-  writeJSON(out, instance, cache, allModules.getArrayRef());
+  writeJSON(out, instance, cache, ASTDelegate, allModules.getArrayRef());
 
   // Update the dependency tracker.
   if (auto depTracker = instance.getDependencyTracker()) {

lib/Serialization/SerializedModuleLoader.cpp

@@ -410,7 +410,7 @@ llvm::ErrorOr<ModuleDependencies> SerializedModuleLoaderBase::scanModuleFile(
     if (dotPos != std::string::npos)
       moduleName = moduleName.slice(0, dotPos);
 
-    dependencies.addModuleDependency(moduleName, addedModuleNames);
+    dependencies.addModuleDependency(moduleName, &addedModuleNames);
   }
 
   return std::move(dependencies);

test/ScanDependencies/Inputs/Swift/EWrapper.swiftinterface

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

test/ScanDependencies/Inputs/Swift/SubE.swiftinterface

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

test/ScanDependencies/Inputs/Swift/SubEWrapper.swiftinterface

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

test/ScanDependencies/module_deps.swift

@@ -52,14 +52,14 @@ import SubE
 // CHECK-NEXT: "swift": "SwiftOnoneSupport"
 // CHECK-NEXT: }
 // CHECK-NEXT: {
+// CHECK-NEXT:   "swift": "_cross_import_E"
+// CHECK-NEXT: }
+// CHECK-NEXT: {
 // CHECK-NEXT: "swift": "F"
 // CHECK-NEXT: }
 // CHECK-NEXT: {
 // CHECK-NEXT: "swift": "A"
 // CHECK-NEXT: }
-// CHECK-NEXT: {
-// CHECK-NEXT:   "swift": "_cross_import_E"
-// CHECK-NEXT: }
 // CHECK-NEXT: ],
 
 // CHECK:      "extraPcmArgs": [

test/ScanDependencies/module_deps_cross_import_overlay.swift

@@ -0,0 +1,33 @@
+// RUN: %empty-directory(%t)
+// RUN: mkdir -p %t/clang-module-cache
+// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -emit-dependencies -emit-dependencies-path %t/deps.d -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4 -disable-implicit-swift-modules -Xcc -Xclang -Xcc -fno-implicit-modules
+
+// Check the contents of the JSON output
+// RUN: %FileCheck %s < %t/deps.json
+
+// REQUIRES: executable_test
+// REQUIRES: objc_interop
+
+import EWrapper
+import SubEWrapper
+
+// CHECK:  "directDependencies": [
+// CHECK-NEXT: {
+// CHECK-NEXT:   "swift": "EWrapper"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "swift": "SubEWrapper"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "swift": "Swift"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "swift": "SwiftOnoneSupport"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "swift": "_cross_import_E"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT:   "swift": "F"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],