Skip to content

Commit 15dda6c

Browse files
artemcmartemcm
authored
Merge pull request #74945 from artemcm/FixCrossImportOnScannerValidatedBinaryModules
[Dependency Scanning] Resolve cross-import overlays relative to defining interface for prebuilt binary Swift dependencies
2 parents 66a5329 + 2464f87 commit 15dda6c

File tree

9 files changed

+134
-27
lines changed

9 files changed

+134
-27
lines changed

‎include/swift/AST/ModuleDependencies.h‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -397,12 +397,14 @@ class SwiftBinaryModuleDependencyStorage
397397
ArrayRef<ScannerImportStatementInfo> moduleImports,
398398
ArrayRef<ScannerImportStatementInfo> optionalModuleImports,
399399
ArrayRef<LinkLibrary> linkLibraries, StringRef headerImport,
400-
bool isFramework, bool isStatic, StringRef moduleCacheKey)
400+
StringRef definingModuleInterface, bool isFramework, bool isStatic,
401+
StringRef moduleCacheKey)
401402
: ModuleDependencyInfoStorageBase(ModuleDependencyKind::SwiftBinary,
402403
moduleImports, optionalModuleImports,
403404
linkLibraries, moduleCacheKey),
404405
compiledModulePath(compiledModulePath), moduleDocPath(moduleDocPath),
405406
sourceInfoPath(sourceInfoPath), headerImport(headerImport),
407+
definingModuleInterfacePath(definingModuleInterface),
406408
isFramework(isFramework), isStatic(isStatic) {}
407409

408410
ModuleDependencyInfoStorageBase *clone() const override {
@@ -421,6 +423,10 @@ class SwiftBinaryModuleDependencyStorage
421423
/// The path of the .h dependency of this module.
422424
const std::string headerImport;
423425

426+
/// The path of the defining .swiftinterface that this
427+
/// binary .swiftmodule was built from, if one exists.
428+
const std::string definingModuleInterfacePath;
429+
424430
/// Source files on which the header inputs depend.
425431
std::vector<std::string> headerSourceFiles;
426432

@@ -433,6 +439,15 @@ class SwiftBinaryModuleDependencyStorage
433439
/// A flag that indicates this dependency is associated with a static archive
434440
const bool isStatic;
435441

442+
/// Return the path to the defining .swiftinterface of this module
443+
/// of one was determined. Otherwise, return the .swiftmodule path
444+
/// itself.
445+
std::string getDefiningModulePath() const {
446+
if (definingModuleInterfacePath.empty())
447+
return compiledModulePath;
448+
return definingModuleInterfacePath;
449+
}
450+
436451
static bool classof(const ModuleDependencyInfoStorageBase *base) {
437452
return base->dependencyKind == ModuleDependencyKind::SwiftBinary;
438453
}
@@ -588,12 +603,13 @@ class ModuleDependencyInfo {
588603
ArrayRef<ScannerImportStatementInfo> moduleImports,
589604
ArrayRef<ScannerImportStatementInfo> optionalModuleImports,
590605
ArrayRef<LinkLibrary> linkLibraries, StringRef headerImport,
591-
bool isFramework, bool isStatic, StringRef moduleCacheKey) {
606+
StringRef definingModuleInterface, bool isFramework,
607+
bool isStatic, StringRef moduleCacheKey) {
592608
return ModuleDependencyInfo(
593609
std::make_unique<SwiftBinaryModuleDependencyStorage>(
594610
compiledModulePath, moduleDocPath, sourceInfoPath, moduleImports,
595-
optionalModuleImports, linkLibraries, headerImport, isFramework,
596-
isStatic, moduleCacheKey));
611+
optionalModuleImports, linkLibraries, headerImport,
612+
definingModuleInterface,isFramework, isStatic, moduleCacheKey));
597613
}
598614

599615
/// Describe the main Swift module.

