Merge pull request #72668 from artemcm/DepScanBinaryModuleHeaderDepModuleDeps

artemcm · web-flow · commit d9c00d8dd66f · 2024-03-29T09:02:10.000-07:00
[Explicit Module Builds] Only specify '-fmodule-map-file' for bridging header Clang module dependencies
diff --git a/include/swift-c/DependencyScan/DependencyScan.h b/include/swift-c/DependencyScan/DependencyScan.h
@@ -25,7 +25,7 @@
 /// SWIFTSCAN_VERSION_MINOR should increase when there are API additions.
 /// SWIFTSCAN_VERSION_MAJOR is intended for "major" source/ABI breaking changes.
 #define SWIFTSCAN_VERSION_MAJOR 0
-#define SWIFTSCAN_VERSION_MINOR 7
+#define SWIFTSCAN_VERSION_MINOR 8
 
 SWIFTSCAN_BEGIN_DECLS
 
@@ -206,6 +206,10 @@ SWIFTSCAN_PUBLIC swiftscan_string_ref_t
 swiftscan_swift_binary_detail_get_header_dependency(
     swiftscan_module_details_t details);
 
+SWIFTSCAN_PUBLIC swiftscan_string_set_t *
+swiftscan_swift_binary_detail_get_header_dependency_module_dependencies(
+    swiftscan_module_details_t details);
+
 SWIFTSCAN_PUBLIC bool
 swiftscan_swift_binary_detail_get_is_framework(
     swiftscan_module_details_t details);
