Skip to content

Commit f7bc374

Browse files
authored
Merge pull request #83532 from artemcm/AdjustDarwinCxxInteropCycleHack
[Dependency Scanning][C++ Interop] Avoid `CxxStdlib` overlay lookup for binary Swift dependencies which were not built with C++ interop
2 parents 1828f5b + 77a61a2 commit f7bc374

File tree

7 files changed

+87
-36
lines changed

7 files changed

+87
-36
lines changed

include/swift/AST/ModuleDependencies.h

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -456,8 +456,8 @@ class SwiftBinaryModuleDependencyStorage
456456
ArrayRef<LinkLibrary> linkLibraries,
457457
ArrayRef<serialization::SearchPath> serializedSearchPaths,
458458
StringRef headerImport, StringRef definingModuleInterface,
459-
bool isFramework, bool isStatic, StringRef moduleCacheKey,
460-
StringRef userModuleVersion)
459+
bool isFramework, bool isStatic, bool isBuiltWithCxxInterop,
460+
StringRef moduleCacheKey, StringRef userModuleVersion)
461461
: ModuleDependencyInfoStorageBase(ModuleDependencyKind::SwiftBinary,
462462
moduleImports, optionalModuleImports,
463463
linkLibraries, moduleCacheKey),
@@ -466,6 +466,7 @@ class SwiftBinaryModuleDependencyStorage
466466
definingModuleInterfacePath(definingModuleInterface),
467467
serializedSearchPaths(serializedSearchPaths),
468468
isFramework(isFramework), isStatic(isStatic),
469+
isBuiltWithCxxInterop(isBuiltWithCxxInterop),
469470
userModuleVersion(userModuleVersion) {}
470471

471472
ModuleDependencyInfoStorageBase *clone() const override {
@@ -503,6 +504,10 @@ class SwiftBinaryModuleDependencyStorage
503504
/// A flag that indicates this dependency is associated with a static archive
504505
const bool isStatic;
505506

507+
/// A flag that indicates this dependency module was built
508+
/// with C++ interop enabled
509+
const bool isBuiltWithCxxInterop;
510+
506511
/// The user module version of this binary module.
507512
const std::string userModuleVersion;
508513

@@ -636,14 +641,14 @@ class ModuleDependencyInfo {
636641
ArrayRef<LinkLibrary> linkLibraries,
637642
ArrayRef<serialization::SearchPath> serializedSearchPaths,
638643
StringRef headerImport, StringRef definingModuleInterface,
639-
bool isFramework, bool isStatic, StringRef moduleCacheKey,
640-
StringRef userModuleVer) {
644+
bool isFramework, bool isStatic, bool isBuiltWithCxxInterop,
645+
StringRef moduleCacheKey, StringRef userModuleVer) {
641646
return ModuleDependencyInfo(
642647
std::make_unique<SwiftBinaryModuleDependencyStorage>(
643648
compiledModulePath, moduleDocPath, sourceInfoPath, moduleImports,
644649
optionalModuleImports, linkLibraries, serializedSearchPaths,
645650
headerImport, definingModuleInterface,isFramework, isStatic,
646-
moduleCacheKey, userModuleVer));
651+
isBuiltWithCxxInterop, moduleCacheKey, userModuleVer));
647652
}
648653

649654
/// Describe the main Swift module.

include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ using IsFrameworkField = BCFixed<1>;
5656
using IsSystemField = BCFixed<1>;
5757
/// A bit that indicates whether or not a module is that of a static archive
5858
using IsStaticField = BCFixed<1>;
59+
/// A bit that indicates whether or not a module is built with C++ interop
60+
using IsBuiltWithCxxInteropField = BCFixed<1>;
5961
/// A bit that indicates whether or not a link library is a force-load one
6062
using IsForceLoadField = BCFixed<1>;
6163
/// A bit that indicates whether or not an import statement is optional
@@ -267,6 +269,7 @@ using SwiftBinaryModuleDetailsLayout =
267269
SearchPathArrayIDField, // serializedSearchPaths
268270
IsFrameworkField, // isFramework
269271
IsStaticField, // isStatic
272+
IsBuiltWithCxxInteropField, // IsBuiltWithCxxInterop
270273
IdentifierIDField, // moduleCacheKey
271274
IdentifierIDField // UserModuleVersion
272275
>;

