Skip to content

Commit 928cc6c

Browse files
hnrklssnhnrklssn
committed
[ClangImporter] Deduplicate imports copied from submodule to TLM
The imported top-level module inherits the imports of all its (transitive) submodules. Since multiple submodules can import the same modules these need to be deduplicated to avoid redundant work.
1 parent 909908d commit 928cc6c

File tree

5 files changed

+73
-68
lines changed

5 files changed

+73
-68
lines changed

‎include/swift/AST/Import.h‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,20 @@ class ImportPath : public detail::ImportPathBase<ImportPath> {
457457
Access getAccessPath(ImportKind importKind) const {
458458
return getAccessPath(isScopedImportKind(importKind));
459459
}
460+
461+
private:
462+
struct UnsafePrivateConstructorTag {};
463+
464+
// Doesn't require a module name like the public constructor.
465+
// Only used for getEmptyKey() and getTombstoneKey().
466+
ImportPath(Raw raw, UnsafePrivateConstructorTag tag) : ImportPathBase(raw) {}
467+
public:
468+
static ImportPath getEmptyKey() {
469+
return swift::ImportPath(llvm::DenseMapInfo<Raw>::getEmptyKey(), UnsafePrivateConstructorTag{});
470+
}
471+
static ImportPath getTombstoneKey() {
472+
return swift::ImportPath(llvm::DenseMapInfo<Raw>::getTombstoneKey(), UnsafePrivateConstructorTag{});
473+
}
460474
};
461475

462476
// MARK: - Abstractions of imports
@@ -810,6 +824,27 @@ struct DenseMapInfo<swift::AttributedImport<ModuleInfo>> {
810824
a.accessLevelRange == b.accessLevelRange;
811825
}
812826
};
827+
828+
template <>
829+
class DenseMapInfo<swift::ImportPath> {
830+
using ImportPath = swift::ImportPath;
831+
public:
832+
static ImportPath getEmptyKey() {
833+
return swift::ImportPath::getEmptyKey();
834+
}
835+
static ImportPath getTombstoneKey() {
836+
return swift::ImportPath::getTombstoneKey();
837+
}
838+
839+
static unsigned getHashValue(const ImportPath &val) {
840+
return llvm::DenseMapInfo<ImportPath::Raw>::getHashValue(val.getRaw());
841+
}
842+
843+
static bool isEqual(const ImportPath &lhs,
844+
const ImportPath &rhs) {
845+
return lhs == rhs;
846+
}
847+
};
813848
}
814849

815850
#endif

‎include/swift/Basic/Located.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ struct DenseMapInfo<swift::Located<T>> {
7171
}
7272

7373
static unsigned getHashValue(const swift::Located<T> &LocatedVal) {
74-
return combineHashValue(DenseMapInfo<T>::getHashValue(LocatedVal.Item),
74+
return detail::combineHashValue(DenseMapInfo<T>::getHashValue(LocatedVal.Item),
7575
DenseMapInfo<swift::SourceLoc>::getHashValue(LocatedVal.Loc));
7676
}
7777

@@ -82,4 +82,12 @@ struct DenseMapInfo<swift::Located<T>> {
8282
};
8383
} // namespace llvm
8484

85+
namespace swift {
86+
template<typename T>
87+
llvm::hash_code hash_value(const Located<T> &LocatedVal) {
88+
return llvm::DenseMapInfo<Located<T>>::getHashValue(LocatedVal);
89+
}
90+
} // namespace swift
91+
92+
8593
#endif // SWIFT_BASIC_LOCATED_H

‎lib/ClangImporter/ClangImporter.cpp‎

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

