AST: Simplify ModuleDecl::forAllVisibleModules()

Author: slavapestov

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.

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,28 @@ bool ModuleDecl::isSystemModule() const {
   return false;
 }
 
-template<bool respectVisibility, typename Callback>
 static bool forAllImportedModules(ModuleDecl *topLevel,
                                   ModuleDecl::AccessPathTy thisPath,
-                                  bool includePrivateTopLevelImports,
-                                  const Callback &fn) {
+                                  llvm::function_ref<bool(ModuleDecl::ImportedModule)> fn) {
   using ImportedModule = ModuleDecl::ImportedModule;
-  using AccessPathTy = ModuleDecl::AccessPathTy;
 
   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);
+  topLevel->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, topLevel));
 
   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,20 +1161,15 @@ 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);
+  return forAllImportedModules(this, thisPath, fn);
 }
 
 bool FileUnit::forAllVisibleModules(
@@ -1215,13 +1198,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()) {

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();

lib/Sema/TypeChecker.cpp

@@ -650,6 +650,8 @@ void swift::performTypeChecking(SourceFile &SF, TopLevelContext &TLC,
             bindExtensionDecl(ED, TC);
         }
       }
+
+      return true;
     });
 
     // Type check the top-level elements of the source file.

tools/swift-ide-test/swift-ide-test.cpp

@@ -2449,6 +2449,8 @@ static int doPrintModuleImports(const CompilerInvocation &InitInvok,
           llvm::outs() << " (Clang)";
         llvm::outs() << "\n";
       }
+
+      return true;
     });
   }