diff --git a/include/swift/AST/SearchPathOptions.h b/include/swift/AST/SearchPathOptions.h
@@ -476,7 +476,7 @@ class SearchPathOptions {
   std::vector<std::string> CandidateCompiledModules;
 
   /// A map of explicit Swift module information.
-  std::string ExplicitSwiftModuleMap;
+  std::string ExplicitSwiftModuleMapPath;
 
   /// Module inputs specified with -swift-module-input,
   /// <ModuleName, Path to .swiftmodule file>
diff --git a/include/swift/Frontend/ModuleInterfaceLoader.h b/include/swift/Frontend/ModuleInterfaceLoader.h
@@ -269,9 +269,11 @@ struct ExplicitClangModuleInputInfo {
   ExplicitClangModuleInputInfo(
       std::string moduleMapPath, std::string modulePath,
       bool isFramework = false, bool isSystem = false,
+      bool isBridgingHeaderDependency = true,
       std::optional<std::string> moduleCacheKey = std::nullopt)
       : moduleMapPath(moduleMapPath), modulePath(modulePath),
         isFramework(isFramework), isSystem(isSystem),
+        isBridgingHeaderDependency(isBridgingHeaderDependency),
         moduleCacheKey(moduleCacheKey) {}
   // Path of the Clang module map file.
   std::string moduleMapPath;
@@ -281,6 +283,8 @@ struct ExplicitClangModuleInputInfo {
   bool isFramework = false;
   // A flag that indicates whether this module is a system module
   bool isSystem = false;
+  // A flag that indicates whether this is a module dependency of a textual header input
+  bool isBridgingHeaderDependency = true;
   // The cache key for clang module.
   std::optional<std::string> moduleCacheKey;
 };
@@ -367,7 +371,12 @@ class ExplicitModuleMapParser {
         swiftModuleSourceInfoPath, swiftModuleCacheKey, clangModuleCacheKey;
     std::optional<std::vector<std::string>> headerDependencyPaths;
     std::string clangModuleMapPath = "", clangModulePath = "";
-    bool isFramework = false, isSystem = false;
+    bool isFramework = false, isSystem = false,
+         // The default value is 'true' in case the build system does not yet
+         // support emitting this field, in which case we must be conservative and
+         // ensure all dependencies get '-fmodule-map-file', instead of strictly
+         // module dependencies of textual header inputs.
+         isBridgingHeaderDependency = true;
     for (auto &entry : *mapNode) {
       auto key = getScalaNodeText(entry.getKey());
       if (key == "prebuiltHeaderDependencyPaths") {
@@ -394,6 +403,8 @@ class ExplicitModuleMapParser {
           swiftModuleCacheKey = val.str();
         } else if (key == "clangModuleCacheKey") {
           clangModuleCacheKey = val.str();
+        } else if (key == "isBridgingHeaderDependency") {
+          isBridgingHeaderDependency = parseBoolValue(val);
         } else {
           // Being forgiving for future fields.
           continue;
@@ -423,6 +434,7 @@ class ExplicitModuleMapParser {
                                          clangModulePath,
                                          isFramework,
                                          isSystem,
+                                         isBridgingHeaderDependency,
                                          clangModuleCacheKey);
       clangModuleMap.try_emplace(moduleName, std::move(entry));
     }
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -1982,7 +1982,7 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts,
       Args.hasArg(OPT_disable_modules_validate_system_headers);
 
   if (const Arg *A = Args.getLastArg(OPT_explicit_swift_module_map))
-    Opts.ExplicitSwiftModuleMap = A->getValue();
+    Opts.ExplicitSwiftModuleMapPath = A->getValue();
   for (auto A : Args.getAllArgValues(options::OPT_swift_module_file)) {
     if (validateSwiftModuleFileArgumentAndAdd(A, Diags,
                                               Opts.ExplicitSwiftModuleInputs))
diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp
@@ -745,18 +745,18 @@ bool CompilerInstance::setUpModuleLoaders() {
   bool ExplicitModuleBuild =
       Invocation.getFrontendOptions().DisableImplicitModules;
   if (ExplicitModuleBuild ||
-      !Invocation.getSearchPathOptions().ExplicitSwiftModuleMap.empty() ||
+      !Invocation.getSearchPathOptions().ExplicitSwiftModuleMapPath.empty() ||
       !Invocation.getSearchPathOptions().ExplicitSwiftModuleInputs.empty()) {
     if (Invocation.getCASOptions().EnableCaching)
       ESML = ExplicitCASModuleLoader::create(
           *Context, getObjectStore(), getActionCache(), getDependencyTracker(),
-          MLM, Invocation.getSearchPathOptions().ExplicitSwiftModuleMap,
+          MLM, Invocation.getSearchPathOptions().ExplicitSwiftModuleMapPath,
           Invocation.getSearchPathOptions().ExplicitSwiftModuleInputs,
           IgnoreSourceInfoFile);
     else
       ESML = ExplicitSwiftModuleLoader::create(
           *Context, getDependencyTracker(), MLM,
-          Invocation.getSearchPathOptions().ExplicitSwiftModuleMap,
+          Invocation.getSearchPathOptions().ExplicitSwiftModuleMapPath,
           Invocation.getSearchPathOptions().ExplicitSwiftModuleInputs,
           IgnoreSourceInfoFile);
   }
diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp
@@ -1847,8 +1847,8 @@ InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl(
   }
 
   // Pass down -explicit-swift-module-map-file
-  StringRef explicitSwiftModuleMap = searchPathOpts.ExplicitSwiftModuleMap;
-  genericSubInvocation.getSearchPathOptions().ExplicitSwiftModuleMap =
+  StringRef explicitSwiftModuleMap = searchPathOpts.ExplicitSwiftModuleMapPath;
+  genericSubInvocation.getSearchPathOptions().ExplicitSwiftModuleMapPath =
     explicitSwiftModuleMap.str();
 
   // Pass down VFSOverlay flags (do not need to inherit the options because
@@ -2198,6 +2198,7 @@ struct ExplicitSwiftModuleLoader::Implementation {
     for (auto &entry : ExplicitClangModuleMap) {
       const auto &moduleMapPath = entry.getValue().moduleMapPath;
       if (!moduleMapPath.empty() &&
+          entry.getValue().isBridgingHeaderDependency &&
           moduleMapsSeen.find(moduleMapPath) == moduleMapsSeen.end()) {
         moduleMapsSeen.insert(moduleMapPath);
         extraClangArgs.push_back(
diff --git a/test/ScanDependencies/bridging_header_modulemap_only.swift b/test/ScanDependencies/bridging_header_modulemap_only.swift
@@ -0,0 +1,51 @@
+// REQUIRES: objc_interop
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/TestInputs)
+// RUN: split-file %s %t
+
+// - Fixup the input module file map
+// RUN: sed -e "s|INPUTSDIR|%/t/TestInputs|g" %t/map.json.template > %t/map.json.template1
+// RUN: sed -e "s|STDLIBMOD|%/stdlib_module|g" %t/map.json.template1 > %t/map.json.template2
+// RUN: sed -e "s|ONONEMOD|%/ononesupport_module|g" %t/map.json.template2 > %t/map.json.template3
+// RUN: sed -e "s|CHEADERSDIR|%/S/Inputs/CHeaders|g" %t/map.json.template3 > %t/map.json.template4
+// RUN: sed -e "s|SWIFTLIBDIR|%swift-lib-dir|g" %t/map.json.template4 > %t/map.json
+
+// - Pre-compile explicit module dependency inputs
+// RUN: %target-swift-emit-pcm -module-name A -o %t/TestInputs/A.pcm %S/Inputs/CHeaders/module.modulemap
+// RUN: %target-swift-emit-pcm -module-name SwiftShims %swift-lib-dir/swift/shims/module.modulemap -o %t/TestInputs/SwiftShims.pcm
+
+// RUN: %target-swift-frontend -c -disable-implicit-swift-modules -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -explicit-swift-module-map-file %t/map.json -primary-file %t/bridging_header_modulemap_only.swift -o %t/bridging_header_modulemap_only.o -dump-clang-diagnostics 2>&1 | %FileCheck %s --check-prefix=CHECK-CLANG-COMMAND
+
+//--- map.json.template
+[
+  {
+      "moduleName": "Swift",
+      "modulePath": "STDLIBMOD",
+      "isFramework": false
+  },
+  {
+      "moduleName": "SwiftOnoneSupport",
+      "modulePath": "ONONEMOD",
+      "isFramework": false
+  },
+  {
+      "moduleName": "SwiftShims",
+      "isFramework": false,
+      "isBridgingHeaderDependency": false,
+      "clangModuleMapPath": "SWIFTLIBDIR/swift/shims/module.modulemap",
+      "clangModulePath": "INPUTSDIR/SwiftShims.pcm"
+  },
+  {
+      "moduleName": "A",
+      "isFramework": false,
+      "isBridgingHeaderDependency": true,      
+      "clangModulePath": "INPUTSDIR/A.pcm",
+      "clangModuleMapPath": "CHEADERSDIR/module.modulemap"
+  }
+]
+
+//--- bridging_header_modulemap_only.swift
+import A
+
+// CHECK-CLANG-COMMAND: -fmodule-map-file={{.*}}{{/|\\}}CHeaders{{/|\\}}module.modulemap
+// CHECK-CLANG-COMMAND-NOT: -fmodule-map-file={{.*}}{{/|\\}}swift{{/|\\}}shims{{/|\\}}module.modulemap
diff --git a/tools/libSwiftScan/libSwiftScan.cpp b/tools/libSwiftScan/libSwiftScan.cpp
@@ -365,6 +365,12 @@ swiftscan_swift_binary_detail_get_header_dependency(
   return details->swift_binary_details.header_dependency;
 }
 
+swiftscan_string_set_t *
+swiftscan_swift_binary_detail_get_header_dependency_module_dependencies(
+    swiftscan_module_details_t details) {
+  return details->swift_binary_details.header_dependencies_module_dependnecies;
+}
+
 bool swiftscan_swift_binary_detail_get_is_framework(
     swiftscan_module_details_t details) {
   return details->swift_binary_details.is_framework;
diff --git a/tools/libSwiftScan/libSwiftScan.exports b/tools/libSwiftScan/libSwiftScan.exports
@@ -25,6 +25,7 @@ swiftscan_swift_binary_detail_get_module_doc_path
 swiftscan_swift_binary_detail_get_module_source_info_path
 swiftscan_swift_binary_detail_get_swift_overlay_dependencies
 swiftscan_swift_binary_detail_get_header_dependency
+swiftscan_swift_binary_detail_get_header_dependency_module_dependencies
 swiftscan_swift_binary_detail_get_is_framework
 swiftscan_swift_binary_detail_get_module_cache_key
 swiftscan_swift_placeholder_detail_get_compiled_module_path
diff --git a/tools/swift-ide-test/swift-ide-test.cpp b/tools/swift-ide-test/swift-ide-test.cpp
@@ -4520,7 +4520,7 @@ int main(int argc, char *argv[]) {
     InitInvok.getLangOptions().addCustomConditionalCompilationFlag(ConfigName);
 
   if (!options::ExplicitSwiftModuleMap.empty()) {
-    InitInvok.getSearchPathOptions().ExplicitSwiftModuleMap =
+    InitInvok.getSearchPathOptions().ExplicitSwiftModuleMapPath =
       options::ExplicitSwiftModuleMap;
     InitInvok.getFrontendOptions().DisableImplicitModules = true;
   }

Original file line number	Diff line number	Diff line change
`@@ -4520,7 +4520,7 @@ int main(int argc, char *argv[]) {`
`4520`	`4520`	`InitInvok.getLangOptions().addCustomConditionalCompilationFlag(ConfigName);`
`4521`	`4521`
`4522`	`4522`	`if (!options::ExplicitSwiftModuleMap.empty()) {`
`4523`		`- InitInvok.getSearchPathOptions().ExplicitSwiftModuleMap =`
	`4523`	`+ InitInvok.getSearchPathOptions().ExplicitSwiftModuleMapPath =`
`4524`	`4524`	`options::ExplicitSwiftModuleMap;`
`4525`	`4525`	`InitInvok.getFrontendOptions().DisableImplicitModules = true;`
`4526`	`4526`	`}`