Merge pull request #72743 from artemcm/60EBMChanges

[6.0🍒][Explicit Module Builds] Cherry-pick Explicit Module Loading fixes and improvements

Author: artemcm

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);

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>

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));
     }

lib/DependencyScan/ScanDependencies.cpp

@@ -264,6 +264,20 @@ static llvm::Error resolveExplicitModuleInputs(
                        : binaryDepDetails->moduleCacheKey;
       commandLine.push_back("-swift-module-file=" + depModuleID.ModuleName + "=" +
                             path);
+      // If this binary module was built with a header, the header's module
+      // dependencies must also specify a .modulemap to the compilation, in
+      // order to resolve the header's own header include directives.
+      for (const auto &bridgingHeaderDepName :
+           binaryDepDetails->headerModuleDependencies) {
+        auto optionalBridgingHeaderDepModuleInfo = cache.findDependency(
+            {bridgingHeaderDepName, ModuleDependencyKind::Clang});
+        assert(optionalDepInfo.has_value());
+        const auto bridgingHeaderDepModuleDetails =
+            optionalBridgingHeaderDepModuleInfo.value()->getAsClangModule();
+        commandLine.push_back(
+            "-fmodule-map-file=" +
+            remapPath(bridgingHeaderDepModuleDetails->moduleMapFile));
+      }
     } break;
     case swift::ModuleDependencyKind::SwiftPlaceholder: {
       auto placeholderDetails = depInfo->getAsPlaceholderDependencyModule();
@@ -278,13 +292,6 @@ static llvm::Error resolveExplicitModuleInputs(
         commandLine.push_back("-Xcc");
         commandLine.push_back("-fmodule-file=" + depModuleID.ModuleName + "=" +
                               clangDepDetails->mappedPCMPath);
-        if (!instance.getInvocation()
-                 .getClangImporterOptions()
-                 .UseClangIncludeTree) {
-          commandLine.push_back("-Xcc");
-          commandLine.push_back("-fmodule-map-file=" +
-                                remapPath(clangDepDetails->moduleMapFile));
-        }
       }
       if (!clangDepDetails->moduleCacheKey.empty()) {
         commandLine.push_back("-Xcc");
@@ -1471,7 +1478,8 @@ static bool diagnoseCycle(const CompilerInstance &instance,
 
   auto kindIsSwiftDependency = [&](const ModuleDependencyID &ID) {
     return ID.Kind == swift::ModuleDependencyKind::SwiftInterface ||
-           ID.Kind == swift::ModuleDependencyKind::SwiftBinary;
+           ID.Kind == swift::ModuleDependencyKind::SwiftBinary ||
+           ID.Kind == swift::ModuleDependencyKind::SwiftSource;
   };
 
   auto emitModulePath = [&](const std::vector<ModuleDependencyID> path,

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))

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);
   }

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
@@ -2194,6 +2194,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(

lib/FrontendTool/FrontendTool.cpp

@@ -410,6 +410,13 @@ static bool buildModuleFromInterface(CompilerInstance &Instance) {
   bool IgnoreAdjacentModules = Instance.hasASTContext() &&
                                Instance.getASTContext().IgnoreAdjacentModules;
 
+  // When building explicit module dependencies, they are
+  // discovered by dependency scanner and the swiftmodule is already rebuilt
+  // ignoring candidate module. There is no need to serialized dependencies for
+  // validation purpose because the build system (swift-driver) is then
+  // responsible for checking whether inputs are up-to-date.
+  bool ShouldSerializeDeps = !FEOpts.ExplicitInterfaceBuild;
+
   // If an explicit interface build was requested, bypass the creation of a new
   // sub-instance from the interface which will build it in a separate thread,
   // and isntead directly use the current \c Instance for compilation.
@@ -422,8 +429,7 @@ static bool buildModuleFromInterface(CompilerInstance &Instance) {
     return ModuleInterfaceLoader::buildExplicitSwiftModuleFromSwiftInterface(
         Instance, Invocation.getClangModuleCachePath(),
         FEOpts.BackupModuleInterfaceDir, PrebuiltCachePath, ABIPath, InputPath,
-        Invocation.getOutputFilename(),
-        /* shouldSerializeDeps */ true,
+        Invocation.getOutputFilename(), ShouldSerializeDeps,
         Invocation.getSearchPathOptions().CandidateCompiledModules);
 
   return ModuleInterfaceLoader::buildSwiftModuleFromSwiftInterface(

test/ScanDependencies/bridging_header_dep_module_map.swift

@@ -0,0 +1,115 @@
+// REQUIRES: objc_interop
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/FooInputs)
+// RUN: %empty-directory(%t/BridgingHeaderDir)
+// RUN: %empty-directory(%t/TestCHeaders)
+// RUN: %empty-directory(%t/TestSwiftInterfaces)
+// RUN: %empty-directory(%t/FooModuleDir)
+// RUN: split-file %s %t
+
+// - Fixup the input module file map
+// RUN: sed -e "s|INPUTSDIR|%/t/FooInputs|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|SWIFTLIBDIR|%swift-lib-dir|g" %t/map.json.template3 > %t/map.json
+
+// - Set up explicit dependencies for Foo
+// RUN: %target-swift-emit-pcm -module-name SwiftShims %swift-lib-dir/swift/shims/module.modulemap -o %t/FooInputs/SwiftShims.pcm
+// - Build Foo module dependency, explicitly, non-resiliently
+// RUN: %target-swift-frontend -emit-module -emit-module-path %t/FooModuleDir/Foo.swiftmodule %t/foo.swift -module-name Foo -import-objc-header %t/BridgingHeaderDir/foo.h -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -disable-implicit-swift-modules -explicit-swift-module-map-file %t/map.json -I %S/Inputs/CHeaders
+
+// - Scan main module and ensure that the "FooClient" recipe includes the modulemap for Foo's briding header's module dependencies
+// but not other dependencies
+// RUN: %target-swift-frontend -scan-dependencies %t/bridging_header_dep_module_map.swift -I %t/FooModuleDir -I %t/TestSwiftInterfaces -I %t/TestCHeaders -I %S/Inputs/CHeaders -o %t/deps.json
+// RUN: %validate-json %t/deps.json | %FileCheck %s
+
+// Given the following dependency graph:
+//
+//           main
+//            |
+//          FooClient (.swiftinterface)
+//            |                 \
+//          Foo(.swiftmodule)   Dart (.pcm)
+//
+// Given that 'Foo.swiftmodule' is built with a bridging header which imports 'X.h' ('X' clang module)
+// We expect that 'Foo' will have a dependency on module 'X', and the scanner will ensure that 'FooClient' is built
+// with the modulemap file for 'X' as an explicit input. 'Dart' Clang module however, must not result in an
+// explicitly-specified modulemap file because no headers of this module will be ingested into the Swift
+// compiler.
+
+// Dependency of the main module
+// CHECK: "swift": "FooClient"
+
+// Definition of 'FooClient' in the dependency graph
+// CHECK: "swift": "FooClient"
+// CHECK: "modulePath": "{{.*}}FooClient-{{.*}}.swiftmodule",
+// CHECK: "directDependencies": [
+// CHECK-DAG:    "swiftPrebuiltExternal": "Foo"
+// CHECK-DAG:    "swift": "SwiftOnoneSupport"
+// CHECK-DAG:    "clang": "Dart"
+// CHECK: ],
+// CHECK: "commandLine": [
+// CHECK: "-fmodule-map-file={{.*}}{{/|\\}}CHeaders{{/|\\}}module.modulemap"
+// CHECK-NOT: "-fmodule-map-file={{.*}}{{/|\\}}TestCHeaders{{/|\\}}module.modulemap"
+// CHECK: ]
+
+// Definition of 'Foo' in the dependency graph
+// CHECK: "swiftPrebuiltExternal": "Foo"
+// CHECK: "modulePath": "{{.*}}Foo.swiftmodule",
+// CHECK-NEXT: "directDependencies": [
+// CHECK-DAG:    "swift": "Swift"
+// CHECK-DAG:    "swift": "SwiftOnoneSupport"
+// CHECK-DAG:    "clang": "X"
+// CHECK: ],
+// CHECK:      "headerDependency": "{{.*}}{{/|\\}}BridgingHeaderDir{{/|\\}}foo.h"
+// CHECK:      "headerModuleDependencies": [
+// CHECK-NEXT:   "X"
+// CHECK-NEXT: ],
+// CHECK:      "headerDependenciesSourceFiles": [
+// CHECK-NEXT:   "{{.*}}{{/|\\}}BridgingHeaderDir{{/|\\}}foo.h"
+// CHECK-NEXT: ],
+
+//--- foo.swift
+extension Profiler {
+    public static let count: Int = 42
+}
+
+//--- BridgingHeaderDir/foo.h
+#include "X.h"
+struct Profiler { void* ptr; };
+
+//--- TestCHeaders/Dart.h
+struct Dart { void* ptr; };
+//--- TestCHeaders/module.modulemap
+module Dart {
+  header "Dart.h"
+  export *
+}
+
+//--- TestSwiftInterfaces/FooClient.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -module-name FooClient
+import Foo
+import Dart
+
+//--- map.json.template
+[
+  {
+      "moduleName": "Swift",
+      "modulePath": "STDLIBMOD",
+      "isFramework": false
+  },
+  {
+      "moduleName": "SwiftOnoneSupport",
+      "modulePath": "ONONEMOD",
+      "isFramework": false
+  },
+  {
+      "moduleName": "SwiftShims",
+      "isFramework": false,
+      "clangModuleMapPath": "SWIFTLIBDIR/swift/shims/module.modulemap",
+      "clangModulePath": "INPUTSDIR/SwiftShims.pcm"
+}]
+
+//--- bridging_header_dep_module_map.swift
+import FooClient

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

test/ScanDependencies/explicit-swift-dependencies.swift

@@ -51,5 +51,5 @@ import F
 // CHECK-NEXT:            "{{.*}}{{/|\\}}F-{{.*}}.swiftmodule"
 // CHECK-DAG:             "-swift-module-file=Swift={{.*}}{{/|\\}}Swift-{{.*}}.swiftmodule"
 // CHECK-DAG:             "-swift-module-file=SwiftOnoneSupport={{.*}}{{/|\\}}SwiftOnoneSupport-{{.*}}.swiftmodule"
-// CHECK-DAG:             "-fmodule-file=F={{.*}}{{/|\\}}F-{{.*}}.pcm",
-// CHECK-DAG:             "-fmodule-file=SwiftShims={{.*}}{{/|\\}}SwiftShims-{{.*}}.pcm",
+// CHECK-DAG:             "-fmodule-file=F={{.*}}{{/|\\}}F-{{.*}}.pcm"
+// CHECK-DAG:             "-fmodule-file=SwiftShims={{.*}}{{/|\\}}SwiftShims-{{.*}}.pcm"