Serialization cleanup

include/swift/AST/Module.h

@@ -400,46 +400,14 @@ class ModuleDecl : public DeclContext, public TypeDecl {
   ///
   /// \param topLevelAccessPath If present, include the top-level module in the
   ///        results, with the given access path.
-  /// \param includePrivateTopLevelImports If true, imports listed in all
-  ///        file units within this module are traversed. Otherwise (the
-  ///        default), only re-exported imports are traversed.
   /// \param fn A callback of type bool(ImportedModule) or void(ImportedModule).
   ///        Return \c false to abort iteration.
   ///
   /// \return True if the traversal ran to completion, false if it ended early
   ///         due to the callback.
   bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
-                            bool includePrivateTopLevelImports,
                             llvm::function_ref<bool(ImportedModule)> fn);
 
-  bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
-                            bool includePrivateTopLevelImports,
-                            llvm::function_ref<void(ImportedModule)> fn) {
-    return forAllVisibleModules(topLevelAccessPath,
-                                includePrivateTopLevelImports,
-                                [=](const ImportedModule &import) -> bool {
-      fn(import);
-      return true;
-    });
-  }
-
-  template <typename Fn>
-  bool forAllVisibleModules(AccessPathTy topLevelAccessPath,
-                            bool includePrivateTopLevelImports,
-                            Fn &&fn) {
-    using RetTy = typename std::result_of<Fn(ImportedModule)>::type;
-    llvm::function_ref<RetTy(ImportedModule)> wrapped{std::forward<Fn>(fn)};
-    return forAllVisibleModules(topLevelAccessPath,
-                                includePrivateTopLevelImports,
-                                wrapped);
-  }
-
-  template <typename Fn>
-  bool forAllVisibleModules(AccessPathTy topLevelAccessPath, Fn &&fn) {
-    return forAllVisibleModules(topLevelAccessPath, false,
-                                std::forward<Fn>(fn));
-  }
-
   /// @}
 
   using LinkLibraryCallback = llvm::function_ref<void(LinkLibrary)>;
@@ -687,23 +655,6 @@ class FileUnit : public DeclContext {
   bool
   forAllVisibleModules(llvm::function_ref<bool(ModuleDecl::ImportedModule)> fn);
 
-  bool
-  forAllVisibleModules(llvm::function_ref<void(ModuleDecl::ImportedModule)> fn) {
-    return forAllVisibleModules([=](ModuleDecl::ImportedModule import) -> bool {
-      fn(import);
-      return true;
-    });
-  }
-
-  template <typename Fn>
-  bool forAllVisibleModules(Fn &&fn) {
-    using RetTy = typename std::result_of<Fn(ModuleDecl::ImportedModule)>::type;
-    llvm::function_ref<RetTy(ModuleDecl::ImportedModule)> wrapped{
-      std::forward<Fn>(fn)
-    };
-    return forAllVisibleModules(wrapped);
-  }
-
   /// @}
 
   /// True if this file contains the main class for the module.

include/swift/Serialization/ModuleFile.h

@@ -156,8 +156,6 @@ class ModuleFile
     bool isHeader() const { return IsHeader; }
     bool isScoped() const { return IsScoped; }
 
-    void forceExported() { IsExported = true; }
-
     std::string getPrettyPrintedPath() const;
   };
 
@@ -399,12 +397,6 @@ class ModuleFile
     /// Whether this module file comes from a framework.
     unsigned IsFramework : 1;
 
-    /// THIS SETTING IS OBSOLETE BUT IS STILL USED BY OLDER MODULES.
-    ///
-    /// Whether this module has a shadowed module that's part of its public
-    /// interface.
-    unsigned HasUnderlyingModule : 1;
-
     /// Whether or not ImportDecls is valid.
     unsigned ComputedImportDecls : 1;
 

include/swift/Serialization/ModuleFormat.h

@@ -583,7 +583,7 @@ namespace input_block {
     LINK_LIBRARY,
     IMPORTED_HEADER,
     IMPORTED_HEADER_CONTENTS,
-    MODULE_FLAGS,
+    MODULE_FLAGS, // [unused]
     SEARCH_PATH
   };
 