lib/DependencyScan/ModuleDependencyCacheSerialization.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -679,13 +679,14 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
679679
unsigned compiledModulePathID, moduleDocPathID, moduleSourceInfoPathID,
680680
headerImportID, definingInterfacePathID, searchPathArrayID,
681681
headerModuleDependenciesArrayID, headerImportsSourceFilesArrayID,
682-
isFramework, isStatic, moduleCacheKeyID, userModuleVersionID;
682+
isFramework, isStatic, isBuiltWithCxxInterop, moduleCacheKeyID,
683+
userModuleVersionID;
683684
SwiftBinaryModuleDetailsLayout::readRecord(
684685
Scratch, compiledModulePathID, moduleDocPathID,
685686
moduleSourceInfoPathID, headerImportID, definingInterfacePathID,
686687
headerModuleDependenciesArrayID, headerImportsSourceFilesArrayID,
687-
searchPathArrayID, isFramework, isStatic, moduleCacheKeyID,
688-
userModuleVersionID);
688+
searchPathArrayID, isFramework, isStatic, isBuiltWithCxxInterop,
689+
moduleCacheKeyID, userModuleVersionID);
689690

690691
auto compiledModulePath = getIdentifier(compiledModulePathID);
691692
if (!compiledModulePath)
@@ -720,7 +721,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(
720721
*compiledModulePath, *moduleDocPath, *moduleSourceInfoPath,
721722
importStatements, optionalImportStatements, linkLibraries,
722723
*searchPaths, *headerImport, *definingInterfacePath, isFramework,
723-
isStatic, *moduleCacheKey, *userModuleVersion);
724+
isStatic, isBuiltWithCxxInterop, *moduleCacheKey, *userModuleVersion);
724725

725726
addCommonDependencyInfo(moduleDep);
726727
addSwiftCommonDependencyInfo(moduleDep);
@@ -1629,6 +1630,7 @@ void ModuleDependenciesCacheSerializer::writeModuleInfo(
16291630
ModuleIdentifierArrayKind::HeaderInputDependencySourceFiles),
16301631
getSearchPathArrayID(moduleID),
16311632
swiftBinDeps->isFramework, swiftBinDeps->isStatic,
1633+
swiftBinDeps->isBuiltWithCxxInterop,
16321634
getIdentifier(swiftBinDeps->moduleCacheKey),
16331635
getIdentifier(swiftBinDeps->userModuleVersion));
16341636
break;