2838-
// Decls in this Swift module may originate in the top-level clang module with the
2839-
// same name, since decls in clang submodules are dumped into the top-level module.
2840-
// Add all transitive submodules as implicit imports in case any of them are marked
2841-
// `explicit`. The content in explicit modules isn't normally made visible when
2842-
// importing the parent module, but in this case the parent module is all we have.
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.
28432845
llvm::SmallVector<const clang::Module *, 32> SubmoduleWorklist;
2846+
llvm::DenseSet<ImportPath> Imported;
28442847
SubmoduleWorklist.push_back(underlying);
2848+
std::string underlyingSwiftModuleName =
2849+
isCxxStdModule(underlying)
2850+
? static_cast<std::string>(SwiftContext.Id_CxxStdlib)
2851+
: underlying->getFullModuleName();
2852+
ImportPath::Builder underlyingImportPath(SwiftContext,
2853+
underlyingSwiftModuleName, '.');
2854+
Imported.insert(underlyingImportPath.get());
2855+
for (auto UI : implicitImportInfo.AdditionalUnloadedImports)
2856+
Imported.insert(UI.module.getImportPath());
2857+
assert(implicitImportInfo.AdditionalImports.empty());
2858+
28452859
while (!SubmoduleWorklist.empty()) {
28462860
const clang::Module *CurrModule = SubmoduleWorklist.pop_back_val();
28472861
for (auto *I : CurrModule->Imports) {
2848-
// Make sure that synthesized Swift code in the clang module wrapper
2849-
// (e.g. _SwiftifyImport macro expansions) can access the same symbols as
2850-
// if it were actually in the clang module, by copying the imports.
2851-
std::string swiftModuleName = isCxxStdModule(I) ?
2852-
static_cast<std::string>(SwiftContext.Id_CxxStdlib) :
2853-
I->getFullModuleName();
2862+
std::string swiftModuleName =
2863+
isCxxStdModule(I)
2864+
? static_cast<std::string>(SwiftContext.Id_CxxStdlib)
2865+
: I->getFullModuleName();
28542866
ImportPath::Builder importPath(SwiftContext, swiftModuleName, '.');
2855-
UnloadedImportedModule importedModule(importPath.copyTo(SwiftContext), ImportKind::Module);
2867+
if (Imported.count(importPath.get()))
2868+
continue;
2869+
UnloadedImportedModule importedModule(importPath.copyTo(SwiftContext),
2870+
ImportKind::Module);
2871+
Imported.insert(importedModule.getImportPath());
28562872
implicitImportInfo.AdditionalUnloadedImports.push_back(importedModule);
28572873
}
28582874
for (auto *Submodule : CurrModule->submodules())
28592875
SubmoduleWorklist.push_back(Submodule);
28602876
}
2877+
28612878
ClangModuleUnit *file = nullptr;
28622879
auto wrapper = ModuleDecl::create(name, SwiftContext, implicitImportInfo,
28632880
[&](ModuleDecl *wrapper, auto addFile) {

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ d1_t b1d(void * _Nonnull __sized_by(size), int size);
8383
// DUMP-NEXT: imports for @__swiftmacro_So3b1b15_SwiftifyImportfMp_.swift:
8484
// DUMP-NEXT: Swift
8585
// DUMP-NEXT: D1
86-
// DUMP-NEXT: D1
8786
// DUMP-NEXT: C1
8887
// DUMP-CXX-NEXT: CxxShim
8988
// DUMP-CXX-NEXT: Cxx
@@ -97,7 +96,6 @@ d1_t b1d(void * _Nonnull __sized_by(size), int size);
9796
// DUMP-NEXT: imports for @__swiftmacro_So3b1c15_SwiftifyImportfMp_.swift:
9897
// DUMP-NEXT: Swift
9998
// DUMP-NEXT: D1
100-
// DUMP-NEXT: D1
10199
// DUMP-NEXT: C1
102100
// DUMP-CXX-NEXT: CxxShim
103101
// DUMP-CXX-NEXT: Cxx
@@ -111,7 +109,6 @@ d1_t b1d(void * _Nonnull __sized_by(size), int size);
111109
// DUMP-NEXT: imports for @__swiftmacro_So3b1d15_SwiftifyImportfMp_.swift:
112110
// DUMP-NEXT: Swift
113111
// DUMP-NEXT: D1
114-
// DUMP-NEXT: D1
115112
// DUMP-NEXT: C1
116113
// DUMP-CXX-NEXT: CxxShim
117114
// DUMP-CXX-NEXT: Cxx

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

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -132,19 +132,13 @@ module A2 {
132132
f1_t a1(void * _Nonnull __sized_by(size), int size);
133133

134134
// DUMP-NEXT: imports for A1.a1:
135-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So2a115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
136-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So2a115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
137135
// DUMP-NEXT: imports for @__swiftmacro_So2a115_SwiftifyImportfMp_.swift:
138136
// DUMP-NEXT: Swift
139137
// DUMP-NEXT: F1
140-
// DUMP-NEXT: D1
141-
// DUMP-NEXT: C1
142-
// DUMP-NEXT: F1
143138
// DUMP-NEXT: G1
144139
// DUMP-NEXT: D1
145140
// DUMP-NEXT: C1
146141
// DUMP-NEXT: B1
147-
// DUMP-NEXT: B1
148142
// DUMP-NEXT: E1
149143
// DUMP-CXX-NEXT: CxxShim
150144
// DUMP-CXX-NEXT: Cxx
@@ -167,19 +161,13 @@ f1_t a1(void * _Nonnull __sized_by(size), int size);
167161
c1_t b1(void * _Nonnull __sized_by(size), int size);
168162

169163
// DUMP-NEXT: imports for A1.b1:
170-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So2b115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
171-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So2b115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
172164
// DUMP-NEXT: imports for @__swiftmacro_So2b115_SwiftifyImportfMp_.swift:
173165
// DUMP-NEXT: Swift
174166
// DUMP-NEXT: F1
175-
// DUMP-NEXT: D1
176-
// DUMP-NEXT: C1
177-
// DUMP-NEXT: F1
178167
// DUMP-NEXT: G1
179168
// DUMP-NEXT: D1
180169
// DUMP-NEXT: C1
181170
// DUMP-NEXT: B1
182-
// DUMP-NEXT: B1
183171
// DUMP-NEXT: E1
184172
// DUMP-CXX-NEXT: CxxShim
185173
// DUMP-CXX-NEXT: Cxx
@@ -200,19 +188,13 @@ typedef int c1_t;
200188
f1_t c1(void * _Nonnull __sized_by(size), int size);
201189

202190
// DUMP-NEXT: imports for A1.c1:
203-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So2c115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
204-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So2c115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
205191
// DUMP-NEXT: imports for @__swiftmacro_So2c115_SwiftifyImportfMp_.swift:
206192
// DUMP-NEXT: Swift
207193
// DUMP-NEXT: F1
208-
// DUMP-NEXT: D1
209-
// DUMP-NEXT: C1
210-
// DUMP-NEXT: F1
211194
// DUMP-NEXT: G1
212195
// DUMP-NEXT: D1
213196
// DUMP-NEXT: C1
214197
// DUMP-NEXT: B1
215-
// DUMP-NEXT: B1
216198
// DUMP-NEXT: E1
217199
// DUMP-CXX-NEXT: CxxShim
218200
// DUMP-CXX-NEXT: Cxx
@@ -232,19 +214,13 @@ f1_t c1(void * _Nonnull __sized_by(size), int size);
232214
f1_t d1(void * _Nonnull __sized_by(size), int size);
233215

234216
// DUMP-NEXT: imports for A1.d1:
235-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So2d115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
236-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So2d115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
237217
// DUMP-NEXT: imports for @__swiftmacro_So2d115_SwiftifyImportfMp_.swift:
238218
// DUMP-NEXT: Swift
239219
// DUMP-NEXT: F1
240-
// DUMP-NEXT: D1
241-
// DUMP-NEXT: C1
242-
// DUMP-NEXT: F1
243220
// DUMP-NEXT: G1
244221
// DUMP-NEXT: D1
245222
// DUMP-NEXT: C1
246223
// DUMP-NEXT: B1
247-
// DUMP-NEXT: B1
248224
// DUMP-NEXT: E1
249225
// DUMP-CXX-NEXT: CxxShim
250226
// DUMP-CXX-NEXT: Cxx
@@ -271,19 +247,13 @@ f1_t e1_1(void * _Nonnull __sized_by(size), int size);
271247
g1_t e1_2(void * _Nonnull __sized_by(size), int size);
272248

273249
// DUMP-NEXT: imports for A1.e1_1:
274-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So4e1_115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
275-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So4e1_115_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
276250
// DUMP-NEXT: imports for @__swiftmacro_So4e1_115_SwiftifyImportfMp_.swift:
277251
// DUMP-NEXT: Swift
278252
// DUMP-NEXT: F1
279-
// DUMP-NEXT: D1
280-
// DUMP-NEXT: C1
281-
// DUMP-NEXT: F1
282253
// DUMP-NEXT: G1
283254
// DUMP-NEXT: D1
284255
// DUMP-NEXT: C1
285256
// DUMP-NEXT: B1
286-
// DUMP-NEXT: B1
287257
// DUMP-NEXT: E1
288258
// DUMP-CXX-NEXT: CxxShim
289259
// DUMP-CXX-NEXT: Cxx
@@ -294,19 +264,13 @@ g1_t e1_2(void * _Nonnull __sized_by(size), int size);
294264
// DUMP-NEXT: Swift
295265

296266
// DUMP-NEXT: imports for A1.e1_2:
297-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So4e1_215_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
298-
// DUMP-NEXT: <unknown>:0: warning: file '@__swiftmacro_So4e1_215_SwiftifyImportfMp_.swift' is part of module 'A1'; ignoring import
299267
// DUMP-NEXT: imports for @__swiftmacro_So4e1_215_SwiftifyImportfMp_.swift:
300268
// DUMP-NEXT: Swift
301269
// DUMP-NEXT: F1
302-
// DUMP-NEXT: D1
303-
// DUMP-NEXT: C1
304-
// DUMP-NEXT: F1
305270
// DUMP-NEXT: G1
306271
// DUMP-NEXT: D1
307272
// DUMP-NEXT: C1
308273
// DUMP-NEXT: B1
309-
// DUMP-NEXT: B1
310274
// DUMP-NEXT: E1
311275
// DUMP-CXX-NEXT: CxxShim
312276
// DUMP-CXX-NEXT: Cxx
@@ -352,10 +316,6 @@ c2_t b2_2(void * _Nonnull __sized_by(size), int size);
352316
// DUMP-NEXT: imports for @__swiftmacro_So4b2_115_SwiftifyImportfMp_.swift:
353317
// DUMP-NEXT: Swift
354318
// DUMP-NEXT: E2
355-
// DUMP-NEXT: C1
356-
// DUMP-NEXT: A1
357-
// DUMP-NEXT: B1
358-
// DUMP-NEXT: A1
359319
// DUMP-NEXT: D2
360320
// DUMP-NEXT: D1
361321
// DUMP-NEXT: A1
@@ -377,10 +337,6 @@ c2_t b2_2(void * _Nonnull __sized_by(size), int size);
377337
// DUMP-NEXT: imports for @__swiftmacro_So4b2_215_SwiftifyImportfMp_.swift:
378338
// DUMP-NEXT: Swift
379339
// DUMP-NEXT: E2
380-
// DUMP-NEXT: C1
381-
// DUMP-NEXT: A1
382-
// DUMP-NEXT: B1
383-
// DUMP-NEXT: A1
384340
// DUMP-NEXT: D2
385341
// DUMP-NEXT: D1
386342
// DUMP-NEXT: A1
@@ -415,10 +371,6 @@ e2_t c2(void * _Nonnull __sized_by(size), int size);
415371
// DUMP-NEXT: imports for @__swiftmacro_So2c215_SwiftifyImportfMp_.swift:
416372
// DUMP-NEXT: Swift
417373
// DUMP-NEXT: E2
418-
// DUMP-NEXT: C1
419-
// DUMP-NEXT: A1
420-
// DUMP-NEXT: B1
421-
// DUMP-NEXT: A1
422374
// DUMP-NEXT: D2
423375
// DUMP-NEXT: D1
424376
// DUMP-NEXT: A1
@@ -449,10 +401,6 @@ e2_t d2(void * _Nonnull __sized_by(size), int size);
449401
// DUMP-NEXT: imports for @__swiftmacro_So2d215_SwiftifyImportfMp_.swift:
450402
// DUMP-NEXT: Swift
451403
// DUMP-NEXT: E2
452-
// DUMP-NEXT: C1
453-
// DUMP-NEXT: A1
454-
// DUMP-NEXT: B1
455-
// DUMP-NEXT: A1
456404
// DUMP-NEXT: D2
457405
// DUMP-NEXT: D1
458406
// DUMP-NEXT: A1

0 commit comments

Comments
 (0)