@@ -616,11 +616,6 @@ namespace input_block {
     BCBlob
   >;
 
-  using ModuleFlagsLayout = BCRecordLayout<
-    MODULE_FLAGS,
-    BCFixed<1> // has underlying module? [[UNUSED]]
-  >;
-
   using SearchPathLayout = BCRecordLayout<
     SEARCH_PATH,
     BCFixed<1>, // framework?

lib/AST/ASTContext.cpp

@@ -2348,6 +2348,7 @@ void ASTContext::diagnoseAttrsRequiringFoundation(SourceFile &SF) {
   SF.forAllVisibleModules([&](ModuleDecl::ImportedModule import) {
     if (import.second->getName() == Id_Foundation)
       ImportsFoundationModule = true;
+    return true;
   });
 
   if (ImportsFoundationModule)

lib/AST/LookupVisibleDecls.cpp

@@ -387,6 +387,7 @@ static void doDynamicLookup(VisibleDeclConsumer &Consumer,
   CurrDC->getParentSourceFile()->forAllVisibleModules(
       [&](ModuleDecl::ImportedModule Import) {
         Import.second->lookupClassMembers(Import.first, ConsumerWrapper);
+        return true;
       });
 }
 

lib/AST/Module.cpp

@@ -1127,40 +1127,25 @@ bool ModuleDecl::isSystemModule() const {
   return false;
 }
 
-template<bool respectVisibility, typename Callback>
-static bool forAllImportedModules(ModuleDecl *topLevel,
-                                  ModuleDecl::AccessPathTy thisPath,
-                                  bool includePrivateTopLevelImports,
-                                  const Callback &fn) {
-  using ImportedModule = ModuleDecl::ImportedModule;
-  using AccessPathTy = ModuleDecl::AccessPathTy;
-
+bool ModuleDecl::forAllVisibleModules(AccessPathTy thisPath,
+                                  llvm::function_ref<bool(ImportedModule)> fn) {
   llvm::SmallSet<ImportedModule, 32, ModuleDecl::OrderImportedModules> visited;
   SmallVector<ImportedModule, 32> stack;
 
-  // Even if we're processing the top-level module like any other, we may
-  // still want to include non-exported modules.
-  ModuleDecl::ImportFilter filter = respectVisibility ? ModuleDecl::ImportFilter::Public
-                                                      : ModuleDecl::ImportFilter::All;
-  ModuleDecl::ImportFilter topLevelFilter =
-    includePrivateTopLevelImports ? ModuleDecl::ImportFilter::All : filter;
-  topLevel->getImportedModules(stack, topLevelFilter);
+  getImportedModules(stack, ModuleDecl::ImportFilter::Public);
 
   // Make sure the top-level module is first; we want pre-order-ish traversal.
-  AccessPathTy overridingPath;
-  if (respectVisibility)
-    overridingPath = thisPath;
-  stack.push_back(ImportedModule(overridingPath, topLevel));
+  stack.push_back(ImportedModule(thisPath, this));
 
   while (!stack.empty()) {
     auto next = stack.pop_back_val();
 
     // Filter any whole-module imports, and skip specific-decl imports if the
     // import path doesn't match exactly.
-    if (next.first.empty() || !respectVisibility)
-      next.first = overridingPath;
-    else if (!overridingPath.empty() &&
-             !ModuleDecl::isSameAccessPath(next.first, overridingPath)) {
+    if (next.first.empty())
+      next.first = thisPath;
+    else if (!thisPath.empty() &&
+             !ModuleDecl::isSameAccessPath(next.first, thisPath)) {
       // If we ever allow importing non-top-level decls, it's possible the rule
       // above isn't what we want.
       assert(next.first.size() == 1 && "import of non-top-level decl");
@@ -1173,22 +1158,12 @@ static bool forAllImportedModules(ModuleDecl *topLevel,
     if (!fn(next))
       return false;
 
-    if (respectVisibility)
-      next.second->getImportedModulesForLookup(stack);
-    else
-      next.second->getImportedModules(stack, filter);
+    next.second->getImportedModulesForLookup(stack);
   }
 
   return true;
 }
 
-bool ModuleDecl::forAllVisibleModules(AccessPathTy thisPath,
-                                      bool includePrivateTopLevelImports,
-                                  llvm::function_ref<bool(ImportedModule)> fn) {
-  return forAllImportedModules<true>(this, thisPath,
-                                     includePrivateTopLevelImports, fn);
-}
-
 bool FileUnit::forAllVisibleModules(
     llvm::function_ref<bool(ModuleDecl::ImportedModule)> fn) {
   if (!getParentModule()->forAllVisibleModules(ModuleDecl::AccessPathTy(), fn))
@@ -1215,13 +1190,15 @@ void ModuleDecl::collectLinkLibraries(LinkLibraryCallback callback) {
 void
 SourceFile::collectLinkLibraries(ModuleDecl::LinkLibraryCallback callback) const {
 
-  const_cast<SourceFile *>(this)->forAllVisibleModules([&](swift::ModuleDecl::ImportedModule import) {
-    swift::ModuleDecl *next = import.second;
-    if (next->getName() == getParentModule()->getName())
-      return;
+  const_cast<SourceFile *>(this)->forAllVisibleModules(
+    [&](swift::ModuleDecl::ImportedModule import) {
+      swift::ModuleDecl *next = import.second;
+      if (next->getName() == getParentModule()->getName())
+        return true;
 
-    next->collectLinkLibraries(callback);
-  });
+      next->collectLinkLibraries(callback);
+      return true;
+    });
 }
 
 bool ModuleDecl::walk(ASTWalker &Walker) {

lib/AST/NameLookup.cpp

@@ -1865,6 +1865,7 @@ bool DeclContext::lookupQualified(Type type,
     SmallVector<ValueDecl *, 4> allDecls;
     forAllVisibleModules(this, [&](ModuleDecl::ImportedModule import) {
       import.second->lookupClassMember(import.first, member, allDecls);
+      return true;
     });
 
     // For each declaration whose context is not something we've
@@ -1927,6 +1928,7 @@ void DeclContext::lookupAllObjCMethods(
   // Collect all of the methods with this selector.
   forAllVisibleModules(this, [&](ModuleDecl::ImportedModule import) {
     import.second->lookupObjCMethods(selector, results);
+    return true;
   });
 
   // Filter out duplicates.

lib/ClangImporter/ClangImporter.cpp

@@ -1576,7 +1576,9 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
       // FIXME: This forces the creation of wrapper modules for all imports as
       // well, and may do unnecessary work.
       cacheEntry.setInt(true);
-      result->forAllVisibleModules({}, [&](ModuleDecl::ImportedModule import) {});
+      result->forAllVisibleModules({}, [&](ModuleDecl::ImportedModule import) {
+          return true;
+        });
     }
   } else {
     // Build the representation of the Clang module in Swift.
@@ -1595,7 +1597,9 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
     // Force load adapter modules for all imported modules.
     // FIXME: This forces the creation of wrapper modules for all imports as
     // well, and may do unnecessary work.
-    result->forAllVisibleModules({}, [](ModuleDecl::ImportedModule import) {});
+    result->forAllVisibleModules({}, [](ModuleDecl::ImportedModule import) {
+        return true;
+      });
   }
 
   if (clangModule->isSubModule()) {
@@ -3039,43 +3043,70 @@ ModuleDecl *ClangModuleUnit::getAdapterModule() const {
 void ClangModuleUnit::getImportedModules(
     SmallVectorImpl<ModuleDecl::ImportedModule> &imports,
     ModuleDecl::ImportFilter filter) const {
-  if (filter != ModuleDecl::ImportFilter::Public)
+  switch (filter) {
+  case ModuleDecl::ImportFilter::All:
+  case ModuleDecl::ImportFilter::Private:
     imports.push_back({ModuleDecl::AccessPathTy(), owner.getStdlibModule()});
+    break;
+  case ModuleDecl::ImportFilter::Public:
+    break;
+  }
 
   SmallVector<clang::Module *, 8> imported;
   if (!clangModule) {
     // This is the special "imported headers" module.
-    if (filter != ModuleDecl::ImportFilter::Private) {
+    switch (filter) {
+    case ModuleDecl::ImportFilter::All:
+    case ModuleDecl::ImportFilter::Public:
       imported.append(owner.ImportedHeaderExports.begin(),
                       owner.ImportedHeaderExports.end());
+      break;
+
+    case ModuleDecl::ImportFilter::Private:
+      break;
     }
 
   } else {
     clangModule->getExportedModules(imported);
-    if (filter != ModuleDecl::ImportFilter::Public) {
-      if (filter == ModuleDecl::ImportFilter::All) {
-        llvm::SmallPtrSet<clang::Module *, 8> knownModules;
-        imported.append(clangModule->Imports.begin(), clangModule->Imports.end());
-        imported.erase(std::remove_if(imported.begin(), imported.end(),
-                                      [&](clang::Module *mod) -> bool {
-                                        return !knownModules.insert(mod).second;
-                                      }),
-                       imported.end());
-      } else {
-        llvm::SmallPtrSet<clang::Module *, 8> knownModules(imported.begin(),
-                                                           imported.end());
-        SmallVector<clang::Module *, 8> privateImports;
-        std::copy_if(clangModule->Imports.begin(), clangModule->Imports.end(),
-                     std::back_inserter(privateImports), [&](clang::Module *mod) {
-                       return knownModules.count(mod) == 0;
-        });
-        imported.swap(privateImports);
-      }
+
+    switch (filter) {
+    case ModuleDecl::ImportFilter::All: {
+      llvm::SmallPtrSet<clang::Module *, 8> knownModules;
+      imported.append(clangModule->Imports.begin(), clangModule->Imports.end());
+      imported.erase(std::remove_if(imported.begin(), imported.end(),
+                                    [&](clang::Module *mod) -> bool {
+                                      return !knownModules.insert(mod).second;
+                                    }),
+                     imported.end());
 
       // FIXME: The parent module isn't exactly a private import, but it is
       // needed for link dependencies.
       if (clangModule->Parent)
         imported.push_back(clangModule->Parent);
+
+      break;
+    }
+
+    case ModuleDecl::ImportFilter::Private: {
+      llvm::SmallPtrSet<clang::Module *, 8> knownModules(imported.begin(),
+                                                         imported.end());
+      SmallVector<clang::Module *, 8> privateImports;
+      std::copy_if(clangModule->Imports.begin(), clangModule->Imports.end(),
+                   std::back_inserter(privateImports), [&](clang::Module *mod) {
+                     return knownModules.count(mod) == 0;
+                   });
+      imported.swap(privateImports);
+
+      // FIXME: The parent module isn't exactly a private import, but it is
+      // needed for link dependencies.
+      if (clangModule->Parent)
+        imported.push_back(clangModule->Parent);
+
+      break;
+    }
+
+    case ModuleDecl::ImportFilter::Public:
+      break;
     }
   }
 

lib/IDE/CodeCompletion.cpp

@@ -3329,6 +3329,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
           break;
         }
       }
+
+      return true;
     });
     return results;
   }
@@ -5602,7 +5604,11 @@ void CodeCompletionCallbacksImpl::doneParsing() {
 
       // FIXME: actually check imports.
       const_cast<ModuleDecl*>(Request.TheModule)
-          ->forAllVisibleModules({}, handleImport);
+          ->forAllVisibleModules({},
+                                 [&](ModuleDecl::ImportedModule Import) {
+                                   handleImport(Import);
+                                   return true;
+                                 });
     } else {
       // Add results from current module.
       Lookup.getToplevelCompletions(Request.OnlyTypes);
@@ -5616,7 +5622,11 @@ void CodeCompletionCallbacksImpl::doneParsing() {
       for (auto Imported : Imports) {
         ModuleDecl *TheModule = Imported.second;
         ModuleDecl::AccessPathTy AccessPath = Imported.first;
-        TheModule->forAllVisibleModules(AccessPath, handleImport);
+        TheModule->forAllVisibleModules(AccessPath,
+                                        [&](ModuleDecl::ImportedModule Import) {
+                                          handleImport(Import);
+                                          return true;
+                                        });
       }
     }
     Lookup.RequestedCachedResults.reset();