Revert "Ignore transitive ObjC imports when cross-importing"

Author: compnerd

include/swift/AST/ClangModuleLoader.h

@@ -128,9 +128,7 @@ class ClangModuleLoader : public ModuleLoader {
 
   /// Retrieves the Swift wrapper for the given Clang module, creating
   /// it if necessary.
-  virtual ModuleDecl *
-  getWrapperForModule(const clang::Module *mod,
-                      bool returnOverlayIfPossible = false) const = 0;
+  virtual ModuleDecl *getWrapperForModule(const clang::Module *mod) const = 0;
 
   /// Adds a new search path to the Clang CompilerInstance, as if specified with
   /// -I or -F.

include/swift/AST/Module.h

@@ -465,7 +465,7 @@ class ModuleDecl : public DeclContext, public TypeDecl {
   /// Retrieve the top-level module. If this module is already top-level, this
   /// returns itself. If this is a submodule such as \c Foo.Bar.Baz, this
   /// returns the module \c Foo.
-  ModuleDecl *getTopLevelModule(bool overlay = false);
+  ModuleDecl *getTopLevelModule();
 
   bool isResilient() const {
     return getResilienceStrategy() != ResilienceStrategy::Default;

include/swift/ClangImporter/ClangImporter.h

@@ -330,9 +330,7 @@ class ClangImporter final : public ClangModuleLoader {
 
   /// Retrieves the Swift wrapper for the given Clang module, creating
   /// it if necessary.
-  ModuleDecl *
-  getWrapperForModule(const clang::Module *mod,
-                      bool returnOverlayIfPossible = false) const override;
+  ModuleDecl *getWrapperForModule(const clang::Module *mod) const override;
 
   std::string getBridgingHeaderContents(StringRef headerPath, off_t &fileSize,
                                         time_t &fileModTime);

lib/AST/Module.cpp

@@ -514,16 +514,15 @@ SourceLookupCache &ModuleDecl::getSourceLookupCache() const {
   return *Cache;
 }
 
-ModuleDecl *ModuleDecl::getTopLevelModule(bool overlay) {
+ModuleDecl *ModuleDecl::getTopLevelModule() {
   // If this is a Clang module, ask the Clang importer for the top-level module.
   // We need to check isNonSwiftModule() to ensure we don't look through
   // overlays.
   if (isNonSwiftModule()) {
     if (auto *underlying = findUnderlyingClangModule()) {
       auto &ctx = getASTContext();
       auto *clangLoader = ctx.getClangModuleLoader();
-      return clangLoader->getWrapperForModule(underlying->getTopLevelModule(),
-                                              overlay);
+      return clangLoader->getWrapperForModule(underlying->getTopLevelModule());
     }
   }
   // Swift modules don't currently support submodules.

lib/ClangImporter/ClangImporter.cpp

@@ -1860,13 +1860,8 @@ ModuleDecl *ClangImporter::getImportedHeaderModule() const {
   return Impl.ImportedHeaderUnit->getParentModule();
 }
 
-ModuleDecl *
-ClangImporter::getWrapperForModule(const clang::Module *mod,
-                                   bool returnOverlayIfPossible) const {
-  auto clangUnit = Impl.getWrapperForModule(mod);
-  if (returnOverlayIfPossible && clangUnit->getOverlayModule())
-    return clangUnit->getOverlayModule();
-  return clangUnit->getParentModule();
+ModuleDecl *ClangImporter::getWrapperForModule(const clang::Module *mod) const {
+  return Impl.getWrapperForModule(mod)->getParentModule();
 }
 
 PlatformAvailability::PlatformAvailability(LangOptions &langOpts)

lib/Sema/ImportResolution.cpp

@@ -172,13 +172,15 @@ class ImportResolver final : public DeclVisitor<ImportResolver> {
   /// The list of fully bound imports.
   SmallVector<ImportedModuleDesc, 16> boundImports;
 
-  /// All imported modules which should be considered when cross-importing.
-  /// This is basically the transitive import graph, but with only top-level
-  /// modules and without reexports from Objective-C modules.
+  /// All imported modules, including by re-exports, and including submodules.
+  llvm::DenseSet<ImportedModuleDesc> visibleModules;
+
+  /// \c visibleModules but without the submodules.
   ///
   /// We use a \c SmallSetVector here because this doubles as the worklist for
   /// cross-importing, so we want to keep it in order; this is feasible
-  /// because this set is usually fairly small.
+  /// because this set is usually fairly small, while \c visibleModules is
+  /// often enormous.
   SmallSetVector<ImportedModuleDesc, 64> crossImportableModules;
 
   /// The subset of \c crossImportableModules which may declare cross-imports.
@@ -221,7 +223,7 @@ class ImportResolver final : public DeclVisitor<ImportResolver> {
 
   /// Adds \p desc and everything it re-exports to \c visibleModules using
   /// the settings from \c desc.
-  void addCrossImportableModules(ImportedModuleDesc desc);
+  void addVisibleModules(ImportedModuleDesc desc);
 
   /// * If \p I is a cross-import overlay, registers \p M as overlaying
   ///   \p I.underlyingModule in \c SF.
@@ -339,7 +341,7 @@ void ImportResolver::bindImport(UnboundImport &&I) {
 
 void ImportResolver::addImport(const UnboundImport &I, ModuleDecl *M) {
   auto importDesc = I.makeDesc(M);
-  addCrossImportableModules(importDesc);
+  addVisibleModules(importDesc);
   boundImports.push_back(importDesc);
 }
 
@@ -934,7 +936,7 @@ static bool isSubmodule(ModuleDecl* M) {
   return clangMod && clangMod->Parent;
 }
 
-void ImportResolver::addCrossImportableModules(ImportedModuleDesc importDesc) {
+void ImportResolver::addVisibleModules(ImportedModuleDesc importDesc) {
   // FIXME: namelookup::getAllImports() doesn't quite do what we need (mainly
   // w.r.t. scoped imports), but it seems like we could extend it to do so, and
   // then eliminate most of this.
@@ -951,34 +953,19 @@ void ImportResolver::addCrossImportableModules(ImportedModuleDesc importDesc) {
                                       nextImport.first))
       continue;
 
-    // If we are importing a submodule, treat it as though we imported its
-    // top-level module (or rather, the top-level module's clang overlay if it
-    // has one).
-    if (isSubmodule(nextImport.second)) {
-      nextImport.second =
-          nextImport.second->getTopLevelModule(/*overlay=*/true);
-
-      // If the rewritten import is now for our own parent module, this was an
-      // import of our own clang submodule in a mixed-language module. We don't
-      // want to process our own cross-imports.
-      if (nextImport.second == SF.getParentModule())
-        continue;
-    }
-
     // Drop this module into the ImportDesc so we treat it as imported with the
     // same options and scope as `I`.
     importDesc.module.second = nextImport.second;
 
-    // Add it to the list of cross-importable modules. If it's already there,
-    // we've already done the rest of the work of this loop iteration and can
-    // skip it.
-    if (!crossImportableModules.insert(importDesc))
+    // If we've already imported it, we've also already imported its
+    // imports.
+    if (!visibleModules.insert(importDesc).second)
       continue;
 
-    // We don't consider the re-exports of ObjC modules because ObjC re-exports
-    // everything, so there isn't enough signal there to work from.
-    if (nextImport.second->isNonSwiftModule())
-      continue;
+    // We don't cross-import submodules, so we shouldn't add them to the
+    // visibility set. (However, we do consider their reexports.)
+    if(!isSubmodule(importDesc.module.second))
+      crossImportableModules.insert(importDesc);
 
     // Add the module's re-exports to worklist.
     nextImport.second->getImportedModules(importsWorklist,

test/CrossImport/Inputs/lib-templates/include/core_mi6.h

@@ -4,11 +4,3 @@ module ClangLibrary {
     header "clang_library_submodule.h"
   }
 }
-module core_mi6 {
-  header "core_mi6.h"
-  export *
-}
-module UniversalExports {
-  header "universal_exports.h"
-  export *
-}

test/CrossImport/Inputs/lib-templates/include/core_mi6.swiftcrossimport/ThinLibrary.swiftoverlay

@@ -2,10 +2,6 @@
 // swift-module-flags: -swift-version 5 -enable-library-evolution -module-name UniversalExports
 
 import Swift
-
-// This is also a clang overlay:
-@_exported import UniversalExports
-
 @_exported import AlwaysImported
 import NeverImported
 import MI6    // @_exported imports NeverImported

test/CrossImport/Inputs/lib-templates/include/module.modulemap

@@ -14,12 +14,6 @@ import UniversalExports
 import ThinLibrary
 #endif
 
-// We should have loaded the underlying clang module.
-fromUniversalExportsClang() // no-error
-
-// We should have loaded core_mi6 too.
-fromCoreMI6() // no-error
-
 // We should have loaded _AlwaysImportedOverlay.
 fromAlwaysImportedOverlay() // no-error