[5.10 🍒][Dependency Scanning] Attempt to lookup optional transitive dependencies of binary module dependencies #68498

include/swift/AST/ModuleDependencies.h

@@ -116,8 +116,10 @@ class ModuleDependencyInfoStorageBase {
 
   ModuleDependencyInfoStorageBase(ModuleDependencyKind dependencyKind,
                                   const std::vector<std::string> &moduleImports,
+                                  const std::vector<std::string> &optionalModuleImports,
                                   StringRef moduleCacheKey = "")
       : dependencyKind(dependencyKind), moduleImports(moduleImports),
+        optionalModuleImports(optionalModuleImports),
         moduleCacheKey(moduleCacheKey.str()), resolved(false), finalized(false)  {}
 
   virtual ModuleDependencyInfoStorageBase *clone() const = 0;
@@ -296,11 +298,12 @@ class SwiftBinaryModuleDependencyStorage : public ModuleDependencyInfoStorageBas
                                      const std::string &moduleDocPath,
                                      const std::string &sourceInfoPath,
                                      const std::vector<std::string> &moduleImports,
+                                     const std::vector<std::string> &optionalModuleImports,
                                      const std::vector<std::string> &headerImports,
                                      const bool isFramework,
                                      const std::string &moduleCacheKey)
       : ModuleDependencyInfoStorageBase(ModuleDependencyKind::SwiftBinary,
-                                        moduleImports, moduleCacheKey),
+                                        moduleImports, optionalModuleImports, moduleCacheKey),
         compiledModulePath(compiledModulePath), moduleDocPath(moduleDocPath),
         sourceInfoPath(sourceInfoPath), preCompiledBridgingHeaderPaths(headerImports),
         isFramework(isFramework) {}
@@ -471,12 +474,14 @@ class ModuleDependencyInfo {
       const std::string &moduleDocPath,
       const std::string &sourceInfoPath,
       const std::vector<std::string> &moduleImports,
+      const std::vector<std::string> &optionalModuleImports,
       const std::vector<std::string> &headerImports,
       bool isFramework, const std::string &moduleCacheKey) {
     return ModuleDependencyInfo(
         std::make_unique<SwiftBinaryModuleDependencyStorage>(
           compiledModulePath, moduleDocPath, sourceInfoPath,
-          moduleImports, headerImports, isFramework, moduleCacheKey));
+          moduleImports, optionalModuleImports,
+          headerImports, isFramework, moduleCacheKey));
   }
 
   /// Describe the main Swift module.

include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

@@ -39,7 +39,7 @@ using llvm::BCVBR;
 
 /// Every .moddepcache file begins with these 4 bytes, for easy identification.
 const unsigned char MODULE_DEPENDENCY_CACHE_FORMAT_SIGNATURE[] = {'I', 'M', 'D','C'};
-const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MAJOR = 4;
+const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MAJOR = 5; // optionalModuleImports
 /// Increment this on every change.
 const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MINOR = 1;
 
@@ -124,6 +124,7 @@ using ModuleInfoLayout =
                    IdentifierIDField,            // moduleName
                    ContextHashIDField,           // contextHash
                    ImportArrayIDField,           // moduleImports
+                   ImportArrayIDField,           // optionalModuleImports
                    DependencyIDArrayIDField      // resolvedDirectModuleDependencies
                    >;
 

lib/DependencyScan/ModuleDependencyCacheSerialization.cpp

@@ -155,6 +155,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
   std::string currentModuleName;
   unsigned currentContextHashID;
   llvm::Optional<std::vector<std::string>> currentModuleImports;
