Merge pull request #73087 from cachemeifyoucan/eng/PR-testable-import-deps-optional

[ScanDependencies] Do not count optional dependencies when not needed

Author: cachemeifyoucan

include/swift/Serialization/SerializedModuleLoader.h

@@ -163,10 +163,8 @@ class SerializedModuleLoaderBase : public ModuleLoader {
   }
 
   /// Scan the given serialized module file to determine dependencies.
-  llvm::ErrorOr<ModuleDependencyInfo> scanModuleFile(Twine modulePath,
-                                                     bool isFramework,
-                                                     bool isTestableImport,
-                                                     bool hasInterface);
+  llvm::ErrorOr<ModuleDependencyInfo>
+  scanModuleFile(Twine modulePath, bool isFramework, bool isTestableImport);
 
   struct BinaryModuleImports {
     llvm::StringSet<> moduleImports;
@@ -176,7 +174,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
   static BinaryModuleImports
   getImportsOfModule(const ModuleFileSharedCore &loadedModule,
                      ModuleLoadingBehavior transitiveBehavior,
-                     StringRef packageName);
+                     StringRef packageName, bool isTestableImport);
 
   /// Load the module file into a buffer and also collect its module name.
   static std::unique_ptr<llvm::MemoryBuffer>

lib/Serialization/ScanningLoaders.cpp

