Merge pull request swiftlang#34410 from varungandhi-apple/vg-revert-import-filtering-refactor

Revert "[NFC] Clarify semantics of getImportedModules."

Author: varungandhi-apple

include/swift/AST/Module.h

@@ -593,8 +593,7 @@ class ModuleDecl : public DeclContext, public TypeDecl {
     Default = 1 << 1,
     /// Include imports declared with `@_implementationOnly`.
     ImplementationOnly = 1 << 2,
-    /// Include imports of SPIs declared with `@_spi`. Non-SPI imports are
-    /// included whether or not this flag is specified.
+    /// Include imports of SPIs declared with `@_spi`
     SPIAccessControl = 1 << 3,
     /// Include imports shadowed by a cross-import overlay. Unshadowed imports
     /// are included whether or not this flag is specified.
@@ -605,33 +604,8 @@ class ModuleDecl : public DeclContext, public TypeDecl {
 
   /// Looks up which modules are imported by this module.
   ///
-  /// \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.
+  /// \p filter controls whether public, private, or any imports are included
+  /// in this list.
   void getImportedModules(SmallVectorImpl<ImportedModule> &imports,
                           ImportFilter filter = ImportFilterKind::Exported) const;
 

include/swift/Basic/OptionSet.h

@@ -19,7 +19,6 @@
 
 #include "llvm/ADT/None.h"
 
-#include <cassert>
 #include <type_traits>
 #include <cstdint>
 #include <initializer_list>
@@ -99,14 +98,6 @@ 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

@@ -174,7 +174,6 @@ ImportSet &ImportCache::getImportSet(const DeclContext *dc) {
   imports.emplace_back(ImportPath::Access(), mod);
 
   if (file) {
-    // Should include both SPI & non-SPI.
     file->getImportedModules(imports,
                              {ModuleDecl::ImportFilterKind::Default,
                               ModuleDecl::ImportFilterKind::ImplementationOnly,
@@ -259,7 +258,6 @@ ImportCache::getAllAccessPathsNotShadowedBy(const ModuleDecl *mod,
   stack.emplace_back(ImportPath::Access(), currentMod);
 
   if (auto *file = dyn_cast<FileUnit>(dc)) {
-    // Should include both SPI & non-SPI
     file->getImportedModules(stack,
                              {ModuleDecl::ImportFilterKind::Default,
                               ModuleDecl::ImportFilterKind::ImplementationOnly,

lib/AST/Module.cpp

@@ -95,7 +95,7 @@ void BuiltinUnit::LookupCache::lookupValue(
        SmallVectorImpl<ValueDecl*> &Result) {
   // Only qualified lookup ever finds anything in the builtin module.
   if (LookupKind != NLKind::QualifiedLookup) return;
-  
+
   ValueDecl *&Entry = Cache[Name];
   ASTContext &Ctx = M.getParentModule()->getASTContext();
   if (!Entry) {
@@ -175,7 +175,7 @@ class swift::SourceLookupCache {
 
   /// Throw away as much memory as possible.
   void invalidate();
-  
+
   void lookupValue(DeclName Name, NLKind LookupKind,
                    SmallVectorImpl<ValueDecl*> &Result);
 
@@ -203,13 +203,13 @@ class swift::SourceLookupCache {
   void lookupVisibleDecls(ImportPath::Access AccessPath,
                           VisibleDeclConsumer &Consumer,
                           NLKind LookupKind);
-  
+
   void populateMemberCache(const SourceFile &SF);
   void populateMemberCache(const ModuleDecl &Mod);
 
   void lookupClassMembers(ImportPath::Access AccessPath,
                           VisibleDeclConsumer &consumer);
-                          
+
   void lookupClassMember(ImportPath::Access accessPath,
                          DeclName name,
                          SmallVectorImpl<ValueDecl*> &results);
@@ -331,7 +331,7 @@ void SourceLookupCache::lookupValue(DeclName Name, NLKind LookupKind,
                                     SmallVectorImpl<ValueDecl*> &Result) {
   auto I = TopLevelValues.find(Name);
   if (I == TopLevelValues.end()) return;
-  
+
   Result.reserve(I->second.size());
   for (ValueDecl *Elt : I->second)
     Result.push_back(Elt);
@@ -398,7 +398,7 @@ void SourceLookupCache::lookupVisibleDecls(ImportPath::Access AccessPath,
 void SourceLookupCache::lookupClassMembers(ImportPath::Access accessPath,
                                            VisibleDeclConsumer &consumer) {
   assert(accessPath.size() <= 1 && "can only refer to top-level decls");
-  
+
   if (!accessPath.empty()) {
     for (auto &member : ClassMembers) {
       // Non-simple names are also stored under their simple name, so make
@@ -432,11 +432,11 @@ void SourceLookupCache::lookupClassMember(ImportPath::Access accessPath,
                                           DeclName name,
                                           SmallVectorImpl<ValueDecl*> &results) {
   assert(accessPath.size() <= 1 && "can only refer to top-level decls");
-  
+
   auto iter = ClassMembers.find(name);
   if (iter == ClassMembers.end())
     return;
-  
+
   if (!accessPath.empty()) {
     for (ValueDecl *vd : iter->second) {
       auto *nominal = vd->getDeclContext()->getSelfNominalTypeDecl();
@@ -1190,13 +1190,6 @@ 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));
 }
 
@@ -1221,12 +1214,11 @@ SourceFile::getImportedModules(SmallVectorImpl<ImportedModule> &modules,
       requiredFilter |= ModuleDecl::ImportFilterKind::Exported;
     else if (desc.options.contains(ImportFlags::ImplementationOnly))
       requiredFilter |= ModuleDecl::ImportFilterKind::ImplementationOnly;
+    else if (desc.options.contains(ImportFlags::SPIAccessControl))
+      requiredFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
     else
       requiredFilter |= ModuleDecl::ImportFilterKind::Default;
 
-    if (desc.options.contains(ImportFlags::SPIAccessControl))
-      requiredFilter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
-
     if (!separatelyImportedOverlays.lookup(desc.module.importedModule).empty())
       requiredFilter |= ModuleDecl::ImportFilterKind::ShadowedByCrossImportOverlay;
 
@@ -2401,7 +2393,7 @@ bool FileUnit::walk(ASTWalker &walker) {
 #ifndef NDEBUG
     PrettyStackTraceDecl debugStack("walking into decl", D);
 #endif
-    
+
     if (D->walk(walker))
       return true;
 
@@ -2540,15 +2532,15 @@ SourceFile::lookupOpaqueResultType(StringRef MangledName) {
   auto found = ValidatedOpaqueReturnTypes.find(MangledName);
   if (found != ValidatedOpaqueReturnTypes.end())
     return found->second;
-    
+
   // If there are unvalidated decls with opaque types, go through and validate
   // them now.
   (void) getOpaqueReturnTypeDecls();
 
   found = ValidatedOpaqueReturnTypes.find(MangledName);
   if (found != ValidatedOpaqueReturnTypes.end())
     return found->second;
-  
+
   // Otherwise, we don't have a matching opaque decl.
   return nullptr;
 }

lib/Frontend/ModuleInterfaceSupport.cpp

@@ -115,8 +115,7 @@ static void printImports(raw_ostream &out,
 
     SmallVector<ImportedModule, 4> ioiImport;
     M->getImportedModules(ioiImport,
-                          {ModuleDecl::ImportFilterKind::ImplementationOnly,
-                           ModuleDecl::ImportFilterKind::SPIAccessControl});
+                          ModuleDecl::ImportFilterKind::ImplementationOnly);
     ioiImportSet.insert(ioiImport.begin(), ioiImport.end());
   }
 

lib/Serialization/Serialization.cpp

@@ -1057,11 +1057,7 @@ void Serializer::writeInputBlock(const SerializationOptions &options) {
   ImportSet privateImportSet =
       getImportsAsSet(M, ModuleDecl::ImportFilterKind::Default);
   ImportSet spiImportSet =
-      getImportsAsSet(M, {
-          ModuleDecl::ImportFilterKind::Exported,
-          ModuleDecl::ImportFilterKind::Default,
-          ModuleDecl::ImportFilterKind::SPIAccessControl
-      });
+      getImportsAsSet(M, ModuleDecl::ImportFilterKind::SPIAccessControl);
 
   auto clangImporter =
     static_cast<ClangImporter *>(M->getASTContext().getClangModuleLoader());