‎lib/AST/ModuleLoader.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ ModuleDependencyInfo::collectCrossImportOverlayNames(
223223
}
224224
case swift::ModuleDependencyKind::SwiftBinary: {
225225
auto *swiftBinaryDep = getAsSwiftBinaryModule();
226-
modulePath = swiftBinaryDep->compiledModulePath;
226+
modulePath = swiftBinaryDep->getDefiningModulePath();
227227
assert(modulePath.has_value());
228228
StringRef parentDir = llvm::sys::path::parent_path(*modulePath);
229229
if (llvm::sys::path::extension(parentDir) == ".swiftmodule") {

‎lib/DependencyScan/ModuleDependencyCacheSerialization.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,12 +514,12 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
514514
if (!headerImport)
515515
llvm::report_fatal_error("Bad binary direct dependencies: no header import");
516516

517-
// TODO: LinkLibraries
517+
// TODO: LinkLibraries, DefiningModulePath
518518
// Form the dependencies storage object
519519
auto moduleDep = ModuleDependencyInfo::forSwiftBinaryModule(
520520
*compiledModulePath, *moduleDocPath, *moduleSourceInfoPath,
521521
currentModuleImports, currentOptionalModuleImports, {},
522-
*headerImport, isFramework, isStatic, *moduleCacheKey);
522+
*headerImport, "", isFramework, isStatic, *moduleCacheKey);
523523

524524
auto headerModuleDependencies = getStringArray(headerModuleDependenciesArrayID);
525525
if (!headerModuleDependencies)

‎lib/Serialization/ModuleFile.cpp‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,6 @@ static bool isTargetTooNew(const llvm::Triple &moduleTarget,
9292
return ctxTarget.isOSVersionLT(moduleTarget);
9393
}
9494

95-
std::string ModuleFile::resolveModuleDefiningFilename(const ASTContext &ctx) {
96-
if (!Core->ModuleInterfacePath.empty()) {
97-
std::string interfacePath = Core->ModuleInterfacePath.str();
98-
if (llvm::sys::path::is_relative(interfacePath)) {
99-
SmallString<128> absoluteInterfacePath(ctx.SearchPathOpts.getSDKPath());
100-
llvm::sys::path::append(absoluteInterfacePath, interfacePath);
101-
return absoluteInterfacePath.str().str();
102-
} else
103-
return interfacePath;
104-
} else
105-
return getModuleLoadedFilename().str();
106-
}
107-
10895
namespace swift {
10996
namespace serialization {
11097
bool areCompatible(const llvm::Triple &moduleTarget,
@@ -272,7 +259,8 @@ Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
272259

273260
ASTContext &ctx = getContext();
274261
// Resolve potentially-SDK-relative module-defining .swiftinterface path
275-
ResolvedModuleDefiningFilename = resolveModuleDefiningFilename(ctx);
262+
ResolvedModuleDefiningFilename =
263+
Core->resolveModuleDefiningFilePath(ctx.SearchPathOpts.getSDKPath());
276264

277265
llvm::Triple moduleTarget(llvm::Triple::normalize(Core->TargetTriple));
278266
if (!areCompatible(moduleTarget, ctx.LangOpts.Target)) {

‎lib/Serialization/ModuleFile.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,6 @@ class ModuleFile
285285
ArrayRef<ProtocolConformanceID>
286286
claimLazyConformanceLoaderToken(uint64_t token);
287287

288-
/// If the module-defining `.swiftinterface` file is an SDK-relative path,
289-
/// resolve it to be absolute to the context's SDK.
290-
std::string resolveModuleDefiningFilename(const ASTContext &ctx);
291-
292288
/// Represents an identifier that may or may not have been deserialized yet.
293289
///
294290
/// If \c Ident is empty, the identifier has not been loaded yet.

‎lib/Serialization/ModuleFileSharedCore.cpp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1785,6 +1785,19 @@ bool ModuleFileSharedCore::hasSourceInfo() const {
17851785
return !!DeclUSRsTable;
17861786
}
17871787

1788+
std::string ModuleFileSharedCore::resolveModuleDefiningFilePath(const StringRef SDKPath) const {
1789+
if (!ModuleInterfacePath.empty()) {
1790+
std::string interfacePath = ModuleInterfacePath.str();
1791+
if (llvm::sys::path::is_relative(interfacePath)) {
1792+
SmallString<128> absoluteInterfacePath(SDKPath);
1793+
llvm::sys::path::append(absoluteInterfacePath, interfacePath);
1794+
return absoluteInterfacePath.str().str();
1795+
} else
1796+
return interfacePath;
1797+
} else
1798+
return ModuleInputBuffer->getBufferIdentifier().str();
1799+
}
1800+
17881801
ModuleLoadingBehavior
17891802
ModuleFileSharedCore::getTransitiveLoadingBehavior(
17901803
const Dependency &dependency,

‎lib/Serialization/ModuleFileSharedCore.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,10 @@ class ModuleFileSharedCore {
623623
return Bits.IsStaticLibrary;
624624
}
625625

626+
/// If the module-defining `.swiftinterface` file is an SDK-relative path,
627+
/// resolve it to be absolute to the specified SDK.
628+
std::string resolveModuleDefiningFilePath(const StringRef SDKPath) const;
629+
626630
/// Returns \c true if this module file contains a section with incremental
627631
/// information.
628632
bool hasIncrementalInfo() const { return HasIncrementalInfo; }

‎lib/Serialization/SerializedModuleLoader.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,11 +511,16 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework,
511511
LibraryKind::Framework));
512512
}
513513

514+
// Attempt to resolve the module's defining .swiftinterface path
515+
std::string definingModulePath =
516+
loadedModuleFile->resolveModuleDefiningFilePath(Ctx.SearchPathOpts.getSDKPath());
517+
514518
// Map the set of dependencies over to the "module dependencies".
515519
auto dependencies = ModuleDependencyInfo::forSwiftBinaryModule(
516520
modulePath.str(), moduleDocPath, sourceInfoPath, moduleImports,
517-
optionalModuleImports, linkLibraries, importedHeader, isFramework,
518-
loadedModuleFile->isStaticLibrary(), /*module-cache-key*/ "");
521+
optionalModuleImports, linkLibraries, importedHeader,
522+
definingModulePath, isFramework, loadedModuleFile->isStaticLibrary(),
523+
/*module-cache-key*/ "");
519524

520525
return std::move(dependencies);
521526
}

‎test/ScanDependencies/module_deps_cross_import_of_binary_module.swift‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// REQUIRES: objc_interop
2+
// RUN: %empty-directory(%t)
3+
// RUN: %empty-directory(%t/module-cache)
4+
// RUN: %empty-directory(%t/inputs)
5+
// RUN: %empty-directory(%t/binaryInputs)
6+
// RUN: %empty-directory(%t/separateModules)
7+
// RUN: %empty-directory(%t/inputs/Foo.swiftcrossimport)
8+
// RUN: split-file %s %t
9+
10+
// - Fixup the input module file map
11+
// RUN: sed -e "s|INPUTSDIR|%/t/inputs|g" %t/map.json.template > %t/map.json.template1
12+
// RUN: sed -e "s|STDLIBMOD|%/stdlib_module|g" %t/map.json.template1 > %t/map.json.template2
13+
// RUN: sed -e "s|ONONEMOD|%/ononesupport_module|g" %t/map.json.template2 > %t/map.json.template3
14+
// RUN: sed -e "s|SWIFTLIBDIR|%swift-lib-dir|g" %t/map.json.template3 > %t/map.json
15+
16+
// - Pre-compile explicit module dependency inputs
17+
// RUN: %target-swift-emit-pcm -module-name SwiftShims %swift-lib-dir/swift/shims/module.modulemap -o %t/inputs/SwiftShims.pcm
18+
19+
// - Pre-compile the Foo module into a separately-stored binary module
20+
// RUN: %target-swift-frontend -compile-module-from-interface %t/separateModules/Foo.swiftinterface -o %t/binaryInputs/Foo.swiftmodule -module-name Foo -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -disable-implicit-swift-modules -Xcc -fno-implicit-modules -Xcc -fno-implicit-module-maps -explicit-swift-module-map-file %t/map.json
21+
22+
// - Run a dependency scan on test.swift which will pick-up the ready-made binary dependency on Foo.swiftmodule
23+
// and use the binary module's serialized originating defining .swiftinterface path to be able to
24+
// discover the cross-import overlay _Foo_Bar.
25+
// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/test.swift -o %t/deps.json -I %t/inputs -I %t/binaryInputs -module-name test -enable-cross-import-overlays -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import
26+
27+
// CHECK: "mainModuleName": "test"
28+
// CHECK: "swift": "test"
29+
// CHECK-NEXT: },
30+
// CHECK-NEXT: {
31+
// CHECK-NEXT: "modulePath": "test.swiftmodule"
32+
// CHECK-NEXT: "sourceFiles": [
33+
// CHECK-NEXT: "{{.*}}{{/|\\}}module_deps_cross_import_of_binary_module.swift.tmp{{/|\\}}test.swift"
34+
// CHECK-NEXT: ]
35+
// CHECK-NEXT: "directDependencies": [
36+
// CHECK-DAG: "swiftPrebuiltExternal": "Swift"
37+
// CHECK-DAG: "swiftPrebuiltExternal": "SwiftOnoneSupport"
38+
// CHECK-DAG: "swiftPrebuiltExternal": "Foo"
39+
// CHECK-DAG: "swift": "Bar"
40+
// CHECK-DAG: "swift": "_Foo_Bar"
41+
42+
//--- map.json.template
43+
[
44+
{
45+
"moduleName": "Swift",
46+
"modulePath": "STDLIBMOD",
47+
"isFramework": false
48+
},
49+
{
50+
"moduleName": "SwiftOnoneSupport",
51+
"modulePath": "ONONEMOD",
52+
"isFramework": false
53+
},
54+
{
55+
"moduleName": "SwiftShims",
56+
"isFramework": false,
57+
"clangModuleMapPath": "SWIFTLIBDIR/swift/shims/module.modulemap",
58+
"clangModulePath": "INPUTSDIR/SwiftShims.pcm"
59+
}]
60+
61+
//--- test.swift
62+
import Foo
63+
import Bar
64+
65+
//--- separateModules/Foo.swiftinterface
66+
// swift-interface-format-version: 1.0
67+
// swift-module-flags: -module-name Foo
68+
public func foo() {}
69+
70+
//--- inputs/Bar.swiftinterface
71+
// swift-interface-format-version: 1.0
72+
// swift-module-flags: -module-name Bar
73+
public func bar() {}
74+
75+
//--- inputs/_Foo_Bar.swiftinterface
76+
// swift-interface-format-version: 1.0
77+
// swift-module-flags: -module-name _Foo_Bar
78+
public func foobar() {}
79+
80+
//--- separateModules/Foo.swiftcrossimport/Bar.swiftoverlay
81+
%YAML 1.2
82+
---
83+
version: 1
84+
modules:
85+
- name: _Foo_Bar

0 commit comments

Comments
 (0)