Skip to content

Commit ecc5b6e

Browse files
[Modules] Avoid false swift module sharing
When the swiftmodule is built with different clang importer arguments, they can have the same module hash, causing them to be wrongly re-used even they contains different interfaces. Add ReducedExtraArgs to the module hash to disambiguate them. However, some Xcc arguments, most commonly `-D` options do not affect the swiftmodule being generated. Do not pass `-Xcc -DARGS` to swift interface compilation to reduce the amount of module variants in the build. rdar://131408266 (cherry picked from commit a32dd95)
1 parent 6741653 commit ecc5b6e

File tree

8 files changed

+114
-22
lines changed

8 files changed

+114
-22
lines changed

include/swift/ClangImporter/ClangImporter.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,8 @@ class ClangImporter final : public ClangModuleLoader {
517517
std::string getClangModuleHash() const;
518518

519519
/// Get clang import creation cc1 args for swift explicit module build.
520-
std::vector<std::string> getSwiftExplicitModuleDirectCC1Args() const;
520+
std::vector<std::string>
521+
getSwiftExplicitModuleDirectCC1Args(bool isInterface) const;
521522

522523
/// If we already imported a given decl successfully, return the corresponding
523524
/// Swift decl as an Optional<Decl *>, but if we previously tried and failed

lib/Basic/LangOptions.cpp

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -706,22 +706,23 @@ namespace {
706706
"-working-directory=",
707707
"-working-directory"};
708708

709-
constexpr std::array<std::string_view, 15> knownClangDependencyIgnorablePrefiexes =
710-
{"-I",
711-
"-F",
712-
"-fmodule-map-file=",
713-
"-iquote",
714-
"-idirafter",
715-
"-iframeworkwithsysroot",
716-
"-iframework",
717-
"-iprefix",
718-
"-iwithprefixbefore",
719-
"-iwithprefix",
720-
"-isystemafter",
721-
"-isystem",
722-
"-isysroot",
723-
"-working-directory=",
724-
"-working-directory"};
709+
constexpr std::array<std::string_view, 16>
710+
knownClangDependencyIgnorablePrefiexes = {"-I",
711+
"-F",
712+
"-fmodule-map-file=",
713+
"-iquote",
714+
"-idirafter",
715+
"-iframeworkwithsysroot",
716+
"-iframework",
717+
"-iprefix",
718+
"-iwithprefixbefore",
719+
"-iwithprefix",
720+
"-isystemafter",
721+
"-isystem",
722+
"-isysroot",
723+
"-working-directory=",
724+
"-working-directory",
725+
"-D"};
725726
}
726727

