Skip to content

Commit 112d963

Browse files
artemcmartemcm
authored
Merge pull request #74949 from artemcm/60FixCrossImportOnScannerValidatedBinaryModules
[6.0 🍒][Dependency Scanning] Resolve cross-import overlays relative to defining interface for prebuilt binary Swift dependencies
2 parents c8aee3c + aef88ea commit 112d963

File tree

9 files changed

+132
-25
lines changed

9 files changed

+132
-25
lines changed

‎include/swift/AST/ModuleDependencies.h‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -367,13 +367,15 @@ class SwiftBinaryModuleDependencyStorage
367367
const std::string &sourceInfoPath,
368368
const std::vector<ScannerImportStatementInfo> &moduleImports,
369369
const std::vector<ScannerImportStatementInfo> &optionalModuleImports,
370-
const std::string &headerImport, const bool isFramework,
370+
const std::string &headerImport,
371+
const std::string &definingModuleInterface, const bool isFramework,
371372
const std::string &moduleCacheKey)
372373
: ModuleDependencyInfoStorageBase(ModuleDependencyKind::SwiftBinary,
373374
moduleImports, optionalModuleImports,
374375
moduleCacheKey),
375376
compiledModulePath(compiledModulePath), moduleDocPath(moduleDocPath),
376377
sourceInfoPath(sourceInfoPath), headerImport(headerImport),
378+
definingModuleInterfacePath(definingModuleInterface),
377379
isFramework(isFramework) {}
378380

379381
ModuleDependencyInfoStorageBase *clone() const override {
@@ -392,6 +394,10 @@ class SwiftBinaryModuleDependencyStorage
392394
/// The path of the .h dependency of this module.
393395
const std::string headerImport;
394396

397+
/// The path of the defining .swiftinterface that this
398+
/// binary .swiftmodule was built from, if one exists.
399+
const std::string definingModuleInterfacePath;
400+
395401
/// Source files on which the header inputs depend.
396402
std::vector<std::string> headerSourceFiles;
397403

@@ -401,6 +407,15 @@ class SwiftBinaryModuleDependencyStorage
401407
/// A flag that indicates this dependency is a framework
402408
const bool isFramework;
403409

410+
/// Return the path to the defining .swiftinterface of this module
411+
/// of one was determined. Otherwise, return the .swiftmodule path
412+
/// itself.
413+
std::string getDefiningModulePath() const {
414+
if (definingModuleInterfacePath.empty())
415+
return compiledModulePath;
416+
return definingModuleInterfacePath;
417+
}
418+
404419
static bool classof(const ModuleDependencyInfoStorageBase *base) {
405420
return base->dependencyKind == ModuleDependencyKind::SwiftBinary;
406421
}
@@ -560,12 +575,13 @@ class ModuleDependencyInfo {
560575
const std::vector<ScannerImportStatementInfo> &moduleImports,
561576
const std::vector<ScannerImportStatementInfo> &optionalModuleImports,
562577
const std::string &headerImport,
578+
const std::string &definingModuleInterface,
563579
bool isFramework, const std::string &moduleCacheKey) {
564580
return ModuleDependencyInfo(
565581
std::make_unique<SwiftBinaryModuleDependencyStorage>(
566582
compiledModulePath, moduleDocPath, sourceInfoPath,
567-
moduleImports, optionalModuleImports,
568-
headerImport, isFramework, moduleCacheKey));
583+
moduleImports, optionalModuleImports, headerImport,
584+
definingModuleInterface, isFramework, moduleCacheKey));
569585
}
570586

571587
/// Describe the main Swift module.

‎lib/AST/ModuleLoader.cpp‎

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

‎lib/DependencyScan/ModuleDependencyCacheSerialization.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,11 +508,12 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
508508
if (!headerImport)
509509
llvm::report_fatal_error("Bad binary direct dependencies: no header import");
510510

511+
// TODO: DefiningModulePath
511512
// Form the dependencies storage object
512513
auto moduleDep = ModuleDependencyInfo::forSwiftBinaryModule(
513514
*compiledModulePath, *moduleDocPath, *moduleSourceInfoPath,
514515
currentModuleImports, currentOptionalModuleImports,
515-
*headerImport, isFramework, *moduleCacheKey);
516+
*headerImport, "", isFramework, *moduleCacheKey);
516517

517518
auto headerModuleDependencies = getStringArray(headerModuleDependenciesArrayID);
518519
if (!headerModuleDependencies)

‎lib/Serialization/ModuleFile.cpp‎

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

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

272259
ASTContext &ctx = getContext();
273260
// Resolve potentially-SDK-relative module-defining .swiftinterface path
274-
ResolvedModuleDefiningFilename = resolveModuleDefiningFilename(ctx);
261+
ResolvedModuleDefiningFilename =
262+
Core->resolveModuleDefiningFilePath(ctx.SearchPathOpts.getSDKPath());
275263

276264
llvm::Triple moduleTarget(llvm::Triple::normalize(Core->TargetTriple));
277265
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
@@ -1773,6 +1773,19 @@ bool ModuleFileSharedCore::hasSourceInfo() const {
17731773
return !!DeclUSRsTable;
17741774
}
17751775

1776+
std::string ModuleFileSharedCore::resolveModuleDefiningFilePath(const StringRef SDKPath) const {
1777+
if (!ModuleInterfacePath.empty()) {
1778+
std::string interfacePath = ModuleInterfacePath.str();
1779+
if (llvm::sys::path::is_relative(interfacePath)) {
1780+
SmallString<128> absoluteInterfacePath(SDKPath);
1781+
llvm::sys::path::append(absoluteInterfacePath, interfacePath);
1782+
return absoluteInterfacePath.str().str();
1783+
} else
1784+
return interfacePath;
1785+
} else
1786+
return ModuleInputBuffer->getBufferIdentifier().str();
1787+
}
1788+
17761789
ModuleLoadingBehavior
17771790
ModuleFileSharedCore::getTransitiveLoadingBehavior(
17781791
const Dependency &dependency,

‎lib/Serialization/ModuleFileSharedCore.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,10 @@ class ModuleFileSharedCore {
605605
return Dependencies;
606606
}
607607

608+
/// If the module-defining `.swiftinterface` file is an SDK-relative path,
609+
/// resolve it to be absolute to the specified SDK.
610+
std::string resolveModuleDefiningFilePath(const StringRef SDKPath) const;
611+
608612
/// Returns \c true if this module file contains a section with incremental
609613
/// information.
610614
bool hasIncrementalInfo() const { return HasIncrementalInfo; }

‎lib/Serialization/SerializedModuleLoader.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,11 +494,15 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework,
494494
optionalModuleImports.push_back(
495495
ScannerImportStatementInfo(optionalImportedModule.str()));
496496

497+
// Attempt to resolve the module's defining .swiftinterface path
498+
std::string definingModulePath =
499+
loadedModuleFile->resolveModuleDefiningFilePath(Ctx.SearchPathOpts.getSDKPath());
500+
497501
// Map the set of dependencies over to the "module dependencies".
498502
auto dependencies = ModuleDependencyInfo::forSwiftBinaryModule(
499503
modulePath.str(), moduleDocPath, sourceInfoPath, moduleImports,
500-
optionalModuleImports, importedHeader, isFramework,
501-
/*module-cache-key*/ "");
504+
optionalModuleImports, importedHeader, definingModulePath,
505+
isFramework, /*module-cache-key*/ "");
502506

503507
return std::move(dependencies);
504508
}

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