+  llvm::Optional<std::vector<std::string>> currentOptionalModuleImports;
   llvm::Optional<std::vector<ModuleDependencyID>> currentModuleDependencyIDs;
 
   auto getContextHash = [&]() {
@@ -212,19 +213,24 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
     case MODULE_NODE: {
       hasCurrentModule = true;
       unsigned moduleNameID, contextHashID,
-               moduleImportsArrayID, moduleDependencyIDArrayID;
+               moduleImportsArrayID, optionalModuleImportsArrayID,
+               moduleDependencyIDArrayID;
       ModuleInfoLayout::readRecord(Scratch, moduleNameID, contextHashID,
                                    moduleImportsArrayID,
+                                   optionalModuleImportsArrayID,
                                    moduleDependencyIDArrayID);
       auto moduleName = getIdentifier(moduleNameID);
       if (!moduleName)
         llvm::report_fatal_error("Bad module name");
       currentModuleName = *moduleName;
       currentContextHashID = contextHashID;
       currentModuleImports = getStringArray(moduleImportsArrayID);
+      currentOptionalModuleImports = getStringArray(optionalModuleImportsArrayID);
       currentModuleDependencyIDs = getModuleDependencyIDArray(moduleDependencyIDArrayID);
       if (!currentModuleImports)
         llvm::report_fatal_error("Bad direct dependencies: no imports");
+      if (!currentOptionalModuleImports)
+        llvm::report_fatal_error("Bad direct dependencies: no optional imports");
       if (!currentModuleDependencyIDs)
         llvm::report_fatal_error("Bad direct dependencies: no qualified dependencies");
       break;
@@ -296,6 +302,9 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
       // Add imports of this module
       for (const auto &moduleName : *currentModuleImports)
         moduleDep.addModuleImport(moduleName);
+      // Add optional imports of this module
+      for (const auto &moduleName : *currentOptionalModuleImports)
+        moduleDep.addOptionalModuleImport(moduleName);
 
       // Add qualified dependencies of this module
       moduleDep.resolveDirectDependencies(*currentModuleDependencyIDs);
@@ -404,6 +413,9 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
       // Add dependencies of this module
       for (const auto &moduleName : *currentModuleImports)
         moduleDep.addModuleImport(moduleName);
+      // Add optional imports of this module
+      for (const auto &moduleName : *currentOptionalModuleImports)
+        moduleDep.addOptionalModuleImport(moduleName);
 
       // Add bridging header file path
       if (bridgingHeaderFileID != 0) {
@@ -488,8 +500,8 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
       // Form the dependencies storage object
       auto moduleDep = ModuleDependencyInfo::forSwiftBinaryModule(
            *compiledModulePath, *moduleDocPath, *moduleSourceInfoPath,
-           *currentModuleImports, *headerImports, isFramework,
-           *moduleCacheKey);
+           *currentModuleImports, *currentOptionalModuleImports,
+           *headerImports, isFramework, *moduleCacheKey);
 
       cache.recordDependency(currentModuleName, std::move(moduleDep),
                              getContextHash());
@@ -523,6 +535,9 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
       // Add dependencies of this module
       for (const auto &moduleName : *currentModuleImports)
         moduleDep.addModuleImport(moduleName);
+      // Add optional imports of this module
+      for (const auto &moduleName : *currentOptionalModuleImports)
+        moduleDep.addOptionalModuleImport(moduleName);
 
       cache.recordDependency(currentModuleName, std::move(moduleDep),
                              getContextHash());
@@ -581,6 +596,9 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
       // Add dependencies of this module
       for (const auto &moduleName : *currentModuleImports)
         moduleDep.addModuleImport(moduleName);
+      // Add optional imports of this module
+      for (const auto &moduleName : *currentOptionalModuleImports)
+        moduleDep.addOptionalModuleImport(moduleName);
 
       cache.recordDependency(currentModuleName, std::move(moduleDep),
                              getContextHash());
@@ -707,6 +725,7 @@ bool swift::dependencies::module_dependency_cache_serialization::
 enum ModuleIdentifierArrayKind : uint8_t {
   Empty = 0,
   DependencyImports,
+  OptionalDependencyImports,
   DependencyHeaders,
   QualifiedModuleDependencyIDs,
   CompiledModuleCandidates,
@@ -904,6 +923,7 @@ void ModuleDependenciesCacheSerializer::writeModuleInfo(
       Out, ScratchRecord, AbbrCodes[ModuleInfoLayout::Code],
       getIdentifier(moduleID.first), contextHashStrID,
       getArrayID(moduleID, ModuleIdentifierArrayKind::DependencyImports),
+      getArrayID(moduleID, ModuleIdentifierArrayKind::OptionalDependencyImports),
       getArrayID(moduleID, ModuleIdentifierArrayKind::QualifiedModuleDependencyIDs));
 
   switch (dependencyInfo.getKind()) {
@@ -1112,6 +1132,8 @@ void ModuleDependenciesCacheSerializer::collectStringsAndArrays(
       // Add the module's dependencies
       addStringArray(moduleID, ModuleIdentifierArrayKind::DependencyImports,
                      dependencyInfo->getModuleImports());
+      addStringArray(moduleID, ModuleIdentifierArrayKind::OptionalDependencyImports,
+                     dependencyInfo->getOptionalModuleImports());
       addDependencyIDArray(
           moduleID, ModuleIdentifierArrayKind::QualifiedModuleDependencyIDs,
           dependencyInfo->getDirectModuleDependencies());

lib/Serialization/SerializedModuleLoader.cpp

@@ -457,6 +457,15 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework) {
   if (!binaryModuleImports)
     return binaryModuleImports.getError();
 
+  // Lookup optional imports of this module also
+  auto binaryModuleOptionalImports = getImportsOfModule(
+      modulePath, ModuleLoadingBehavior::Optional, isFramework,
+      isRequiredOSSAModules(), Ctx.LangOpts.SDKName, Ctx.LangOpts.PackageName,
+      Ctx.SourceMgr.getFileSystem().get(),
+      Ctx.SearchPathOpts.DeserializedPathRecoverer);
+  if (!binaryModuleOptionalImports)
+    return binaryModuleImports.getError();
+
   auto importedModuleSet = binaryModuleImports.get().moduleImports;
   std::vector<std::string> importedModuleNames;
   importedModuleNames.reserve(importedModuleSet.size());
@@ -475,11 +484,17 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework) {
                      return N.str();
                   });
 
+  auto &importedOptionalModuleSet = binaryModuleOptionalImports.get().moduleImports;
+  std::vector<std::string> importedOptionalModuleNames;
+  for (const auto optionalImportedModule : importedOptionalModuleSet.keys())
+    if (!importedModuleSet.contains(optionalImportedModule))
+      importedOptionalModuleNames.push_back(optionalImportedModule.str());
+
   // Map the set of dependencies over to the "module dependencies".
   auto dependencies = ModuleDependencyInfo::forSwiftBinaryModule(
        modulePath.str(), moduleDocPath, sourceInfoPath,
-       importedModuleNames, importedHeaders, isFramework,
-       /*module-cache-key*/ "");
+       importedModuleNames, importedOptionalModuleNames,
+       importedHeaders, isFramework, /*module-cache-key*/ "");
 
   return std::move(dependencies);
 }

test/ScanDependencies/optional_deps_of_binary_testable_imports.swift

@@ -0,0 +1,21 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/clang-module-cache)
+// RUN: %empty-directory(%t/Foo.swiftmodule)
+// RUN: echo "@_implementationOnly import A; public func foo() {}" > %t/Foo.swift
+// REQUIRES: executable_test
+// REQUIRES: objc_interop
+
+@testable import Foo
+
+// Step 1: build a binary swift module for `Foo`, make it testable
+// RUN: %target-swift-frontend -emit-module %t/Foo.swift -emit-module-path %t/Foo.swiftmodule/%target-swiftmodule-name -module-name Foo -I %S/Inputs/CHeaders -I %S/Inputs/Swift -enable-testing
+
+// Step 2: scan dependencies
+// RUN: %target-swift-frontend -scan-dependencies %s -o %t/deps.json -I %t -sdk %t -prebuilt-module-cache-path %t/clang-module-cache -I %S/Inputs/CHeaders -I %S/Inputs/Swift
+// RUN: %validate-json %t/deps.json | %FileCheck %s
+
+// The dependency of `Foo` on `A` will not be visible if the scanner simply scans the textual interface
+// of `Foo`. So we verify that for a `@testable` import, the scanner also opens up the adjacent binary module and
+// attemtps to resolve optional dependencies contained within.
+//
+// CHECK: "swift": "A"

