Fixup getOverlayModule

ClangModuleUnit::getOverlayModule implies that it requests the Swift
module with the same name as the top level module, and loads it if it is
available. Unfortunately, it used ASTContext::getModule to do so, which
consults every module loader, including the clang importer. So, even
though you couldn't actually get a clang module out at the end of the
day, you could still force a clang module to load implicitly.

When combined with namelookup's import graph computation forcing
overlays, this meant the entire transitive import graph would be loaded
because of the complicated mix of recursion and re-entrancy this
created.

The first step in teasing this apart is to define an API that doesn't
re-enter the clang importer when it loads modules. Then, the callers
that were relying on this need to be updated to explicitly call
ASTContext::getModule themselves.

This will also fix rdar://70745521 by happenstance.

Author: CodaFi

include/swift/AST/ASTContext.h

@@ -909,6 +909,13 @@ class ASTContext final {
   /// \returns The requested module, or NULL if the module cannot be found.
   ModuleDecl *getModule(ImportPath::Module ModulePath);
 
+  /// Attempts to load the matching overlay module for the given clang
+  /// module into this ASTContext.
+  ///
+  /// \returns The Swift overlay module corresponding to the given Clang module,
+  /// or NULL if the overlay module cannot be found.
+  ModuleDecl *getOverlayModule(const FileUnit *ClangModule);
+
   ModuleDecl *getModuleByName(StringRef ModuleName);
 
   ModuleDecl *getModuleByIdentifier(Identifier ModuleID);

lib/AST/ASTContext.cpp

@@ -1928,6 +1928,27 @@ ASTContext::getModule(ImportPath::Module ModulePath) {
   return nullptr;
 }
 
+ModuleDecl *ASTContext::getOverlayModule(const FileUnit *FU) {
+  assert(FU && FU->getKind() == FileUnitKind::ClangModule &&
+         "Overlays can only be retrieved for clang modules!");
+  ImportPath::Module::Builder builder(FU->getParentModule()->getName());
+  auto ModPath = builder.get();
+  if (auto *Existing = getLoadedModule(ModPath)) {
+    if (!Existing->isNonSwiftModule())
+      return Existing;
+  }
+
+  for (auto &importer : getImpl().ModuleLoaders) {
+    if (importer.get() == getClangModuleLoader())
+      continue;
+    if (ModuleDecl *M = importer->loadModule(SourceLoc(), ModPath)) {
+      return M;
+    }
+  }
+
+  return nullptr;
+}
+
 ModuleDecl *ASTContext::getModuleByName(StringRef ModuleName) {
   ImportPath::Module::Builder builder(*this, ModuleName, /*separator=*/'.');
   return getModule(builder.get());

lib/ClangImporter/ClangImporter.cpp

@@ -3425,18 +3425,23 @@ ModuleDecl *ClangModuleUnit::getOverlayModule() const {
     // FIXME: Include proper source location.
     ModuleDecl *M = getParentModule();
     ASTContext &Ctx = M->getASTContext();
-    auto overlay = Ctx.getModuleByIdentifier(M->getName());
-    if (overlay == M) {
-      overlay = nullptr;
-    } else {
-      // FIXME: This bizarre and twisty invariant is due to nested
-      // re-entrancy in both clang module loading and overlay module loading.
-      auto *sharedModuleRef = Ctx.getLoadedModule(M->getName());
-      assert(!sharedModuleRef || sharedModuleRef == overlay ||
-             sharedModuleRef == M);
+    auto overlay = Ctx.getOverlayModule(this);
+    if (overlay) {
       Ctx.addLoadedModule(overlay);
+    } else {
+      // FIXME: This is the awful legacy of the old implementation of overlay
+      // loading laid bare. Because the previous implementation used
+      // ASTContext::getModuleByIdentifier, it consulted the clang importer
+      // recursively which forced the current module, its dependencies, and
+      // the overlays of those dependencies to load and
+      // become visible in the current context. All of the callers of
+      // ClangModuleUnit::getOverlayModule are relying on this behavior, and
+      // untangling them is going to take a heroic amount of effort.
+      // Clang module loading should *never* *ever* be allowed to load unrelated
+      // Swift modules.
+      ImportPath::Module::Builder builder(M->getName());
+      (void) owner.loadModule(SourceLoc(), std::move(builder).get());
     }
-
     auto mutableThis = const_cast<ClangModuleUnit *>(this);
     mutableThis->overlayModule.setPointerAndInt(overlay, true);
   }