Merge pull request #33986 from varungandhi-apple/vg-import-filter-cle…

…anup

[NFC] Clarify import filtering logic and naming.

include/swift/AST/FileUnit.h

@@ -241,7 +241,7 @@ class FileUnit : public DeclContext {
   /// \see ModuleDecl::getImportedModulesForLookup
   virtual void getImportedModulesForLookup(
       SmallVectorImpl<ModuleDecl::ImportedModule> &imports) const {
-    return getImportedModules(imports, ModuleDecl::ImportFilterKind::Public);
+    return getImportedModules(imports, ModuleDecl::ImportFilterKind::Exported);
   }
 
   /// Generates the list of libraries needed to link this file, based on its

include/swift/AST/Module.h

@@ -648,27 +648,52 @@ class ModuleDecl : public DeclContext, public TypeDecl {
   /// \sa getImportedModules
   enum class ImportFilterKind {
     /// Include imports declared with `@_exported`.
-    Public = 1 << 0,
+    Exported = 1 << 0,
     /// Include "regular" imports with no special annotation.
-    Private = 1 << 1,
+    Default = 1 << 1,
     /// Include imports declared with `@_implementationOnly`.
     ImplementationOnly = 1 << 2,
-    /// Include imports of SPIs declared with `@_spi`
+    /// Include imports of SPIs declared with `@_spi`. Non-SPI imports are
+    /// included whether or not this flag is specified.
     SPIAccessControl = 1 << 3,
-    /// Include imports shadowed by a separately-imported overlay (i.e. a
-    /// cross-import overlay). Unshadowed imports are included whether or not
-    /// this flag is specified.
-    ShadowedBySeparateOverlay = 1 << 4
+    /// Include imports shadowed by a cross-import overlay. Unshadowed imports
+    /// are included whether or not this flag is specified.
+    ShadowedByCrossImportOverlay = 1 << 4
   };
   /// \sa getImportedModules
   using ImportFilter = OptionSet<ImportFilterKind>;
 
   /// Looks up which modules are imported by this module.
   ///
-  /// \p filter controls whether public, private, or any imports are included
-  /// in this list.
+  /// \p filter controls which imports are included in the list.
+  ///
+  /// There are three axes for categorizing imports:
+  /// 1. Privacy: Exported/Private/ImplementationOnly (mutually exclusive).
+  /// 2. SPI/non-SPI: An import of any privacy level may be @_spi("SPIName").
+  /// 3. Shadowed/Non-shadowed: An import of any privacy level may be shadowed
+  ///    by a cross-import overlay.
+  ///
+  /// It is also possible for SPI imports to be shadowed by a cross-import
+  /// overlay.
+  ///
+  /// If \p filter contains multiple privacy levels, modules at all the privacy
+  /// levels are included.
+  ///
+  /// If \p filter contains \c ImportFilterKind::SPIAccessControl, then both
+  /// SPI and non-SPI imports are included. Otherwise, only non-SPI imports are
+  /// included.
+  ///
+  /// If \p filter contains \c ImportFilterKind::ShadowedByCrossImportOverlay,
+  /// both shadowed and non-shadowed imports are included. Otherwise, only
+  /// non-shadowed imports are included.
+  ///
+  /// Clang modules have some additional complexities; see the implementation of
+  /// \c ClangModuleUnit::getImportedModules for details.
+  ///
+  /// \pre \p filter must contain at least one privacy level, i.e. one of
+  ///      \c Exported or \c Private or \c ImplementationOnly.
   void getImportedModules(SmallVectorImpl<ImportedModule> &imports,
-                          ImportFilter filter = ImportFilterKind::Public) const;
+                          ImportFilter filter = ImportFilterKind::Exported) const;
 
   /// Looks up which modules are imported by this module, ignoring any that
   /// won't contain top-level decls.

include/swift/Basic/OptionSet.h

@@ -19,6 +19,7 @@
 
 #include "llvm/ADT/None.h"
 
+#include <cassert>
 #include <type_traits>
 #include <cstdint>
 #include <initializer_list>
@@ -98,6 +99,14 @@ class OptionSet {
     return Storage == set.Storage;
   }
 
+  /// Check if this option set contains any options from \p set.
+  ///
+  /// \pre \p set must be non-empty.
+  bool containsAny(OptionSet set) const {
+    assert((bool)set && "argument must be non-empty");
+    return (bool)((*this) & set);
+  }
+
   // '==' and '!=' are deliberately not defined because they provide a pitfall
   // where someone might use '==' but really want 'contains'. If you actually
   // want '==' behavior, use 'containsOnly'.

lib/AST/ImportCache.cpp

@@ -175,8 +175,9 @@ ImportSet &ImportCache::getImportSet(const DeclContext *dc) {
       ModuleDecl::ImportedModule{ImportPath::Access(), mod});
 
   if (file) {
+    // Should include both SPI & non-SPI.
     file->getImportedModules(imports,
-                             {ModuleDecl::ImportFilterKind::Private,
+                             {ModuleDecl::ImportFilterKind::Default,
                               ModuleDecl::ImportFilterKind::ImplementationOnly,
                               ModuleDecl::ImportFilterKind::SPIAccessControl});
   }
@@ -260,8 +261,9 @@ ImportCache::getAllAccessPathsNotShadowedBy(const ModuleDecl *mod,
       ModuleDecl::ImportedModule{ImportPath::Access(), currentMod});
 
   if (auto *file = dyn_cast<FileUnit>(dc)) {
+    // Should include both SPI & non-SPI
     file->getImportedModules(stack,
-                             {ModuleDecl::ImportFilterKind::Private,
+                             {ModuleDecl::ImportFilterKind::Default,
                               ModuleDecl::ImportFilterKind::ImplementationOnly,
                               ModuleDecl::ImportFilterKind::SPIAccessControl});
   }

lib/AST/Module.cpp

@@ -1163,6 +1163,13 @@ void SourceFile::lookupPrecedenceGroupDirect(
 
 void ModuleDecl::getImportedModules(SmallVectorImpl<ImportedModule> &modules,
                                     ModuleDecl::ImportFilter filter) const {
+  assert(filter.containsAny(ImportFilter({
+      ModuleDecl::ImportFilterKind::Exported,
+      ModuleDecl::ImportFilterKind::Default,
+      ModuleDecl::ImportFilterKind::ImplementationOnly}))
+    && "filter should have at least one of Exported|Private|ImplementationOnly"
+  );
+
   FORWARD(getImportedModules, (modules, filter));
 }
 
@@ -1184,16 +1191,17 @@ SourceFile::getImportedModules(SmallVectorImpl<ModuleDecl::ImportedModule> &modu
   for (auto desc : *Imports) {
     ModuleDecl::ImportFilter requiredFilter;
     if (desc.importOptions.contains(ImportFlags::Exported))
-      requiredFilter |= ModuleDecl::ImportFilterKind::Public;
+      requiredFilter |= ModuleDecl::ImportFilterKind::Exported;
     else if (desc.importOptions.contains(ImportFlags::ImplementationOnly))
       requiredFilter |= ModuleDecl::ImportFilterKind::ImplementationOnly;
-    else if (desc.importOptions.contains(ImportFlags::SPIAccessControl))
-      requiredFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
     else
-      requiredFilter |= ModuleDecl::ImportFilterKind::Private;
+      requiredFilter |= ModuleDecl::ImportFilterKind::Default;
+
+    if (desc.importOptions.contains(ImportFlags::SPIAccessControl))
+      requiredFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
 
     if (!separatelyImportedOverlays.lookup(desc.module.importedModule).empty())
-      requiredFilter |= ModuleDecl::ImportFilterKind::ShadowedBySeparateOverlay;
+      requiredFilter |= ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay;
 
     if (filter.contains(requiredFilter))
       modules.push_back(desc.module);
@@ -1445,8 +1453,8 @@ SourceFile::collectLinkLibraries(ModuleDecl::LinkLibraryCallback callback) const
   SmallVector<ModuleDecl::ImportedModule, 32> stack;
 
   ModuleDecl::ImportFilter filter = {
-      ModuleDecl::ImportFilterKind::Public,
-      ModuleDecl::ImportFilterKind::Private,
+      ModuleDecl::ImportFilterKind::Exported,
+      ModuleDecl::ImportFilterKind::Default,
       ModuleDecl::ImportFilterKind::SPIAccessControl};
 
   auto *topLevel = getParentModule();
@@ -1680,7 +1688,7 @@ ModuleDecl::getDeclaringModuleAndBystander() {
   SmallVector<ModuleDecl::ImportedModule, 16> furtherImported;
   ModuleDecl *overlayModule = this;
 
-  getImportedModules(imported, ModuleDecl::ImportFilterKind::Public);
+  getImportedModules(imported, ModuleDecl::ImportFilterKind::Exported);
   while (!imported.empty()) {
     ModuleDecl *importedModule = imported.back().importedModule;
     imported.pop_back();
@@ -1706,7 +1714,7 @@ ModuleDecl::getDeclaringModuleAndBystander() {
 
     furtherImported.clear();
     importedModule->getImportedModules(furtherImported,
-                                       ModuleDecl::ImportFilterKind::Public);
+                                       ModuleDecl::ImportFilterKind::Exported);
     imported.append(furtherImported.begin(), furtherImported.end());
   }
 
@@ -1988,10 +1996,10 @@ bool ModuleDecl::isImportedImplementationOnly(const ModuleDecl *module) const {
   // Look through non-implementation-only imports to see if module is imported
   // in some other way. Otherwise we assume it's implementation-only imported.
   ModuleDecl::ImportFilter filter = {
-    ModuleDecl::ImportFilterKind::Public,
-    ModuleDecl::ImportFilterKind::Private,
+    ModuleDecl::ImportFilterKind::Exported,
+    ModuleDecl::ImportFilterKind::Default,
     ModuleDecl::ImportFilterKind::SPIAccessControl,
-    ModuleDecl::ImportFilterKind::ShadowedBySeparateOverlay};
+    ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay};
   SmallVector<ModuleDecl::ImportedModule, 4> results;
   getImportedModules(results, filter);
 

lib/ClangImporter/ClangImporter.cpp

@@ -3443,26 +3443,26 @@ void ClangModuleUnit::getImportedModules(
 
   // [NOTE: Pure-Clang-modules-privately-import-stdlib]:
   // Needed for implicitly synthesized conformances.
-  if (filter.contains(ModuleDecl::ImportFilterKind::Private))
+  if (filter.contains(ModuleDecl::ImportFilterKind::Default))
     if (auto stdlib = owner.getStdlibModule())
       imports.push_back({ImportPath::Access(), stdlib});
 
   SmallVector<clang::Module *, 8> imported;
   if (!clangModule) {
     // This is the special "imported headers" module.
-    if (filter.contains(ModuleDecl::ImportFilterKind::Public)) {
+    if (filter.contains(ModuleDecl::ImportFilterKind::Exported)) {
       imported.append(owner.ImportedHeaderExports.begin(),
                       owner.ImportedHeaderExports.end());
     }
 
   } else {
     clangModule->getExportedModules(imported);
 
-    if (filter.contains(ModuleDecl::ImportFilterKind::Private)) {
+    if (filter.contains(ModuleDecl::ImportFilterKind::Default)) {
       // Copy in any modules that are imported but not exported.
       llvm::SmallPtrSet<clang::Module *, 8> knownModules(imported.begin(),
                                                          imported.end());
-      if (!filter.contains(ModuleDecl::ImportFilterKind::Public)) {
+      if (!filter.contains(ModuleDecl::ImportFilterKind::Exported)) {
         // Remove the exported ones now that we're done with them.
         imported.clear();
       }

lib/Frontend/ModuleInterfaceSupport.cpp

@@ -101,8 +101,8 @@ static void printImports(raw_ostream &out,
   // FIXME: This is very similar to what's in Serializer::writeInputBlock, but
   // it's not obvious what higher-level optimization would be factored out here.
   ModuleDecl::ImportFilter allImportFilter = {
-      ModuleDecl::ImportFilterKind::Public,
-      ModuleDecl::ImportFilterKind::Private,
+      ModuleDecl::ImportFilterKind::Exported,
+      ModuleDecl::ImportFilterKind::Default,
       ModuleDecl::ImportFilterKind::SPIAccessControl};
 
   // With -experimental-spi-imports:
@@ -116,7 +116,8 @@ static void printImports(raw_ostream &out,
 
     SmallVector<ModuleDecl::ImportedModule, 4> ioiImport;
     M->getImportedModules(ioiImport,
-                          ModuleDecl::ImportFilterKind::ImplementationOnly);
+                          {ModuleDecl::ImportFilterKind::ImplementationOnly,
+                           ModuleDecl::ImportFilterKind::SPIAccessControl});
     ioiImportSet.insert(ioiImport.begin(), ioiImport.end());
   }
 
@@ -128,7 +129,7 @@ static void printImports(raw_ostream &out,
   // Collect the public imports as a subset so that we can mark them with
   // '@_exported'.
   SmallVector<ModuleDecl::ImportedModule, 8> publicImports;
-  M->getImportedModules(publicImports, ModuleDecl::ImportFilterKind::Public);
+  M->getImportedModules(publicImports, ModuleDecl::ImportFilterKind::Exported);
   llvm::SmallSet<ModuleDecl::ImportedModule, 8,
                  ModuleDecl::OrderImportedModules> publicImportSet;
 

lib/FrontendTool/FrontendTool.cpp

@@ -305,11 +305,11 @@ static void getImmediateImports(
     ModuleDecl *module,
     SmallPtrSetImpl<ModuleDecl *> &imports,
     ModuleDecl::ImportFilter importFilter = {
-      ModuleDecl::ImportFilterKind::Public,
-      ModuleDecl::ImportFilterKind::Private,
+      ModuleDecl::ImportFilterKind::Exported,
+      ModuleDecl::ImportFilterKind::Default,
       ModuleDecl::ImportFilterKind::ImplementationOnly,
       ModuleDecl::ImportFilterKind::SPIAccessControl,
-      ModuleDecl::ImportFilterKind::ShadowedBySeparateOverlay
+      ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay
     }) {
   SmallVector<ModuleDecl::ImportedModule, 8> importList;
   module->getImportedModules(importList, importFilter);
@@ -507,7 +507,7 @@ bool ABIDependencyEvaluator::isOverlayOfClangModule(ModuleDecl *swiftModule) {
 
   llvm::SmallPtrSet<ModuleDecl *, 8> importList;
   ::getImmediateImports(swiftModule, importList,
-                        {ModuleDecl::ImportFilterKind::Public});
+                        {ModuleDecl::ImportFilterKind::Exported});
   bool isOverlay =
       llvm::any_of(importList, [&](ModuleDecl *importedModule) -> bool {
         return isClangOverlayOf(swiftModule, importedModule);
@@ -642,7 +642,7 @@ void ABIDependencyEvaluator::computeABIDependenciesForSwiftModule(
 
   SmallPtrSet<ModuleDecl *, 32> reexportedImports;
   ::getImmediateImports(module, reexportedImports,
-                        {ModuleDecl::ImportFilterKind::Public});
+                        {ModuleDecl::ImportFilterKind::Exported});
   for (auto reexportedImport: reexportedImports) {
     reexposeImportedABI(module, reexportedImport);
   }

lib/FrontendTool/ImportedModules.cpp

@@ -80,8 +80,8 @@ bool swift::emitImportedModules(ModuleDecl *mainModule,
     if (!clangImporter->importBridgingHeader(implicitHeaderPath, mainModule)) {
       SmallVector<ModuleDecl::ImportedModule, 16> imported;
       clangImporter->getImportedHeaderModule()->getImportedModules(
-          imported, {ModuleDecl::ImportFilterKind::Public,
-                     ModuleDecl::ImportFilterKind::Private,
+          imported, {ModuleDecl::ImportFilterKind::Exported,
+                     ModuleDecl::ImportFilterKind::Default,
                      ModuleDecl::ImportFilterKind::ImplementationOnly,
                      ModuleDecl::ImportFilterKind::SPIAccessControl});
 

lib/IDE/CodeCompletion.cpp

@@ -2040,8 +2040,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
     SmallVector<ModuleDecl::ImportedModule, 16> FurtherImported;
     CurrDeclContext->getParentSourceFile()->getImportedModules(
         Imported,
-        {ModuleDecl::ImportFilterKind::Public,
-         ModuleDecl::ImportFilterKind::Private,
+        {ModuleDecl::ImportFilterKind::Exported,
+         ModuleDecl::ImportFilterKind::Default,
          ModuleDecl::ImportFilterKind::ImplementationOnly});
     while (!Imported.empty()) {
       ModuleDecl *MD = Imported.back().importedModule;
@@ -2050,7 +2050,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
         continue;
       FurtherImported.clear();
       MD->getImportedModules(FurtherImported,
-                             ModuleDecl::ImportFilterKind::Public);
+                             ModuleDecl::ImportFilterKind::Exported);
       Imported.append(FurtherImported.begin(), FurtherImported.end());
     }
   }
@@ -5993,8 +5993,8 @@ static void deliverCompletionResults(CodeCompletionContext &CompletionContext,
       // Add results for all imported modules.
       SmallVector<ModuleDecl::ImportedModule, 4> Imports;
       SF.getImportedModules(
-          Imports, {ModuleDecl::ImportFilterKind::Public,
-                    ModuleDecl::ImportFilterKind::Private,
+          Imports, {ModuleDecl::ImportFilterKind::Exported,
+                    ModuleDecl::ImportFilterKind::Default,
                     ModuleDecl::ImportFilterKind::ImplementationOnly});
 
       for (auto Imported : Imports) {

lib/IDE/REPLCodeCompletion.cpp

@@ -227,7 +227,7 @@ doCodeCompletion(SourceFile &SF, StringRef EnteredCode, unsigned *BufferID,
   // Carry over the private imports from the last module.
   SmallVector<ModuleDecl::ImportedModule, 8> imports;
   lastModule->getImportedModules(imports,
-                                 ModuleDecl::ImportFilterKind::Private);
+                                 ModuleDecl::ImportFilterKind::Default);
   for (auto &import : imports) {
     implicitImports.AdditionalModules.emplace_back(import.importedModule,
                                                    /*exported*/ false);

lib/IRGen/IRGenDebugInfo.cpp

@@ -1866,8 +1866,8 @@ void IRGenDebugInfoImpl::finalize() {
   // from all ImportDecls).
   SmallVector<ModuleDecl::ImportedModule, 8> ModuleWideImports;
   IGM.getSwiftModule()->getImportedModules(
-      ModuleWideImports, {ModuleDecl::ImportFilterKind::Public,
-                          ModuleDecl::ImportFilterKind::Private,
+      ModuleWideImports, {ModuleDecl::ImportFilterKind::Exported,
+                          ModuleDecl::ImportFilterKind::Default,
                           ModuleDecl::ImportFilterKind::ImplementationOnly});
   for (auto M : ModuleWideImports)
     if (!ImportedModules.count(M.importedModule))

lib/Index/Index.cpp

@@ -126,8 +126,8 @@ class SourceFileOrModule {
   void
   getImportedModules(SmallVectorImpl<ModuleDecl::ImportedModule> &Modules) const {
     constexpr ModuleDecl::ImportFilter ImportFilter = {
-        ModuleDecl::ImportFilterKind::Public,
-        ModuleDecl::ImportFilterKind::Private,
+        ModuleDecl::ImportFilterKind::Exported,
+        ModuleDecl::ImportFilterKind::Default,
         ModuleDecl::ImportFilterKind::ImplementationOnly};
 
     if (auto *SF = SFOrMod.dyn_cast<SourceFile *>()) {
@@ -1599,9 +1599,9 @@ void IndexSwiftASTWalker::collectRecursiveModuleImports(
   }
 
   ModuleDecl::ImportFilter ImportFilter;
-  ImportFilter |= ModuleDecl::ImportFilterKind::Public;
-  ImportFilter |= ModuleDecl::ImportFilterKind::Private;
-  // FIXME: ImportFilterKind::ShadowedBySeparateOverlay?
+  ImportFilter |= ModuleDecl::ImportFilterKind::Exported;
+  ImportFilter |= ModuleDecl::ImportFilterKind::Default;
+  // FIXME: ImportFilterKind::ShadowedByCrossImportOverlay?
   SmallVector<ModuleDecl::ImportedModule, 8> Imports;
   TopMod.getImportedModules(Imports);
 

lib/Index/IndexRecord.cpp

@@ -581,8 +581,8 @@ emitDataForSwiftSerializedModule(ModuleDecl *module,
   }
 
   SmallVector<ModuleDecl::ImportedModule, 8> imports;
-  module->getImportedModules(imports, {ModuleDecl::ImportFilterKind::Public,
-                                       ModuleDecl::ImportFilterKind::Private});
+  module->getImportedModules(imports, {ModuleDecl::ImportFilterKind::Exported,
+                                       ModuleDecl::ImportFilterKind::Default});
   StringScratchSpace moduleNameScratch;
   addModuleDependencies(imports, indexStorePath, indexSystemModules, skipStdlib,
                         targetTriple, clangCI, diags, unitWriter,
@@ -621,8 +621,8 @@ recordSourceFileUnit(SourceFile *primarySourceFile, StringRef indexUnitToken,
   // Module dependencies.
   SmallVector<ModuleDecl::ImportedModule, 8> imports;
   primarySourceFile->getImportedModules(
-      imports, {ModuleDecl::ImportFilterKind::Public,
-                ModuleDecl::ImportFilterKind::Private,
+      imports, {ModuleDecl::ImportFilterKind::Exported,
+                ModuleDecl::ImportFilterKind::Default,
                 ModuleDecl::ImportFilterKind::ImplementationOnly});
   StringScratchSpace moduleNameScratch;
   addModuleDependencies(imports, indexStorePath, indexSystemModules, skipStdlib,