Skip to content

Commit ea526c6

Browse files
dfsweeneydfsweeney
authored
Converting ModuleDecl::ImportedModule from std::pair to a dedicated struct. (swiftlang#31360)
1 parent f503a2d commit ea526c6

25 files changed

+211
-171
lines changed

‎include/swift/AST/Module.h

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -209,26 +209,46 @@ class ModuleDecl : public DeclContext, public TypeDecl {
209209

210210
public:
211211
typedef ArrayRef<Located<Identifier>> AccessPathTy;
212-
typedef std::pair<ModuleDecl::AccessPathTy, ModuleDecl*> ImportedModule;
213-
212+
/// Convenience struct to keep track of a module along with its access path.
213+
struct ImportedModule {
214+
/// The access path from an import: `import Foo.Bar` -> `Foo.Bar`.
215+
ModuleDecl::AccessPathTy accessPath;
216+
/// The actual module corresponding to the import.
217+
///
218+
/// Invariant: The pointer is non-null.
219+
ModuleDecl *importedModule;
220+
221+
ImportedModule(ModuleDecl::AccessPathTy accessPath,
222+
ModuleDecl *importedModule)
223+
: accessPath(accessPath), importedModule(importedModule) {
224+
assert(this->importedModule);
225+
}
226+
227+
bool operator==(const ModuleDecl::ImportedModule &other) const {
228+
return (this->importedModule == other.importedModule) &&
229+
(this->accessPath == other.accessPath);
230+
}
231+
};
232+
214233
static bool matchesAccessPath(AccessPathTy AccessPath, DeclName Name) {
215234
assert(AccessPath.size() <= 1 && "can only refer to top-level decls");
216235

217236
return AccessPath.empty()
218237
|| DeclName(AccessPath.front().Item).matchesRef(Name);
219238
}
220-
239+
221240
/// Arbitrarily orders ImportedModule records, for inclusion in sets and such.
222241
class OrderImportedModules {
223242
public:
224243
bool operator()(const ImportedModule &lhs,
225244
const ImportedModule &rhs) const {
226-
if (lhs.second != rhs.second)
227-
return std::less<const ModuleDecl *>()(lhs.second, rhs.second);
228-
if (lhs.first.data() != rhs.first.data())
229-
return std::less<AccessPathTy::iterator>()(lhs.first.begin(),
230-
rhs.first.begin());
231-
return lhs.first.size() < rhs.first.size();
245+
if (lhs.importedModule != rhs.importedModule)
246+
return std::less<const ModuleDecl *>()(lhs.importedModule,
247+
rhs.importedModule);
248+
if (lhs.accessPath.data() != rhs.accessPath.data())
249+
return std::less<AccessPathTy::iterator>()(lhs.accessPath.begin(),
250+
rhs.accessPath.begin());
251+
return lhs.accessPath.size() < rhs.accessPath.size();
232252
}
233253
};
234254

@@ -860,14 +880,14 @@ namespace llvm {
860880
}
861881

862882
static unsigned getHashValue(const ModuleDecl::ImportedModule &val) {
863-
auto pair = std::make_pair(val.first.size(), val.second);
883+
auto pair = std::make_pair(val.accessPath.size(), val.importedModule);
864884
return llvm::DenseMapInfo<decltype(pair)>::getHashValue(pair);
865885
}
866886

867887
static bool isEqual(const ModuleDecl::ImportedModule &lhs,
868888
const ModuleDecl::ImportedModule &rhs) {
869-
return lhs.second == rhs.second &&
870-
ModuleDecl::isSameAccessPath(lhs.first, rhs.first);
889+
return lhs.importedModule == rhs.importedModule &&
890+
ModuleDecl::isSameAccessPath(lhs.accessPath, rhs.accessPath);
871891
}
872892
};
873893
}