test/ScanDependencies/optional_deps_of_testable_imports.swift

@@ -10,7 +10,7 @@
 // Step 1: build swift interface and swift module side by side, make them testable
 // RUN: %target-swift-frontend -emit-module %t/Foo.swift -emit-module-path %t/Foo.swiftmodule/%target-swiftmodule-name -module-name Foo -emit-module-interface-path %t/Foo.swiftmodule/%target-swiftinterface-name -I %S/Inputs/CHeaders -I %S/Inputs/Swift -enable-testing
 
-// Step 3: scan dependencies
+// Step 2: scan dependencies
 // RUN: %target-swift-frontend -scan-dependencies %s -o %t/deps.json -I %t -sdk %t -prebuilt-module-cache-path %t/clang-module-cache -I %S/Inputs/CHeaders -I %S/Inputs/Swift
 // RUN: %validate-json %t/deps.json | %FileCheck %s
 

test/ScanDependencies/required_deps_of_testable_imports.swift

@@ -10,7 +10,7 @@
 // Step 1: build swift interface and swift module side by side, make them testable
 // RUN: %target-swift-frontend -emit-module %t/Foo.swift -emit-module-path %t/Foo.swiftmodule/%target-swiftmodule-name -module-name Foo -emit-module-interface-path %t/Foo.swiftmodule/%target-swiftinterface-name -I %S/Inputs/CHeaders -I %S/Inputs/Swift -enable-testing -enable-experimental-feature AccessLevelOnImport
 
-// Step 3: scan dependencies
+// Step 2: scan dependencies
 // RUN: %target-swift-frontend -scan-dependencies %s -o %t/deps.json -I %t -sdk %t -prebuilt-module-cache-path %t/clang-module-cache -I %S/Inputs/CHeaders -I %S/Inputs/Swift
 // RUN: %validate-json %t/deps.json | %FileCheck %s