@@ -57,8 +57,7 @@ std::error_code SwiftModuleScanner::findModuleFilesInDirectory(
     if (fs.exists(ModPath)) {
       // The module file will be loaded directly.
       auto dependencies =
-          scanModuleFile(ModPath, IsFramework, isTestableDependencyLookup,
-                         /*hasInterface=*/false);
+          scanModuleFile(ModPath, IsFramework, isTestableDependencyLookup);
       if (dependencies) {
         this->dependencies = std::move(dependencies.get());
         return std::error_code();
@@ -160,9 +159,8 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath,
             !Ctx.SearchPathOpts.NoScannerModuleValidation) {
           assert(compiledCandidates.size() == 1 &&
                  "Should only have 1 candidate module");
-          auto BinaryDep =
-              scanModuleFile(compiledCandidates[0], isFramework,
-                             isTestableImport, /*hasInterface=*/true);
+          auto BinaryDep = scanModuleFile(compiledCandidates[0], isFramework,
+                                          isTestableImport);
           if (BinaryDep) {
             Result = *BinaryDep;
             return std::error_code();

lib/Serialization/SerializedModuleLoader.cpp

@@ -395,7 +395,8 @@ std::error_code SerializedModuleLoaderBase::openModuleFile(
 SerializedModuleLoaderBase::BinaryModuleImports
 SerializedModuleLoaderBase::getImportsOfModule(
     const ModuleFileSharedCore &loadedModuleFile,
-    ModuleLoadingBehavior transitiveBehavior, StringRef packageName) {
+    ModuleLoadingBehavior transitiveBehavior, StringRef packageName,
+    bool isTestableImport) {
   llvm::StringSet<> importedModuleNames;
   std::string importedHeader = "";
   for (const auto &dependency : loadedModuleFile.getDependencies()) {
@@ -410,8 +411,7 @@ SerializedModuleLoaderBase::getImportsOfModule(
         loadedModuleFile.getTransitiveLoadingBehavior(
             dependency,
             /*debuggerMode*/ false,
-            /*isPartialModule*/ false, packageName,
-            loadedModuleFile.isTestable());
+            /*isPartialModule*/ false, packageName, isTestableImport);
     if (dependencyTransitiveBehavior > transitiveBehavior)
       continue;
 
@@ -431,8 +431,7 @@ SerializedModuleLoaderBase::getImportsOfModule(
 
 llvm::ErrorOr<ModuleDependencyInfo>
 SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework,
-                                           bool isTestableImport,
-                                           bool hasInterface) {
+                                           bool isTestableImport) {
   const std::string moduleDocPath;
   const std::string sourceInfoPath;
 
@@ -455,16 +454,6 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework,
                            modulePath.str());
       return std::make_error_code(std::errc::no_such_file_or_directory);
     }
-
-    // If the module file has interface file and not testable imported, don't
-    // import the testable module because it contains more interfaces than
-    // needed and can pull in more dependencies.
-    if (loadedModuleFile->isTestable() && !isTestableImport && hasInterface) {
-      if (Ctx.LangOpts.EnableModuleLoadingRemarks)
-        Ctx.Diags.diagnose(SourceLoc(), diag::skip_module_testable,
-                           modulePath.str());
-      return std::make_error_code(std::errc::no_such_file_or_directory);
-    }
   }
 
   // Some transitive dependencies of binary modules are not required to be
@@ -475,12 +464,12 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework,
   //       optional.
   auto binaryModuleImports =
       getImportsOfModule(*loadedModuleFile, ModuleLoadingBehavior::Required,
-                         Ctx.LangOpts.PackageName);
+                         Ctx.LangOpts.PackageName, isTestableImport);
 
   // Lookup optional imports of this module also
   auto binaryModuleOptionalImports =
       getImportsOfModule(*loadedModuleFile, ModuleLoadingBehavior::Optional,
-                         Ctx.LangOpts.PackageName);
+                         Ctx.LangOpts.PackageName, isTestableImport);
 
   auto importedModuleSet = binaryModuleImports.moduleImports;
   std::vector<std::string> importedModuleNames;

test/ScanDependencies/testable-dependencies.swift

@@ -16,13 +16,12 @@
 // RUN:   -emit-module-interface-path %t/regular/A.swiftinterface -enable-library-evolution -I %t/internal \
 // RUN:   %t/A.swift
 
-/// Import testable build, should use interface.
+/// Import testable build, should use binary but no extra dependencies.
 // RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-serialized -module-name Test %t/main.swift \
 // RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \
-// RUN:   -o %t/deps1.json -I %t/testable -swift-version 5 -Rmodule-loading 2>&1 | %FileCheck %s --check-prefix DIAG
-// DIAG: remark: skip swiftmodule built with '-enable-testing'
+// RUN:   -o %t/deps1.json -I %t/testable -swift-version 5 -Rmodule-loading
 // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps1.json Test directDependencies | %FileCheck %s --check-prefix TEST1
-// TEST1: "swift": "A"
+// TEST1:  "swiftPrebuiltExternal": "A"
 // RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps1.json A directDependencies | %FileCheck %s --check-prefix EMPTY --allow-empty
 // EMPTY-NOT: B
 
@@ -56,12 +55,15 @@
 // RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib -enable-testing \
 // RUN:   -o %t/deps5.json -I %t/regular -swift-version 5 -Rmodule-loading
 
-/// Regular import a testable module with no interface, will try to import binary module but fail to look up the dependency.
+/// Regular import a testable module with no interface, don't load optional dependencies.
 // RUN: rm %t/testable/A.swiftinterface
 // RUN: %target-swift-frontend -scan-dependencies -module-load-mode prefer-interface -module-name Test %t/main.swift \
 // RUN:   -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib -enable-testing \
-// RUN:   -o %t/deps6.json -I %t/testable -swift-version 5 -Rmodule-loading 2>&1 | %FileCheck %s --check-prefix ERROR
-// ERROR: error: Unable to find module dependency: 'B'
+// RUN:   -o %t/deps6.json -I %t/testable -swift-version 5 -Rmodule-loading
+// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps6.json Test directDependencies | %FileCheck %s --check-prefix TEST6
+// TEST6:  "swiftPrebuiltExternal": "A"
+// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps6.json swiftPrebuiltExternal:A directDependencies | %FileCheck %s --check-prefix EMPTY --allow-empty
+
 
 //--- main.swift
 import A