‎lib/AST/AutoDiff.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ bool swift::isDifferentiableProgrammingEnabled(SourceFile &SF) {
134134
// the given source file.
135135
bool importsDifferentiationModule = false;
136136
for (auto import : namelookup::getAllImports(&SF)) {
137-
if (import.second->getName() == ctx.Id_Differentiation) {
137+
if (import.importedModule->getName() == ctx.Id_Differentiation) {
138138
importsDifferentiationModule = true;
139139
break;
140140
}

‎lib/AST/ImportCache.cpp

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -55,25 +55,26 @@ void ImportSet::Profile(
5555
ArrayRef<ModuleDecl::ImportedModule> topLevelImports) {
5656
ID.AddInteger(topLevelImports.size());
5757
for (auto import : topLevelImports) {
58-
ID.AddInteger(import.first.size());
59-
for (auto accessPathElt : import.first) {
58+
ID.AddInteger(import.accessPath.size());
59+
for (auto accessPathElt : import.accessPath) {
6060
ID.AddPointer(accessPathElt.Item.getAsOpaquePointer());
6161
}
62-
ID.AddPointer(import.second);
62+
ID.AddPointer(import.importedModule);
6363
}
6464
}
6565

6666
static void collectExports(ModuleDecl::ImportedModule next,
6767
SmallVectorImpl<ModuleDecl::ImportedModule> &stack) {
6868
SmallVector<ModuleDecl::ImportedModule, 4> exports;
69-
next.second->getImportedModulesForLookup(exports);
69+
next.importedModule->getImportedModulesForLookup(exports);
7070
for (auto exported : exports) {
71-
if (next.first.empty())
71+
if (next.accessPath.empty())
7272
stack.push_back(exported);
73-
else if (exported.first.empty()) {
74-
exported.first = next.first;
73+
else if (exported.accessPath.empty()) {
74+
exported.accessPath = next.accessPath;
7575
stack.push_back(exported);
76-
} else if (ModuleDecl::isSameAccessPath(next.first, exported.first)) {
76+
} else if (ModuleDecl::isSameAccessPath(next.accessPath,
77+
exported.accessPath)) {
7778
stack.push_back(exported);
7879
}
7980
}
@@ -97,7 +98,7 @@ ImportCache::getImportSet(ASTContext &ctx,
9798
continue;
9899

99100
topLevelImports.push_back(next);
100-
if (next.second == headerImportModule)
101+
if (next.importedModule == headerImportModule)
101102
hasHeaderImportModule = true;
102103
}
103104

@@ -127,7 +128,7 @@ ImportCache::getImportSet(ASTContext &ctx,
127128
continue;
128129

129130
transitiveImports.push_back(next);
130-
if (next.second == headerImportModule)
131+
if (next.importedModule == headerImportModule)
131132
hasHeaderImportModule = true;
132133

133134
collectExports(next, stack);
@@ -173,7 +174,9 @@ ImportSet &ImportCache::getImportSet(const DeclContext *dc) {
173174
ctx.Stats->getFrontendCounters().ImportSetCacheMiss++;
174175

175176
SmallVector<ModuleDecl::ImportedModule, 4> imports;
176-
imports.emplace_back(ModuleDecl::AccessPathTy(), mod);
177+
178+
imports.emplace_back(
179+
ModuleDecl::ImportedModule{ModuleDecl::AccessPathTy(), mod});
177180

178181
if (file) {
179182
ModuleDecl::ImportFilter importFilter;
@@ -220,10 +223,10 @@ ImportCache::getAllVisibleAccessPaths(const ModuleDecl *mod,
220223
SmallVector<ModuleDecl::AccessPathTy, 1> accessPaths;
221224
for (auto next : getImportSet(dc).getAllImports()) {
222225
// If we found 'mod', record the access path.
223-
if (next.second == mod) {
226+
if (next.importedModule == mod) {
224227
// Make sure the list of access paths is unique.
225-
if (!llvm::is_contained(accessPaths, next.first))
226-
accessPaths.push_back(next.first);
228+
if (!llvm::is_contained(accessPaths, next.accessPath))
229+
accessPaths.push_back(next.accessPath);
227230
}
228231
}
229232

@@ -258,7 +261,8 @@ ImportCache::getAllAccessPathsNotShadowedBy(const ModuleDecl *mod,
258261
SmallVector<ModuleDecl::ImportedModule, 4> stack;
259262
llvm::SmallDenseSet<ModuleDecl::ImportedModule, 32> visited;
260263

261-
stack.emplace_back(ModuleDecl::AccessPathTy(), currentMod);
264+
stack.emplace_back(
265+
ModuleDecl::ImportedModule{ModuleDecl::AccessPathTy(), currentMod});
262266

263267
if (auto *file = dyn_cast<FileUnit>(dc)) {
264268
ModuleDecl::ImportFilter importFilter;
@@ -278,15 +282,15 @@ ImportCache::getAllAccessPathsNotShadowedBy(const ModuleDecl *mod,
278282
continue;
279283

280284
// Don't visit the 'other' module's re-exports.
281-
if (next.second == other)
285+
if (next.importedModule == other)
282286
continue;
283287

284288
// If we found 'mod' via some access path, remember the access
285289
// path.
286-
if (next.second == mod) {
290+
if (next.importedModule == mod) {
287291
// Make sure the list of access paths is unique.
288-
if (!llvm::is_contained(accessPaths, next.first))
289-
accessPaths.push_back(next.first);
292+
if (!llvm::is_contained(accessPaths, next.accessPath))
293+
accessPaths.push_back(next.accessPath);
290294
}
291295

292296
collectExports(next, stack);

‎lib/AST/Module.cpp

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,17 +1215,16 @@ lookupOperatorDeclForName(const FileUnit &File, SourceLoc Loc,
12151215
ImportedOperatorsMap<OP_DECL> importedOperators;
12161216
for (auto &imported : SourceFile::Impl::getImportsForSourceFile(SF)) {
12171217
// Protect against source files that contrive to import their own modules.
1218-
if (imported.module.second == ownModule)
1218+
if (imported.module.importedModule == ownModule)
12191219
continue;
12201220

12211221
bool isExported =
12221222
imported.importOptions.contains(SourceFile::ImportFlags::Exported);
12231223
if (!includePrivate && !isExported)
12241224
continue;
12251225

1226-
Optional<OP_DECL *> maybeOp =
1227-
lookupOperatorDeclForName<OP_DECL>(imported.module.second, Loc, Name,
1228-
isCascading);
1226+
Optional<OP_DECL *> maybeOp = lookupOperatorDeclForName<OP_DECL>(
1227+
imported.module.importedModule, Loc, Name, isCascading);
12291228
if (!maybeOp)
12301229
return None;
12311230

@@ -1401,7 +1400,7 @@ SourceFile::getImportedModules(SmallVectorImpl<ModuleDecl::ImportedModule> &modu
14011400
else
14021401
requiredFilter |= ModuleDecl::ImportFilterKind::Private;
14031402

1404-
if (!separatelyImportedOverlays.lookup(desc.module.second).empty())
1403+
if (!separatelyImportedOverlays.lookup(desc.module.importedModule).empty())
14051404
requiredFilter |= ModuleDecl::ImportFilterKind::ShadowedBySeparateOverlay;
14061405

14071406
if (filter.contains(requiredFilter))
@@ -1496,26 +1495,26 @@ ModuleDecl::removeDuplicateImports(SmallVectorImpl<ImportedModule> &imports) {
14961495
std::sort(imports.begin(), imports.end(),
14971496
[](const ImportedModule &lhs, const ImportedModule &rhs) -> bool {
14981497
// Arbitrarily sort by name to get a deterministic order.
1499-
if (lhs.second != rhs.second) {
1498+
if (lhs.importedModule != rhs.importedModule) {
15001499
return std::lexicographical_compare(
1501-
lhs.second->getReverseFullModuleName(), {},
1502-
rhs.second->getReverseFullModuleName(), {});
1500+
lhs.importedModule->getReverseFullModuleName(), {},
1501+
rhs.importedModule->getReverseFullModuleName(), {});
15031502
}
15041503
using AccessPathElem = Located<Identifier>;
1505-
return std::lexicographical_compare(lhs.first.begin(), lhs.first.end(),
1506-
rhs.first.begin(), rhs.first.end(),
1507-
[](const AccessPathElem &lElem,
1508-
const AccessPathElem &rElem) {
1509-
return lElem.Item.str() < rElem.Item.str();
1510-
});
1511-
});
1512-
auto last = std::unique(imports.begin(), imports.end(),
1513-
[](const ImportedModule &lhs,
1514-
const ImportedModule &rhs) -> bool {
1515-
if (lhs.second != rhs.second)
1516-
return false;
1517-
return ModuleDecl::isSameAccessPath(lhs.first, rhs.first);
1504+
return std::lexicographical_compare(
1505+
lhs.accessPath.begin(), lhs.accessPath.end(), rhs.accessPath.begin(),
1506+
rhs.accessPath.end(),
1507+
[](const AccessPathElem &lElem, const AccessPathElem &rElem) {
1508+
return lElem.Item.str() < rElem.Item.str();
1509+
});
15181510
});
1511+
auto last = std::unique(
1512+
imports.begin(), imports.end(),
1513+
[](const ImportedModule &lhs, const ImportedModule &rhs) -> bool {
1514+
if (lhs.importedModule != rhs.importedModule)
1515+
return false;
1516+
return ModuleDecl::isSameAccessPath(lhs.accessPath, rhs.accessPath);
1517+
});
15191518
imports.erase(last, imports.end());
15201519
}
15211520

@@ -1676,11 +1675,12 @@ SourceFile::collectLinkLibraries(ModuleDecl::LinkLibraryCallback callback) const
16761675
topLevel->getImportedModules(stack, topLevelFilter);
16771676

16781677
// Make sure the top-level module is first; we want pre-order-ish traversal.
1679-
stack.emplace_back(ModuleDecl::AccessPathTy(),
1680-
const_cast<ModuleDecl *>(topLevel));
1678+
auto topLevelModule =
1679+
ModuleDecl::ImportedModule{ModuleDecl::AccessPathTy(), topLevel};
1680+
stack.emplace_back(topLevelModule);
16811681

16821682
while (!stack.empty()) {
1683-
auto next = stack.pop_back_val().second;
1683+
auto next = stack.pop_back_val().importedModule;
16841684

16851685
if (!visited.insert(next).second)
16861686
continue;
@@ -1888,7 +1888,7 @@ ModuleDecl::getDeclaringModuleAndBystander() {
18881888

18891889
getImportedModules(imported, ModuleDecl::ImportFilterKind::Public);
18901890
while (!imported.empty()) {
1891-
ModuleDecl *importedModule = std::get<1>(imported.back());
1891+
ModuleDecl *importedModule = imported.back().importedModule;
18921892
imported.pop_back();
18931893
if (!seen.insert(importedModule).second)
18941894
continue;
@@ -2107,15 +2107,15 @@ bool SourceFile::hasTestableOrPrivateImport(
21072107
Imports->begin(), Imports->end(),
21082108
[module, queryKind](ImportedModuleDesc desc) -> bool {
21092109
if (queryKind == ImportQueryKind::TestableAndPrivate)
2110-
return desc.module.second == module &&
2110+
return desc.module.importedModule == module &&
21112111
(desc.importOptions.contains(ImportFlags::PrivateImport) ||
21122112
desc.importOptions.contains(ImportFlags::Testable));
21132113
else if (queryKind == ImportQueryKind::TestableOnly)
2114-
return desc.module.second == module &&
2114+
return desc.module.importedModule == module &&
21152115
desc.importOptions.contains(ImportFlags::Testable);
21162116
else {
21172117
assert(queryKind == ImportQueryKind::PrivateOnly);
2118-
return desc.module.second == module &&
2118+
return desc.module.importedModule == module &&
21192119
desc.importOptions.contains(ImportFlags::PrivateImport);
21202120
}
21212121
});
@@ -2148,7 +2148,7 @@ bool SourceFile::hasTestableOrPrivateImport(
21482148

21492149
return std::any_of(Imports->begin(), Imports->end(),
21502150
[module, filename](ImportedModuleDesc desc) -> bool {
2151-
return desc.module.second == module &&
2151+
return desc.module.importedModule == module &&
21522152
desc.importOptions.contains(
21532153
ImportFlags::PrivateImport) &&
21542154
desc.filename == filename;
@@ -2171,7 +2171,7 @@ bool SourceFile::isImportedImplementationOnly(const ModuleDecl *module) const {
21712171

21722172
// If the module is imported this way, it's not imported
21732173
// implementation-only.
2174-
if (imports.isImportedBy(module, desc.module.second))
2174+
if (imports.isImportedBy(module, desc.module.importedModule))
21752175
return false;
21762176
}
21772177

@@ -2183,7 +2183,7 @@ void SourceFile::lookupImportedSPIGroups(const ModuleDecl *importedModule,
21832183
SmallVectorImpl<Identifier> &spiGroups) const {
21842184
for (auto &import : *Imports) {
21852185
if (import.importOptions.contains(ImportFlags::SPIAccessControl) &&
2186-
importedModule == std::get<ModuleDecl*>(import.module)) {
2186+
importedModule == import.module.importedModule) {
21872187
auto importedSpis = import.spiGroups;
21882188
spiGroups.append(importedSpis.begin(), importedSpis.end());
21892189
}

‎lib/AST/ModuleNameLookup.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,14 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(
184184

185185
auto visitImport = [&](ModuleDecl::ImportedModule import,
186186
const DeclContext *moduleScopeContext) {
187-
if (import.first.empty())
188-
import.first = accessPath;
187+
if (import.accessPath.empty())
188+
import.accessPath = accessPath;
189189
else if (!accessPath.empty() &&
190-
!ModuleDecl::isSameAccessPath(import.first, accessPath))
190+
!ModuleDecl::isSameAccessPath(import.accessPath, accessPath))
191191
return;
192192

193-
getDerived()->doLocalLookup(import.second, import.first, decls);
193+
getDerived()->doLocalLookup(import.importedModule, import.accessPath,
194+
decls);
194195
updateNewDecls(moduleScopeContext);
195196
};
196197

@@ -201,7 +202,8 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(
201202
if (auto *loader = ctx.getClangModuleLoader()) {
202203
headerImportModule = loader->getImportedHeaderModule();
203204
if (headerImportModule) {
204-
ModuleDecl::ImportedModule import({}, headerImportModule);
205+
ModuleDecl::ImportedModule import{ModuleDecl::AccessPathTy(),
206+
headerImportModule};
205207
visitImport(import, nullptr);
206208
}
207209
}
@@ -210,19 +212,19 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(
210212
for (auto import : imports.getTopLevelImports()) {
211213
// A module appears in its own top-level import list; since we checked
212214
// it already, skip it.
213-
if (import.second == module)
215+
if (import.importedModule == module)
214216
continue;
215217

216218
// Skip the special import set module; we've already visited it.
217-
if (import.second == headerImportModule)
219+
if (import.importedModule == headerImportModule)
218220
continue;
219221

220222
visitImport(import, moduleScopeContext);
221223
}
222224

223225
for (auto import : imports.getTransitiveImports()) {
224226
// Skip the special import set module; we've already visited it.
225-
if (import.second == headerImportModule)
227+
if (import.importedModule == headerImportModule)
226228
continue;
227229

228230
visitImport(import, nullptr);

0 commit comments

Comments
 (0)