Skip to content

Commit 369fc18

Browse files
hnrklssnhnrklssn
committed
[ClangImporter] Only import explicit submodules to the current module
Non-explicit submodules don't need to be explicitly added to the list of imports to be visible, since their decls are implicitly exported. Skip these modules even when present in the list of imports. Explicit submodules are imported *regardless* of whether another module imports them however.
1 parent c80b6f6 commit 369fc18

File tree

5 files changed

+90
-74
lines changed

5 files changed

+90
-74
lines changed

‎include/swift/ClangImporter/ClangImporter.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,12 @@ bool isCxxStdModule(const clang::Module *module);
702702
/// (std_vector, std_iosfwd, etc).
703703
bool isCxxStdModule(StringRef moduleName, bool IsSystem);
704704

705+
/// Build the path corresponding to the Swift wrapper for a clang module.
706+
/// Returns ImportPath::Builder to leave control over if and where to clone the
707+
/// path to the caller.
708+
ImportPath::Builder getSwiftModulePath(ASTContext &SwiftContext,
709+
const clang::Module *M);
710+
705711
/// Returns the pointee type if the given type is a C++ `const`
706712
/// reference type, `None` otherwise.
707713
std::optional<clang::QualType>

‎lib/ClangImporter/ClangImporter.cpp‎

Lines changed: 66 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2835,42 +2835,67 @@ ClangModuleUnit *ClangImporter::Implementation::getWrapperForModule(
28352835
implicitImportInfo = mainModule->getImplicitImportInfo();
28362836
}
28372837

