Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 2, 2024
6 changes: 5 additions & 1 deletion include/swift-c/DependencyScan/DependencyScan.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/SearchPathOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
14 changes: 13 additions & 1 deletion include/swift/Frontend/ModuleInterfaceLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
};
Expand Down Expand Up @@ -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") {
Expand All @@ -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;
Expand Down Expand Up @@ -423,6 +434,7 @@ class ExplicitModuleMapParser {
clangModulePath,
isFramework,
isSystem,
isBridgingHeaderDependency,
clangModuleCacheKey);
clangModuleMap.try_emplace(moduleName, std::move(entry));
}
Expand Down
24 changes: 16 additions & 8 deletions lib/DependencyScan/ScanDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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");
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions lib/Frontend/Frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions lib/Frontend/ModuleInterfaceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 8 additions & 2 deletions lib/FrontendTool/FrontendTool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(
Expand Down
115 changes: 115 additions & 0 deletions test/ScanDependencies/bridging_header_dep_module_map.swift
Original file line number Diff line number Diff line change
@@ -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
51 changes: 51 additions & 0 deletions test/ScanDependencies/bridging_header_modulemap_only.swift
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions test/ScanDependencies/explicit-swift-dependencies.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Loading