727728
std::vector<std::string> ClangImporterOptions::getRemappedExtraArgs(

lib/ClangImporter/ClangImporter.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4055,7 +4055,7 @@ std::string ClangImporter::getClangModuleHash() const {
40554055
}
40564056

40574057
std::vector<std::string>
4058-
ClangImporter::getSwiftExplicitModuleDirectCC1Args() const {
4058+
ClangImporter::getSwiftExplicitModuleDirectCC1Args(bool isInterface) const {
40594059
llvm::SmallVector<const char*> clangArgs;
40604060
clangArgs.reserve(Impl.ClangArgs.size());
40614061
llvm::for_each(Impl.ClangArgs, [&](const std::string &Arg) {
@@ -4105,6 +4105,14 @@ ClangImporter::getSwiftExplicitModuleDirectCC1Args() const {
41054105
PPOpts.MacroIncludes.clear();
41064106
PPOpts.Includes.clear();
41074107

4108+
// Clear specific options that will not affect swiftinterface compilation, but
4109+
// might affect main Module.
4110+
if (isInterface) {
4111+
// Interfacefile should not need `-D` but pass to main module in case it
4112+
// needs to directly import clang headers.
4113+
PPOpts.Macros.clear();
4114+
}
4115+
41084116
if (Impl.SwiftContext.ClangImporterOpts.UseClangIncludeTree) {
41094117
// FileSystemOptions.
41104118
auto &FSOpts = instance.getFileSystemOpts();

lib/DependencyScan/ModuleDependencyScanner.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,8 @@ ModuleDependencyScanner::getMainModuleDependencyInfo(ModuleDecl *mainModule) {
369369
std::vector<std::string> buildArgs;
370370
if (ScanASTContext.ClangImporterOpts.ClangImporterDirectCC1Scan) {
371371
buildArgs.push_back("-direct-clang-cc1-module-build");
372-
for (auto &arg : clangImporter->getSwiftExplicitModuleDirectCC1Args()) {
372+
for (auto &arg : clangImporter->getSwiftExplicitModuleDirectCC1Args(
373+
/*isInterface=*/false)) {
373374
buildArgs.push_back("-Xcc");
374375
buildArgs.push_back(arg);
375376
}

lib/Frontend/ModuleInterfaceLoader.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2035,6 +2035,8 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath,
20352035
auto normalizedTargetTriple =
20362036
getTargetSpecificModuleTriple(genericSubInvocation.getLangOptions().Target);
20372037
std::string sdkBuildVersion = getSDKBuildVersion(sdkPath);
2038+
const auto ExtraArgs = genericSubInvocation.getClangImporterOptions()
2039+
.getReducedExtraArgsForSwiftModuleDependency();
20382040

20392041
llvm::hash_code H = hash_combine(
20402042
// Start with the compiler version (which will be either tag names or
@@ -2076,6 +2078,10 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath,
20762078
// correctly load the dependencies.
20772079
genericSubInvocation.getCASOptions().getModuleScanningHashComponents(),
20782080

2081+
// Clang ExtraArgs that affects how clang types are imported into swift
2082+
// module.
2083+
llvm::hash_combine_range(ExtraArgs.begin(), ExtraArgs.end()),
2084+
20792085
// Whether or not OSSA modules are enabled.
20802086
//
20812087
// If OSSA modules are enabled, we use a separate namespace of modules to

lib/Serialization/ScanningLoaders.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,8 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath,
182182
Args.push_back("-direct-clang-cc1-module-build");
183183
auto *importer =
184184
static_cast<ClangImporter *>(Ctx.getClangModuleLoader());
185-
for (auto &Arg : importer->getSwiftExplicitModuleDirectCC1Args()) {
185+
for (auto &Arg : importer->getSwiftExplicitModuleDirectCC1Args(
186+
/*isInterface=*/true)) {
186187
Args.push_back("-Xcc");
187188
Args.push_back(Arg);
188189
}

test/CAS/Xcc_args.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
private import _Macro
3737

3838
public func test() {
39-
let _ = VERSION
39+
// Check the VERSION is from command-line, thus a Int32, not string.
40+
let _ : Int32 = VERSION
4041
}
4142

4243
//--- include/module.modulemap
@@ -49,7 +50,7 @@ module _Macro {
4950
#if defined(_VERSION)
5051
#define VERSION _VERSION
5152
#else
52-
#define VERSION 0
53+
#define VERSION "not available"
5354
#endif
5455

5556
//--- hmap.json
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// REQUIRES: objc_interop
2+
// RUN: %empty-directory(%t)
3+
// RUN: split-file %s %t
4+
5+
// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
6+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
7+
// RUN: -o %t/deps-1.json -I %t/include
8+
9+
// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
10+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
11+
// RUN: -o %t/deps-2.json -Xcc -DHAS_FOO=1 -I %t/include
12+
13+
// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
14+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
15+
// RUN: -o %t/deps-3.json -Xcc -fapplication-extension -I %t/include
16+
17+
/// Check module hash for the swiftmodule. 1 and 2 should match, but not 3.
18+
// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-1.json Library modulePath > %t/path-1
19+
// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-2.json Library modulePath > %t/path-2
20+
// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-3.json Library modulePath > %t/path-3
21+
// RUN: diff %t/path-1 %t/path-2
22+
// RUN: not diff %t/path-1 %t/path-3
23+
24+
/// Check build command (exclude dependency module file path). 1 and 2 should match, but not 3.
25+
// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-1.json Library | grep -v fmodule-file= > %t/lib-1.cmd
26+
// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-2.json Library | grep -v fmodule-file= > %t/lib-2.cmd
27+
// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-3.json Library | grep -v fmodule-file= > %t/lib-3.cmd
28+
// RUN: diff %t/lib-1.cmd %t/lib-2.cmd
29+
// RUN: not diff %t/lib-1.cmd %t/lib-3.cmd
30+
31+
/// Test direct-cc1 mode.
32+
// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
33+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
34+
// RUN: -o %t/deps-4.json -I %t/include -experimental-clang-importer-direct-cc1-scan
35+
// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
36+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
37+
// RUN: -o %t/deps-5.json -Xcc -DHAS_FOO=1 -I %t/include -experimental-clang-importer-direct-cc1-scan
38+
// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/main.swift -module-cache-path %t/clang-module-cache \
39+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -O \
40+
// RUN: -o %t/deps-6.json -Xcc -fapplication-extension -I %t/include -experimental-clang-importer-direct-cc1-scan
41+
42+
// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-4.json Library modulePath > %t/path-4
43+
// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-5.json Library modulePath > %t/path-5
44+
// RUN: %{python} %S/../CAS/Inputs/SwiftDepsExtractor.py %t/deps-6.json Library modulePath > %t/path-6
45+
// RUN: diff %t/path-4 %t/path-5
46+
// RUN: not diff %t/path-4 %t/path-6
47+
// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-4.json Library | grep -v fmodule-file= > %t/lib-4.cmd
48+
// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-5.json Library | grep -v fmodule-file= > %t/lib-5.cmd
49+
// RUN: %{python} %S/../CAS/Inputs/BuildCommandExtractor.py %t/deps-6.json Library | grep -v fmodule-file= > %t/lib-6.cmd
50+
// RUN: diff %t/lib-4.cmd %t/lib-5.cmd
51+
// RUN: not diff %t/lib-4.cmd %t/lib-6.cmd
52+
53+
//--- main.swift
54+
import Library
55+
56+
//--- include/Library.swiftinterface
57+
// swift-interface-format-version: 1.0
58+
// swift-module-flags: -module-name Library -O -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -user-module-version 1.0
59+
import Swift
60+
@_exported import A
61+
public func test() {}
62+
63+
//--- include/a.h
64+
#ifdef HAS_FOO
65+
void foo(void);
66+
#endif
67+
void bar(void);
68+
69+
//--- include/module.modulemap
70+
module A {
71+
header "a.h"
72+
export *
73+
}

0 commit comments

Comments
 (0)