2838-
// Make sure that synthesized Swift code in the clang module wrapper
2839-
// (e.g. _SwiftifyImport macro expansions) can access the same symbols
2840-
// as if it were actually in the clang module, by copying the imports.
2841-
// Because this top-level module wrapper contains all the imported decls
2842-
// of the clang submodules, we need to add the imports of all the
2843-
// transitive submodules, since we don't know at this point of the
2844-
// compilation which submodules will contain relevant macros.
28452838
if (!underlying->isSubModule()) {
2839+
// Make sure that synthesized Swift code in the clang module wrapper
2840+
// (e.g. _SwiftifyImport macro expansions) can access the same symbols
2841+
// as if it were actually in the clang module, by copying the imports.
2842+
// Because this top-level module wrapper contains all the imported decls
2843+
// of the clang submodules, we need to add the imports of all the
2844+
// transitive submodules, since we don't know at this point of the
2845+
// compilation which submodules will contain relevant macros.
2846+
// We also need to add (transitive) explicit submodules as imports,
2847+
// to make sure that they are marked as imported *somewhere* (clang modules
2848+
// including them don't count) - otherwise their decls won't be found after
2849+
// non-visible clang decls are filtered out.
28462850
llvm::SmallVector<const clang::Module *, 32> SubmoduleWorklist;
28472851
llvm::DenseSet<ImportPath> Imported;
28482852
SubmoduleWorklist.push_back(underlying);
2849-
std::string underlyingSwiftModuleName =
2850-
isCxxStdModule(underlying)
2851-
? static_cast<std::string>(SwiftContext.Id_CxxStdlib)
2852-
: underlying->getFullModuleName();
2853-
ImportPath::Builder underlyingImportPath(SwiftContext,
2854-
underlyingSwiftModuleName, '.');
2855-
Imported.insert(underlyingImportPath.get());
2853+
ImportPath::Builder underlyingSwiftModulePath =
2854+
getSwiftModulePath(underlying);
2855+
Imported.insert(underlyingSwiftModulePath.get());
28562856
for (auto UI : implicitImportInfo.AdditionalUnloadedImports)
28572857
Imported.insert(UI.module.getImportPath());
28582858
assert(implicitImportInfo.AdditionalImports.empty());
28592859

2860+
auto addImplicitImport = [&implicitImportInfo, &Imported,
2861+
this](const clang::Module *M,
2862+
bool guaranteedUnique) {
2863+
ImportPath::Builder builder = getSwiftModulePath(M);
2864+
if (!guaranteedUnique && Imported.count(builder.get()))
2865+
return;
2866+
2867+
// Don't perform this clone for modules already added to the list
2868+
ImportPath importedModulePath = builder.copyTo(SwiftContext);
2869+
2870+
#ifndef NDEBUG
2871+
const bool performSanityCheck = true;
2872+
#else
2873+
const bool performSanityCheck = false;
2874+
#endif
2875+
if (!guaranteedUnique || performSanityCheck) {
2876+
bool WasInserted = Imported.insert(importedModulePath).second;
2877+
assert(WasInserted);
2878+
}
2879+
2880+
UnloadedImportedModule importedModule(importedModulePath,
2881+
ImportKind::Module);
2882+
implicitImportInfo.AdditionalUnloadedImports.push_back(
2883+
std::move(importedModule));
2884+
};
2885+
28602886
while (!SubmoduleWorklist.empty()) {
28612887
const clang::Module *CurrModule = SubmoduleWorklist.pop_back_val();
2888+
if (CurrModule->IsExplicit) {
2889+
// We don't add imports under the same TLM, and submodules form
2890+
// a tree, so these don't require deduplication.
2891+
addImplicitImport(CurrModule, /*guaranteedUnique=*/true);
2892+
}
28622893
for (auto *I : CurrModule->Imports) {
2863-
std::string swiftModuleName =
2864-
isCxxStdModule(I)
2865-
? static_cast<std::string>(SwiftContext.Id_CxxStdlib)
2866-
: I->getFullModuleName();
2867-
ImportPath::Builder importPath(SwiftContext, swiftModuleName, '.');
2868-
if (Imported.count(importPath.get()))
2894+
// `underlying` is the current TLM. Only explicit submodules need to
2895+
// be imported under the same TLM, which is handled above.
2896+
if (I->getTopLevelModule() == underlying)
28692897
continue;
2870-
UnloadedImportedModule importedModule(importPath.copyTo(SwiftContext),
2871-
ImportKind::Module);
2872-
Imported.insert(importedModule.getImportPath());
2873-
implicitImportInfo.AdditionalUnloadedImports.push_back(importedModule);
2898+
addImplicitImport(I, /*guaranteedUnique=*/false);
28742899
}
28752900
for (auto *Submodule : CurrModule->submodules())
28762901
SubmoduleWorklist.push_back(Submodule);
@@ -3780,19 +3805,7 @@ ImportDecl *swift::createImportDecl(ASTContext &Ctx,
37803805
auto *ImportedMod = ClangN.getClangModule();
37813806
assert(ImportedMod);
37823807

3783-
ImportPath::Builder importPath;
3784-
auto *TmpMod = ImportedMod;
3785-
while (TmpMod) {
3786-
// If this is a C++ stdlib module, print its name as `CxxStdlib` instead of
3787-
// `std`. `CxxStdlib` is the only accepted spelling of the C++ stdlib module
3788-
// name in Swift.
3789-
Identifier moduleName = !TmpMod->isSubModule() && TmpMod->Name == "std"
3790-
? Ctx.Id_CxxStdlib
3791-
: Ctx.getIdentifier(TmpMod->Name);
3792-
importPath.push_back(moduleName);
3793-
TmpMod = TmpMod->Parent;
3794-
}
3795-
std::reverse(importPath.begin(), importPath.end());
3808+
ImportPath::Builder importPath = getSwiftModulePath(Ctx, ImportedMod);
37963809

37973810
bool IsExported = false;
37983811
for (auto *ExportedMod : Exported) {
@@ -8756,6 +8769,22 @@ bool importer::isCxxStdModule(StringRef moduleName, bool IsSystem) {
87568769
return false;
87578770
}
87588771

8772+
ImportPath::Builder importer::getSwiftModulePath(ASTContext &SwiftContext, const clang::Module *M) {
8773+
if (isCxxStdModule(M))
8774+
return ImportPath::Builder(SwiftContext.Id_CxxStdlib);
8775+
ImportPath::Builder builder;
8776+
while (M) {
8777+
builder.push_back(SwiftContext.getIdentifier(M->Name));
8778+
M = M->Parent;
8779+
}
8780+
std::reverse(builder.begin(), builder.end());
8781+
return builder;
8782+
}
8783+
8784+
ImportPath::Builder ClangImporter::Implementation::getSwiftModulePath(const clang::Module *M) {
8785+
return ::getSwiftModulePath(SwiftContext, M);
8786+
}
8787+
87598788
std::optional<clang::QualType>
87608789
importer::getCxxReferencePointeeTypeOrNone(const clang::Type *type) {
87618790
if (type->isReferenceType())

‎lib/ClangImporter/ImporterImpl.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,6 +1278,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
12781278
/// \returns The named module, or null if the module has not been imported.
12791279
ModuleDecl *getNamedModule(StringRef name);
12801280

1281+
ImportPath::Builder getSwiftModulePath(const clang::Module *M);
1282+
12811283
/// Returns the "Foundation" module, if it can be loaded.
12821284
///
12831285
/// After this has been called, the Foundation module will or won't be loaded

‎test/Interop/C/swiftify-import/clang-includes-indirect-explicit-modules.swift‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
// RUN: %target-swift-frontend -dump-source-file-imports -emit-module -plugin-path %swift-plugin-dir -o %t/ClangIncludesExplicit.swiftmodule -I %t/Inputs -enable-experimental-feature SafeInteropWrappers %t/test.swift 2>&1 | %FileCheck %s --check-prefix DUMP
77
// RUN: %target-swift-frontend -dump-source-file-imports -emit-module -plugin-path %swift-plugin-dir -o %t/ClangIncludesExplicit.swiftmodule -I %t/Inputs -enable-experimental-feature SafeInteropWrappers %t/test.swift -cxx-interoperability-mode=default 2>&1 | %FileCheck %s --check-prefix DUMP --check-prefix DUMP-CXX
88

9-
109
// RUN: %target-swift-ide-test -print-module -module-print-hidden -module-to-print=A1.B1 -plugin-path %swift-plugin-dir -I %t -source-filename=x -enable-experimental-feature SafeInteropWrappers | %FileCheck %s --check-prefix CHECK-B1 --implicit-check-not=import --match-full-lines
1110
// RUN: %target-swift-ide-test -print-module -module-print-hidden -module-to-print=A1.B1.C1 -plugin-path %swift-plugin-dir -I %t -source-filename=x -enable-experimental-feature SafeInteropWrappers | %FileCheck %s --check-prefix CHECK-C1 --implicit-check-not=import --match-full-lines
1211

@@ -84,6 +83,7 @@ d1_t b1d(void * _Nonnull __sized_by(size), int size);
8483
// DUMP-NEXT: Swift
8584
// DUMP-NEXT: D1
8685
// DUMP-NEXT: C1
86+
// DUMP-NEXT: B1
8787
// DUMP-CXX-NEXT: CxxShim
8888
// DUMP-CXX-NEXT: Cxx
8989
// DUMP-NEXT: _StringProcessing
@@ -97,6 +97,7 @@ d1_t b1d(void * _Nonnull __sized_by(size), int size);
9797
// DUMP-NEXT: Swift
9898
// DUMP-NEXT: D1
9999
// DUMP-NEXT: C1
100+
// DUMP-NEXT: B1
100101
// DUMP-CXX-NEXT: CxxShim
101102
// DUMP-CXX-NEXT: Cxx
102103
// DUMP-NEXT: _StringProcessing
@@ -110,6 +111,7 @@ d1_t b1d(void * _Nonnull __sized_by(size), int size);
110111
// DUMP-NEXT: Swift
111112
// DUMP-NEXT: D1
112113
// DUMP-NEXT: C1
114+
// DUMP-NEXT: B1
113115
// DUMP-CXX-NEXT: CxxShim
114116
// DUMP-CXX-NEXT: Cxx
115117
// DUMP-NEXT: _StringProcessing
@@ -157,6 +159,7 @@ d1_t b2d(void * _Nonnull __sized_by(size), int size);
157159
// DUMP-NEXT: A1
158160
// DUMP-NEXT: B1
159161
// DUMP-NEXT: A1
162+
// DUMP-NEXT: B2
160163
// DUMP-CXX-NEXT: CxxShim
161164
// DUMP-CXX-NEXT: Cxx
162165
// DUMP-NEXT: _StringProcessing
@@ -174,6 +177,7 @@ d1_t b2d(void * _Nonnull __sized_by(size), int size);
174177
// DUMP-NEXT: A1
175178
// DUMP-NEXT: B1
176179
// DUMP-NEXT: A1
180+
// DUMP-NEXT: B2
177181
// DUMP-CXX-NEXT: CxxShim
178182
// DUMP-CXX-NEXT: Cxx
179183
// DUMP-NEXT: _StringProcessing
@@ -191,6 +195,7 @@ d1_t b2d(void * _Nonnull __sized_by(size), int size);
191195
// DUMP-NEXT: A1
192196
// DUMP-NEXT: B1
193197
// DUMP-NEXT: A1
198+
// DUMP-NEXT: B2
194199
// DUMP-CXX-NEXT: CxxShim
195200
// DUMP-CXX-NEXT: Cxx
196201
// DUMP-NEXT: _StringProcessing

‎test/Interop/C/swiftify-import/clang-includes-redundant-imports.swift‎

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,9 @@ f1_t a1(void * _Nonnull __sized_by(size), int size);
134134
// DUMP-NEXT: imports for A1.a1:
135135
// DUMP-NEXT: imports for @__swiftmacro_So2a115_SwiftifyImportfMp_.swift:
136136
// DUMP-NEXT: Swift
137-
// DUMP-NEXT: F1
138-
// DUMP-NEXT: G1
139137
// DUMP-NEXT: D1
140-
// DUMP-NEXT: C1
141138
// DUMP-NEXT: B1
142-
// DUMP-NEXT: E1
139+
// DUMP-NEXT: F1
143140
// DUMP-CXX-NEXT: CxxShim
144141
// DUMP-CXX-NEXT: Cxx
145142
// DUMP-NEXT: _StringProcessing
@@ -163,12 +160,9 @@ c1_t b1(void * _Nonnull __sized_by(size), int size);
163160
// DUMP-NEXT: imports for A1.b1:
164161
// DUMP-NEXT: imports for @__swiftmacro_So2b115_SwiftifyImportfMp_.swift:
165162
// DUMP-NEXT: Swift
166-
// DUMP-NEXT: F1
167-
// DUMP-NEXT: G1
168163
// DUMP-NEXT: D1
169-
// DUMP-NEXT: C1
170164
// DUMP-NEXT: B1
171-
// DUMP-NEXT: E1
165+
// DUMP-NEXT: F1
172166
// DUMP-CXX-NEXT: CxxShim
173167
// DUMP-CXX-NEXT: Cxx
174168
// DUMP-NEXT: _StringProcessing
@@ -190,12 +184,9 @@ f1_t c1(void * _Nonnull __sized_by(size), int size);
190184
// DUMP-NEXT: imports for A1.c1:
191185
// DUMP-NEXT: imports for @__swiftmacro_So2c115_SwiftifyImportfMp_.swift:
192186
// DUMP-NEXT: Swift
193-
// DUMP-NEXT: F1
194-
// DUMP-NEXT: G1
195187
// DUMP-NEXT: D1
196-
// DUMP-NEXT: C1
197188
// DUMP-NEXT: B1
198-
// DUMP-NEXT: E1
189+
// DUMP-NEXT: F1
199190
// DUMP-CXX-NEXT: CxxShim
200191
// DUMP-CXX-NEXT: Cxx
201192
// DUMP-NEXT: _StringProcessing
@@ -216,12 +207,9 @@ f1_t d1(void * _Nonnull __sized_by(size), int size);
216207
// DUMP-NEXT: imports for A1.d1:
217208
// DUMP-NEXT: imports for @__swiftmacro_So2d115_SwiftifyImportfMp_.swift:
218209
// DUMP-NEXT: Swift
219-
// DUMP-NEXT: F1
220-
// DUMP-NEXT: G1
221210
// DUMP-NEXT: D1
222-
// DUMP-NEXT: C1
223211
// DUMP-NEXT: B1
224-
// DUMP-NEXT: E1
212+
// DUMP-NEXT: F1
225213
// DUMP-CXX-NEXT: CxxShim
226214
// DUMP-CXX-NEXT: Cxx
227215
// DUMP-NEXT: _StringProcessing
@@ -249,12 +237,9 @@ g1_t e1_2(void * _Nonnull __sized_by(size), int size);
249237
// DUMP-NEXT: imports for A1.e1_1:
250238
// DUMP-NEXT: imports for @__swiftmacro_So4e1_115_SwiftifyImportfMp_.swift:
251239
// DUMP-NEXT: Swift
252-
// DUMP-NEXT: F1
253-
// DUMP-NEXT: G1
254240
// DUMP-NEXT: D1
255-
// DUMP-NEXT: C1
256241
// DUMP-NEXT: B1
257-
// DUMP-NEXT: E1
242+
// DUMP-NEXT: F1
258243
// DUMP-CXX-NEXT: CxxShim
259244
// DUMP-CXX-NEXT: Cxx
260245
// DUMP-NEXT: _StringProcessing
@@ -266,12 +251,9 @@ g1_t e1_2(void * _Nonnull __sized_by(size), int size);
266251
// DUMP-NEXT: imports for A1.e1_2:
267252
// DUMP-NEXT: imports for @__swiftmacro_So4e1_215_SwiftifyImportfMp_.swift:
268253
// DUMP-NEXT: Swift
269-
// DUMP-NEXT: F1
270-
// DUMP-NEXT: G1
271254
// DUMP-NEXT: D1
272-
// DUMP-NEXT: C1
273255
// DUMP-NEXT: B1
274-
// DUMP-NEXT: E1
256+
// DUMP-NEXT: F1
275257
// DUMP-CXX-NEXT: CxxShim
276258
// DUMP-CXX-NEXT: Cxx
277259
// DUMP-NEXT: _StringProcessing
@@ -315,11 +297,9 @@ c2_t b2_2(void * _Nonnull __sized_by(size), int size);
315297
// DUMP-NEXT: imports for A2.b2_1:
316298
// DUMP-NEXT: imports for @__swiftmacro_So4b2_115_SwiftifyImportfMp_.swift:
317299
// DUMP-NEXT: Swift
318-
// DUMP-NEXT: E2
319-
// DUMP-NEXT: D2
300+
// DUMP-NEXT: C2
320301
// DUMP-NEXT: D1
321302
// DUMP-NEXT: A1
322-
// DUMP-NEXT: C2
323303
// DUMP-NEXT: C1
324304
// DUMP-NEXT: A1
325305
// DUMP-NEXT: B1
@@ -336,11 +316,9 @@ c2_t b2_2(void * _Nonnull __sized_by(size), int size);
336316
// DUMP-NEXT: imports for A2.b2_2:
337317
// DUMP-NEXT: imports for @__swiftmacro_So4b2_215_SwiftifyImportfMp_.swift:
338318
// DUMP-NEXT: Swift
339-
// DUMP-NEXT: E2
340-
// DUMP-NEXT: D2
319+
// DUMP-NEXT: C2
341320
// DUMP-NEXT: D1
342321
// DUMP-NEXT: A1
343-
// DUMP-NEXT: C2
344322
// DUMP-NEXT: C1
345323
// DUMP-NEXT: A1
346324
// DUMP-NEXT: B1
@@ -370,11 +348,9 @@ e2_t c2(void * _Nonnull __sized_by(size), int size);
370348
// DUMP-NEXT: imports for A2.c2:
371349
// DUMP-NEXT: imports for @__swiftmacro_So2c215_SwiftifyImportfMp_.swift:
372350
// DUMP-NEXT: Swift
373-
// DUMP-NEXT: E2
374-
// DUMP-NEXT: D2
351+
// DUMP-NEXT: C2
375352
// DUMP-NEXT: D1
376353
// DUMP-NEXT: A1
377-
// DUMP-NEXT: C2
378354
// DUMP-NEXT: C1
379355
// DUMP-NEXT: A1
380356
// DUMP-NEXT: B1
@@ -400,11 +376,9 @@ e2_t d2(void * _Nonnull __sized_by(size), int size);
400376
// DUMP-NEXT: imports for A2.d2:
401377
// DUMP-NEXT: imports for @__swiftmacro_So2d215_SwiftifyImportfMp_.swift:
402378
// DUMP-NEXT: Swift
403-
// DUMP-NEXT: E2
404-
// DUMP-NEXT: D2
379+
// DUMP-NEXT: C2
405380
// DUMP-NEXT: D1
406381
// DUMP-NEXT: A1
407-
// DUMP-NEXT: C2
408382
// DUMP-NEXT: C1
409383
// DUMP-NEXT: A1
410384
// DUMP-NEXT: B1

0 commit comments

Comments
 (0)