lib/DependencyScan/ModuleDependencyScanner.cpp

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,23 +1553,34 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependenciesForModule(
15531553
recordResult(clangDep);
15541554

15551555
// C++ Interop requires additional handling
1556-
bool lookupCxxStdLibOverlay = ScanCompilerInvocation.getLangOptions().EnableCXXInterop;
1557-
if (lookupCxxStdLibOverlay && moduleID.Kind == ModuleDependencyKind::SwiftInterface) {
1556+
bool lookupCxxStdLibOverlay =
1557+
ScanCompilerInvocation.getLangOptions().EnableCXXInterop;
1558+
if (lookupCxxStdLibOverlay &&
1559+
moduleID.Kind == ModuleDependencyKind::SwiftInterface) {
15581560
const auto &moduleInfo = cache.findKnownDependency(moduleID);
15591561
const auto commandLine = moduleInfo.getCommandline();
15601562
// If the textual interface was built without C++ interop, do not query
15611563
// the C++ Standard Library Swift overlay for its compilation.
1562-
//
1563-
// FIXME: We always declare the 'Darwin' module as formally having been built
1564-
// without C++Interop, for compatibility with prior versions. Once we are certain
1565-
// that we are only building against modules built with support of
1566-
// '-formal-cxx-interoperability-mode', this hard-coded check should be removed.
1567-
if (moduleID.ModuleName == "Darwin" ||
1568-
llvm::find(commandLine, "-formal-cxx-interoperability-mode=off") !=
1569-
commandLine.end())
1564+
if (llvm::find(commandLine, "-formal-cxx-interoperability-mode=off") !=
1565+
commandLine.end())
1566+
lookupCxxStdLibOverlay = false;
1567+
} else if (lookupCxxStdLibOverlay &&
1568+
moduleID.Kind == ModuleDependencyKind::SwiftBinary) {
1569+
const auto &moduleDetails =
1570+
cache.findKnownDependency(moduleID).getAsSwiftBinaryModule();
1571+
// If the binary module was built without C++ interop, do not query
1572+
// the C++ Standard Library Swift overlay.
1573+
if (!moduleDetails->isBuiltWithCxxInterop)
15701574
lookupCxxStdLibOverlay = false;
15711575
}
15721576

1577+
// FIXME: We always declare the 'Darwin' module as formally having been built
1578+
// without C++Interop, for compatibility with prior versions. Once we are certain
1579+
// that we are only building against modules built with support of
1580+
// '-formal-cxx-interoperability-mode', this hard-coded check should be removed.
1581+
if (lookupCxxStdLibOverlay && moduleID.ModuleName == "Darwin")
1582+
lookupCxxStdLibOverlay = false;
1583+
15731584
if (lookupCxxStdLibOverlay) {
15741585
for (const auto &clangDepNameEntry : allClangDependencies) {
15751586
auto clangDepName = clangDepNameEntry;

lib/Serialization/ModuleFileSharedCore.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,11 @@ class ModuleFileSharedCore {
653653
return Bits.IsStaticLibrary;
654654
}
655655

656+
/// Was this module built with C++ interop enabled.
657+
bool isBuiltWithCxxInterop() const {
658+
return Bits.HasCxxInteroperability;
659+
}
660+
656661
llvm::VersionTuple getUserModuleVersion() const {
657662
return UserModuleVersion;
658663
}

lib/Serialization/ScanningLoaders.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ llvm::ErrorOr<ModuleDependencyInfo> SwiftModuleScanner::scanBinaryModuleFile(
340340
binaryModuleOptionalImports->moduleImports, linkLibraries,
341341
serializedSearchPaths, binaryModuleImports->headerImport,
342342
definingModulePath, isFramework, loadedModuleFile->isStaticLibrary(),
343+
loadedModuleFile->isBuiltWithCxxInterop(),
343344
/*module-cache-key*/ "", userModuleVer);
344345

345346
for (auto &macro : loadedModuleFile->getExternalMacros()) {

test/ScanDependencies/no-cxx-overlay-for-non-interop-interface.swift

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
12
// RUN: %empty-directory(%t)
23
// RUN: %empty-directory(%t/deps)
34
// RUN: split-file %s %t
@@ -8,17 +9,29 @@
89
// RUN: %target-swift-frontend -scan-dependencies -o %t/deps_no_interop_dep.json %t/clientNoInteropDep.swift -I %t/deps -cxx-interoperability-mode=default -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -verify
910
// RUN: cat %t/deps_no_interop_dep.json | %FileCheck %s -check-prefix=DISABLE-CHECK
1011

12+
// RUN: %target-swift-frontend -emit-module %t/BinaryDepNoInterop.swift -emit-module-path %t/deps/BinaryDepNoInterop.swiftmodule -module-name BinaryDepNoInterop -I %t/deps -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import
13+
// RUN: %target-swift-frontend -scan-dependencies -o %t/deps_no_interop_binary_dep.json %t/clientNoInteropBinaryDep.swift -I %t/deps -cxx-interoperability-mode=default -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -verify
14+
// RUN: cat %t/deps_no_interop_binary_dep.json | %FileCheck %s -check-prefix=DISABLE-BINARY-CHECK
15+
1116
// RUN: %target-swift-frontend -scan-dependencies -o %t/deps_darwin_dep.json %t/clientDarwin.swift -I %t/deps -cxx-interoperability-mode=default -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -verify
1217
// RUN: cat %t/deps_darwin_dep.json | %FileCheck %s -check-prefix=DARWIN-CHECK
1318

1419
//--- deps/bar.h
1520
void bar(void);
1621

22+
//--- deps/baz.h
23+
#include "bar.h"
24+
void baz(void);
25+
1726
//--- deps/module.modulemap
1827
module std_Bar [system] {
1928
header "bar.h"
2029
export *
2130
}
31+
module normal {
32+
header "baz.h"
33+
export *
34+
}
2235

2336
//--- deps/Foo.swiftinterface
2437
// swift-interface-format-version: 1.0
@@ -39,12 +52,19 @@ public struct Foo2 {}
3952
import std_Bar
4053
public struct Foo3 {}
4154

55+
//--- BinaryDepNoInterop.swift
56+
import normal
57+
public struct Foo6 {}
58+
4259
//--- clientWithInteropDep.swift
4360
import Foo
4461

4562
//--- clientNoInteropDep.swift
4663
import FooNoInterop
4764

65+
//--- clientNoInteropBinaryDep.swift
66+
import BinaryDepNoInterop
67+
4868
//--- clientDarwin.swift
4969
import Darwin
5070

@@ -87,21 +107,25 @@ import Darwin
87107
// DISABLE-CHECK: }
88108
// DISABLE-CHECK: ],
89109

90-
// Ensure that the the 'Darwin' dependency does not get the C++ standard library overlay for its 'std_*' dependencies
91-
//
92-
// 'Darwin' as it appears in direct deps
93-
// DARWIN-CHECK: "swift": "Darwin"
94-
// 'Darwin' as it appears in source-import deps
95-
// DARWIN-CHECK: "swift": "Darwin"
96-
// Actual dependency info node
97-
// DARWIN-CHECK: "swift": "Darwin"
110+
// DISABLE-BINARY-CHECK: "modulePath": "{{.*}}{{/|\\}}BinaryDepNoInterop.swiftmodule"
111+
// DISABLE-BINARY-CHECK: "directDependencies": [
112+
// DISABLE-BINARY-CHECK-NEXT: {
113+
// DISABLE-BINARY-CHECK-NEXT: "swift": "Swift"
114+
// DISABLE-BINARY-CHECK-NEXT: },
115+
// DISABLE-BINARY-CHECK-NEXT: {
116+
// DISABLE-BINARY-CHECK-NEXT: "swift": "SwiftOnoneSupport"
117+
// DISABLE-BINARY-CHECK-NEXT: },
118+
// DISABLE-BINARY-CHECK-NEXT: {
119+
// DISABLE-BINARY-CHECK-NEXT: "clang": "normal"
120+
// DISABLE-BINARY-CHECK-NEXT: }
121+
// DISABLE-BINARY-CHECK-NEXT: ],
122+
123+
// DARWIN-CHECK: "modulePath": "{{.*}}{{/|\\}}Darwin-{{.*}}.swiftmodule"
98124
// DARWIN-CHECK: "directDependencies": [
99-
// DARWIN-CHECK: {
100-
// DARWIN-CHECK: "swift": "SwiftOnoneSupport"
101-
// DARWIN-CHECK: },
102-
// DARWIN-CHECK: {
103-
// DARWIN-CHECK: "clang": "std_Bar"
104-
// DARWIN-CHECK: }
105-
// DARWIN-CHECK: ],
106-
107-
125+
// DARWIN-CHECK-NEXT: {
126+
// DARWIN-CHECK-NEXT: "swift": "SwiftOnoneSupport"
127+
// DARWIN-CHECK-NEXT: },
128+
// DARWIN-CHECK-NEXT: {
129+
// DARWIN-CHECK-NEXT: "clang": "std_Bar"
130+
// DARWIN-CHECK-NEXT: }
131+
// DARWIN-CHECK-NEXT: ],

0 commit comments

Comments
 (0)