Skip to content

Commit 734b8a2

Browse files
varungandhi-applevarungandhi-apple
authored
Merge pull request swiftlang#33986 from varungandhi-apple/vg-import-filter-cleanup
[NFC] Clarify import filtering logic and naming.
2 parents d0805ef + 4b5d885 commit 734b8a2

23 files changed

+127
-78
lines changed

‎include/swift/AST/FileUnit.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ class FileUnit : public DeclContext {
241241
/// \see ModuleDecl::getImportedModulesForLookup
242242
virtual void getImportedModulesForLookup(
243243
SmallVectorImpl<ModuleDecl::ImportedModule> &imports) const {
244-
return getImportedModules(imports, ModuleDecl::ImportFilterKind::Public);
244+
return getImportedModules(imports, ModuleDecl::ImportFilterKind::Exported);
245245
}
246246

247247
/// Generates the list of libraries needed to link this file, based on its

‎include/swift/AST/Module.h‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -648,27 +648,52 @@ class ModuleDecl : public DeclContext, public TypeDecl {
648648
/// \sa getImportedModules
649649
enum class ImportFilterKind {
650650
/// Include imports declared with `@_exported`.
651-
Public = 1 << 0,
651+
Exported = 1 << 0,
652652
/// Include "regular" imports with no special annotation.
653-
Private = 1 << 1,
653+
Default = 1 << 1,
654654
/// Include imports declared with `@_implementationOnly`.
655655
ImplementationOnly = 1 << 2,
656-
/// Include imports of SPIs declared with `@_spi`
656+
/// Include imports of SPIs declared with `@_spi`. Non-SPI imports are
657+
/// included whether or not this flag is specified.
657658
SPIAccessControl = 1 << 3,
658-
/// Include imports shadowed by a separately-imported overlay (i.e. a
659-
/// cross-import overlay). Unshadowed imports are included whether or not
660-
/// this flag is specified.
661-
ShadowedBySeparateOverlay = 1 << 4
659+
/// Include imports shadowed by a cross-import overlay. Unshadowed imports
660+
/// are included whether or not this flag is specified.
661+
ShadowedByCrossImportOverlay = 1 << 4
662662
};
663663
/// \sa getImportedModules
664664
using ImportFilter = OptionSet<ImportFilterKind>;
665665

666666
/// Looks up which modules are imported by this module.
667667
///
668-
/// \p filter controls whether public, private, or any imports are included
669-
/// in this list.
668+
/// \p filter controls which imports are included in the list.
669+
///
670+
/// There are three axes for categorizing imports:
671+
/// 1. Privacy: Exported/Private/ImplementationOnly (mutually exclusive).
672+
/// 2. SPI/non-SPI: An import of any privacy level may be @_spi("SPIName").
673+
/// 3. Shadowed/Non-shadowed: An import of any privacy level may be shadowed
674+
/// by a cross-import overlay.
675+
///
676+
/// It is also possible for SPI imports to be shadowed by a cross-import
677+
/// overlay.
678+
///
679+
/// If \p filter contains multiple privacy levels, modules at all the privacy
680+
/// levels are included.
681+
///
682+
/// If \p filter contains \c ImportFilterKind::SPIAccessControl, then both
683+
/// SPI and non-SPI imports are included. Otherwise, only non-SPI imports are
684+
/// included.
685+
///
686+
/// If \p filter contains \c ImportFilterKind::ShadowedByCrossImportOverlay,
687+
/// both shadowed and non-shadowed imports are included. Otherwise, only
688+
/// non-shadowed imports are included.
689+
///
690+
/// Clang modules have some additional complexities; see the implementation of
691+
/// \c ClangModuleUnit::getImportedModules for details.
692+
///
693+
/// \pre \p filter must contain at least one privacy level, i.e. one of
694+
/// \c Exported or \c Private or \c ImplementationOnly.
670695
void getImportedModules(SmallVectorImpl<ImportedModule> &imports,
671-
ImportFilter filter = ImportFilterKind::Public) const;
696+
ImportFilter filter = ImportFilterKind::Exported) const;
672697

673698
/// Looks up which modules are imported by this module, ignoring any that
674699
/// won't contain top-level decls.

‎include/swift/Basic/OptionSet.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "llvm/ADT/None.h"
2121

22+
#include <cassert>
2223
#include <type_traits>
2324
#include <cstdint>
2425
#include <initializer_list>
@@ -98,6 +99,14 @@ class OptionSet {
9899
return Storage == set.Storage;
99100
}
100101

102+
/// Check if this option set contains any options from \p set.
103+
///
104+
/// \pre \p set must be non-empty.
105+
bool containsAny(OptionSet set) const {
106+
assert((bool)set && "argument must be non-empty");
107+
return (bool)((*this) & set);
108+
}
109+
101110
// '==' and '!=' are deliberately not defined because they provide a pitfall
102111
// where someone might use '==' but really want 'contains'. If you actually
103112
// want '==' behavior, use 'containsOnly'.

‎lib/AST/ImportCache.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,9 @@ ImportSet &ImportCache::getImportSet(const DeclContext *dc) {
175175
ModuleDecl::ImportedModule{ImportPath::Access(), mod});
176176

177177
if (file) {
178+
// Should include both SPI & non-SPI.
178179
file->getImportedModules(imports,
179-
{ModuleDecl::ImportFilterKind::Private,
180+
{ModuleDecl::ImportFilterKind::Default,
180181
ModuleDecl::ImportFilterKind::ImplementationOnly,
181182
ModuleDecl::ImportFilterKind::SPIAccessControl});
182183
}
@@ -260,8 +261,9 @@ ImportCache::getAllAccessPathsNotShadowedBy(const ModuleDecl *mod,
260261
ModuleDecl::ImportedModule{ImportPath::Access(), currentMod});
261262

262263
if (auto *file = dyn_cast<FileUnit>(dc)) {
264+
// Should include both SPI & non-SPI
263265
file->getImportedModules(stack,
264-
{ModuleDecl::ImportFilterKind::Private,
266+
{ModuleDecl::ImportFilterKind::Default,
265267
ModuleDecl::ImportFilterKind::ImplementationOnly,
266268
ModuleDecl::ImportFilterKind::SPIAccessControl});
267269
}

‎lib/AST/Module.cpp‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,6 +1163,13 @@ void SourceFile::lookupPrecedenceGroupDirect(
11631163

11641164
void ModuleDecl::getImportedModules(SmallVectorImpl<ImportedModule> &modules,
11651165
ModuleDecl::ImportFilter filter) const {
1166+
assert(filter.containsAny(ImportFilter({
1167+
ModuleDecl::ImportFilterKind::Exported,
1168+
ModuleDecl::ImportFilterKind::Default,
1169+
ModuleDecl::ImportFilterKind::ImplementationOnly}))
1170+
&& "filter should have at least one of Exported|Private|ImplementationOnly"
1171+
);
1172+
11661173
FORWARD(getImportedModules, (modules, filter));
11671174
}
11681175

@@ -1184,16 +1191,17 @@ SourceFile::getImportedModules(SmallVectorImpl<ModuleDecl::ImportedModule> &modu
11841191
for (auto desc : *Imports) {
11851192
ModuleDecl::ImportFilter requiredFilter;
11861193
if (desc.importOptions.contains(ImportFlags::Exported))
1187-
requiredFilter |= ModuleDecl::ImportFilterKind::Public;
1194+
requiredFilter |= ModuleDecl::ImportFilterKind::Exported;
11881195
else if (desc.importOptions.contains(ImportFlags::ImplementationOnly))
11891196
requiredFilter |= ModuleDecl::ImportFilterKind::ImplementationOnly;
1190-
else if (desc.importOptions.contains(ImportFlags::SPIAccessControl))
1191-
requiredFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
11921197
else
1193-
requiredFilter |= ModuleDecl::ImportFilterKind::Private;
1198+
requiredFilter |= ModuleDecl::ImportFilterKind::Default;
1199+
1200+
if (desc.importOptions.contains(ImportFlags::SPIAccessControl))
1201+
requiredFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
11941202

11951203
if (!separatelyImportedOverlays.lookup(desc.module.importedModule).empty())
1196-
requiredFilter |= ModuleDecl::ImportFilterKind::ShadowedBySeparateOverlay;
1204+
requiredFilter |= ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay;
11971205

11981206
if (filter.contains(requiredFilter))
11991207
modules.push_back(desc.module);
@@ -1445,8 +1453,8 @@ SourceFile::collectLinkLibraries(ModuleDecl::LinkLibraryCallback callback) const
14451453
SmallVector<ModuleDecl::ImportedModule, 32> stack;
14461454

14471455
ModuleDecl::ImportFilter filter = {
1448-
ModuleDecl::ImportFilterKind::Public,
1449-
ModuleDecl::ImportFilterKind::Private,
1456+
ModuleDecl::ImportFilterKind::Exported,
1457+
ModuleDecl::ImportFilterKind::Default,
14501458
ModuleDecl::ImportFilterKind::SPIAccessControl};
14511459

14521460
auto *topLevel = getParentModule();
@@ -1680,7 +1688,7 @@ ModuleDecl::getDeclaringModuleAndBystander() {
16801688
SmallVector<ModuleDecl::ImportedModule, 16> furtherImported;
16811689
ModuleDecl *overlayModule = this;
16821690

1683-
getImportedModules(imported, ModuleDecl::ImportFilterKind::Public);
1691+
getImportedModules(imported, ModuleDecl::ImportFilterKind::Exported);
16841692
while (!imported.empty()) {
16851693
ModuleDecl *importedModule = imported.back().importedModule;
16861694
imported.pop_back();
@@ -1706,7 +1714,7 @@ ModuleDecl::getDeclaringModuleAndBystander() {
17061714

17071715
furtherImported.clear();
17081716
importedModule->getImportedModules(furtherImported,
1709-
ModuleDecl::ImportFilterKind::Public);
1717+
ModuleDecl::ImportFilterKind::Exported);
17101718
imported.append(furtherImported.begin(), furtherImported.end());
17111719
}
17121720

@@ -1988,10 +1996,10 @@ bool ModuleDecl::isImportedImplementationOnly(const ModuleDecl *module) const {
19881996
// Look through non-implementation-only imports to see if module is imported
19891997
// in some other way. Otherwise we assume it's implementation-only imported.
19901998
ModuleDecl::ImportFilter filter = {
1991-
ModuleDecl::ImportFilterKind::Public,
1992-
ModuleDecl::ImportFilterKind::Private,
1999+
ModuleDecl::ImportFilterKind::Exported,
2000+
ModuleDecl::ImportFilterKind::Default,
19932001
ModuleDecl::ImportFilterKind::SPIAccessControl,
1994-
ModuleDecl::ImportFilterKind::ShadowedBySeparateOverlay};
2002+
ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay};
19952003
SmallVector<ModuleDecl::ImportedModule, 4> results;
19962004
getImportedModules(results, filter);
19972005

‎lib/ClangImporter/ClangImporter.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3443,26 +3443,26 @@ void ClangModuleUnit::getImportedModules(
34433443

34443444
// [NOTE: Pure-Clang-modules-privately-import-stdlib]:
34453445
// Needed for implicitly synthesized conformances.
3446-
if (filter.contains(ModuleDecl::ImportFilterKind::Private))
3446+
if (filter.contains(ModuleDecl::ImportFilterKind::Default))
34473447
if (auto stdlib = owner.getStdlibModule())
34483448
imports.push_back({ImportPath::Access(), stdlib});
34493449

34503450
SmallVector<clang::Module *, 8> imported;
34513451
if (!clangModule) {
34523452
// This is the special "imported headers" module.
3453-
if (filter.contains(ModuleDecl::ImportFilterKind::Public)) {
3453+
if (filter.contains(ModuleDecl::ImportFilterKind::Exported)) {
34543454
imported.append(owner.ImportedHeaderExports.begin(),
34553455
owner.ImportedHeaderExports.end());
34563456
}
34573457

34583458
} else {
34593459
clangModule->getExportedModules(imported);
34603460

3461-
if (filter.contains(ModuleDecl::ImportFilterKind::Private)) {
3461+
if (filter.contains(ModuleDecl::ImportFilterKind::Default)) {
34623462
// Copy in any modules that are imported but not exported.
34633463
llvm::SmallPtrSet<clang::Module *, 8> knownModules(imported.begin(),
34643464
imported.end());
3465-
if (!filter.contains(ModuleDecl::ImportFilterKind::Public)) {
3465+
if (!filter.contains(ModuleDecl::ImportFilterKind::Exported)) {
34663466
// Remove the exported ones now that we're done with them.
34673467
imported.clear();
34683468
}

‎lib/Frontend/ModuleInterfaceSupport.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ static void printImports(raw_ostream &out,
101101
// FIXME: This is very similar to what's in Serializer::writeInputBlock, but
102102
// it's not obvious what higher-level optimization would be factored out here.
103103
ModuleDecl::ImportFilter allImportFilter = {
104-
ModuleDecl::ImportFilterKind::Public,
105-
ModuleDecl::ImportFilterKind::Private,
104+
ModuleDecl::ImportFilterKind::Exported,
105+
ModuleDecl::ImportFilterKind::Default,
106106
ModuleDecl::ImportFilterKind::SPIAccessControl};
107107

108108
// With -experimental-spi-imports:
@@ -116,7 +116,8 @@ static void printImports(raw_ostream &out,
116116

117117
SmallVector<ModuleDecl::ImportedModule, 4> ioiImport;
118118
M->getImportedModules(ioiImport,
119-
ModuleDecl::ImportFilterKind::ImplementationOnly);
119+
{ModuleDecl::ImportFilterKind::ImplementationOnly,
120+
ModuleDecl::ImportFilterKind::SPIAccessControl});
120121
ioiImportSet.insert(ioiImport.begin(), ioiImport.end());
121122
}
122123

@@ -128,7 +129,7 @@ static void printImports(raw_ostream &out,
128129
// Collect the public imports as a subset so that we can mark them with
129130
// '@_exported'.
130131
SmallVector<ModuleDecl::ImportedModule, 8> publicImports;
131-
M->getImportedModules(publicImports, ModuleDecl::ImportFilterKind::Public);
132+
M->getImportedModules(publicImports, ModuleDecl::ImportFilterKind::Exported);
132133
llvm::SmallSet<ModuleDecl::ImportedModule, 8,
133134
ModuleDecl::OrderImportedModules> publicImportSet;
134135

‎lib/FrontendTool/FrontendTool.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,11 @@ static void getImmediateImports(
305305
ModuleDecl *module,
306306
SmallPtrSetImpl<ModuleDecl *> &imports,
307307
ModuleDecl::ImportFilter importFilter = {
308-
ModuleDecl::ImportFilterKind::Public,
309-
ModuleDecl::ImportFilterKind::Private,
308+
ModuleDecl::ImportFilterKind::Exported,
309+
ModuleDecl::ImportFilterKind::Default,
310310
ModuleDecl::ImportFilterKind::ImplementationOnly,
311311
ModuleDecl::ImportFilterKind::SPIAccessControl,
312-
ModuleDecl::ImportFilterKind::ShadowedBySeparateOverlay
312+
ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay
313313
}) {
314314
SmallVector<ModuleDecl::ImportedModule, 8> importList;
315315
module->getImportedModules(importList, importFilter);
@@ -507,7 +507,7 @@ bool ABIDependencyEvaluator::isOverlayOfClangModule(ModuleDecl *swiftModule) {
507507

508508
llvm::SmallPtrSet<ModuleDecl *, 8> importList;
509509
::getImmediateImports(swiftModule, importList,
510-
{ModuleDecl::ImportFilterKind::Public});
510+
{ModuleDecl::ImportFilterKind::Exported});
511511
bool isOverlay =
512512
llvm::any_of(importList, [&](ModuleDecl *importedModule) -> bool {
513513
return isClangOverlayOf(swiftModule, importedModule);
@@ -642,7 +642,7 @@ void ABIDependencyEvaluator::computeABIDependenciesForSwiftModule(
642642

643643
SmallPtrSet<ModuleDecl *, 32> reexportedImports;
644644
::getImmediateImports(module, reexportedImports,
645-
{ModuleDecl::ImportFilterKind::Public});
645+
{ModuleDecl::ImportFilterKind::Exported});
646646
for (auto reexportedImport: reexportedImports) {
647647
reexposeImportedABI(module, reexportedImport);
648648
}

‎lib/FrontendTool/ImportedModules.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ bool swift::emitImportedModules(ModuleDecl *mainModule,
8080
if (!clangImporter->importBridgingHeader(implicitHeaderPath, mainModule)) {
8181
SmallVector<ModuleDecl::ImportedModule, 16> imported;
8282
clangImporter->getImportedHeaderModule()->getImportedModules(
83-
imported, {ModuleDecl::ImportFilterKind::Public,
84-
ModuleDecl::ImportFilterKind::Private,
83+
imported, {ModuleDecl::ImportFilterKind::Exported,
84+
ModuleDecl::ImportFilterKind::Default,
8585
ModuleDecl::ImportFilterKind::ImplementationOnly,
8686
ModuleDecl::ImportFilterKind::SPIAccessControl});
8787

‎lib/IDE/CodeCompletion.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2040,8 +2040,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
20402040
SmallVector<ModuleDecl::ImportedModule, 16> FurtherImported;
20412041
CurrDeclContext->getParentSourceFile()->getImportedModules(
20422042
Imported,
2043-
{ModuleDecl::ImportFilterKind::Public,
2044-
ModuleDecl::ImportFilterKind::Private,
2043+
{ModuleDecl::ImportFilterKind::Exported,
2044+
ModuleDecl::ImportFilterKind::Default,
20452045
ModuleDecl::ImportFilterKind::ImplementationOnly});
20462046
while (!Imported.empty()) {
20472047
ModuleDecl *MD = Imported.back().importedModule;
@@ -2050,7 +2050,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
20502050
continue;
20512051
FurtherImported.clear();
20522052
MD->getImportedModules(FurtherImported,
2053-
ModuleDecl::ImportFilterKind::Public);
2053+
ModuleDecl::ImportFilterKind::Exported);
20542054
Imported.append(FurtherImported.begin(), FurtherImported.end());
20552055
}
20562056
}
@@ -5993,8 +5993,8 @@ static void deliverCompletionResults(CodeCompletionContext &CompletionContext,
59935993
// Add results for all imported modules.
59945994
SmallVector<ModuleDecl::ImportedModule, 4> Imports;
59955995
SF.getImportedModules(
5996-
Imports, {ModuleDecl::ImportFilterKind::Public,
5997-
ModuleDecl::ImportFilterKind::Private,
5996+
Imports, {ModuleDecl::ImportFilterKind::Exported,
5997+
ModuleDecl::ImportFilterKind::Default,
59985998
ModuleDecl::ImportFilterKind::ImplementationOnly});
59995999

60006000
for (auto Imported : Imports) {

0 commit comments

Comments
 (0)