Merge pull request swiftlang#32145 from CodaFi/port-of-call

[NFC] Miscellaneous Cleanups To Clang Module Loading

Author: CodaFi

include/swift/AST/Evaluator.h

@@ -243,10 +243,7 @@ class Evaluator {
 public:
   /// Construct a new evaluator that can emit cyclic-dependency
   /// diagnostics through the given diagnostics engine.
-  Evaluator(DiagnosticEngine &diags,
-            bool debugDumpCycles,
-            bool buildDependencyGraph,
-            bool enableExperimentalPrivateDeps);
+  Evaluator(DiagnosticEngine &diags, const LangOptions &opts);
 
   /// Emit GraphViz output visualizing the request graph.
   void emitRequestEvaluatorGraphViz(llvm::StringRef graphVizPath);

lib/AST/ASTContext.cpp

@@ -549,10 +549,7 @@ ASTContext::ASTContext(LangOptions &langOpts, TypeCheckerOptions &typeckOpts,
   : LangOpts(langOpts),
     TypeCheckerOpts(typeckOpts),
     SearchPathOpts(SearchPathOpts), SourceMgr(SourceMgr), Diags(Diags),
-    evaluator(Diags,
-              langOpts.DebugDumpCycles,
-              langOpts.BuildRequestDependencyGraph,
-              langOpts.EnableExperientalPrivateIntransitiveDependencies),
+    evaluator(Diags, langOpts),
     TheBuiltinModule(createBuiltinModule(*this)),
     StdlibModuleName(getIdentifier(STDLIB_NAME)),
     SwiftShimsModuleName(getIdentifier(SWIFT_SHIMS_NAME)),

lib/AST/Evaluator.cpp

@@ -16,6 +16,7 @@
 //===----------------------------------------------------------------------===//
 #include "swift/AST/Evaluator.h"
 #include "swift/AST/DiagnosticEngine.h"
+#include "swift/Basic/LangOptions.h"
 #include "swift/Basic/Range.h"
 #include "swift/Basic/SourceManager.h"
 #include "llvm/ADT/StringExtras.h"
@@ -62,21 +63,20 @@ void Evaluator::registerRequestFunctions(
 }
 
 static evaluator::DependencyRecorder::Mode
-computeDependencyModeFromFlags(bool enableExperimentalPrivateDeps) {
+computeDependencyModeFromFlags(const LangOptions &opts) {
   using Mode = evaluator::DependencyRecorder::Mode;
-  if (enableExperimentalPrivateDeps) {
+  if (opts.EnableExperientalPrivateIntransitiveDependencies) {
     return Mode::ExperimentalPrivateDependencies;
   }
 
   return Mode::StatusQuo;
 }
 
-Evaluator::Evaluator(DiagnosticEngine &diags, bool debugDumpCycles,
-                     bool buildDependencyGraph,
-                     bool enableExperimentalPrivateDeps)
-    : diags(diags), debugDumpCycles(debugDumpCycles),
-      buildDependencyGraph(buildDependencyGraph),
-      recorder{computeDependencyModeFromFlags(enableExperimentalPrivateDeps)} {}
+Evaluator::Evaluator(DiagnosticEngine &diags, const LangOptions &opts)
+    : diags(diags),
+      debugDumpCycles(opts.DebugDumpCycles),
+      buildDependencyGraph(opts.BuildRequestDependencyGraph),
+      recorder{computeDependencyModeFromFlags(opts)} {}
 
 void Evaluator::emitRequestEvaluatorGraphViz(llvm::StringRef graphVizPath) {
   std::error_code error;

lib/ClangImporter/ClangImporter.cpp

@@ -1786,8 +1786,7 @@ ModuleDecl *ClangImporter::Implementation::loadModuleClang(
   if (!clangModule)
     return nullptr;
 
-  return finishLoadingClangModule(importLoc, clangModule,
-                                  /*preferOverlay=*/false);
+  return finishLoadingClangModule(clangModule, importLoc);
 }
 
 ModuleDecl *
@@ -1807,59 +1806,30 @@ ModuleDecl *ClangImporter::Implementation::loadModule(
 }
 
 ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
-    SourceLoc importLoc, const clang::Module *clangModule, bool findOverlay) {
+    const clang::Module *clangModule, SourceLoc importLoc) {
   assert(clangModule);
 
   // Bump the generation count.
   bumpGeneration();
 
-  auto &cacheEntry = ModuleWrappers[clangModule];
-  ModuleDecl *result;
-  ClangModuleUnit *wrapperUnit;
-  if ((wrapperUnit = cacheEntry.getPointer())) {
-    result = wrapperUnit->getParentModule();
-    if (!cacheEntry.getInt()) {
-      // Force load overlays for all imported modules.
-      // FIXME: This forces the creation of wrapper modules for all imports as
-      // well, and may do unnecessary work.
-      cacheEntry.setInt(true);
-      (void) namelookup::getAllImports(result);
-    }
-  } else {
-    // Build the representation of the Clang module in Swift.
-    // FIXME: The name of this module could end up as a key in the ASTContext,
-    // but that's not correct for submodules.
-    Identifier name = SwiftContext.getIdentifier((*clangModule).Name);
-    result = ModuleDecl::create(name, SwiftContext);
-    result->setIsSystemModule(clangModule->IsSystem);
-    result->setIsNonSwiftModule();
-    result->setHasResolvedImports();
-
-    wrapperUnit =
-      new (SwiftContext) ClangModuleUnit(*result, *this, clangModule);
-    result->addFile(*wrapperUnit);
-    SwiftContext.getClangModuleLoader()
-        ->findOverlayFiles(importLoc, result, wrapperUnit);
-    cacheEntry.setPointerAndInt(wrapperUnit, true);
-
-    // Force load overlays for all imported modules.
-    // FIXME: This forces the creation of wrapper modules for all imports as
-    // well, and may do unnecessary work.
+  // Force load overlays for all imported modules.
+  // FIXME: This forces the creation of wrapper modules for all imports as
+  // well, and may do unnecessary work.
+  ClangModuleUnit *wrapperUnit = getWrapperForModule(clangModule, importLoc);
+  ModuleDecl *result = wrapperUnit->getParentModule();
+  if (!ModuleWrappers[clangModule].getInt()) {
+    ModuleWrappers[clangModule].setInt(true);
     (void) namelookup::getAllImports(result);
   }
 
   if (clangModule->isSubModule()) {
-    finishLoadingClangModule(importLoc, clangModule->getTopLevelModule(), true);
+    finishLoadingClangModule(clangModule->getTopLevelModule(), importLoc);
   } else {
     ModuleDecl *&loaded = SwiftContext.LoadedModules[result->getName()];
     if (!loaded)
       loaded = result;
   }
 
-  if (findOverlay)
-    if (ModuleDecl *overlay = wrapperUnit->getOverlayModule())
-      result = overlay;
-
   return result;
 }
 
@@ -1883,8 +1853,7 @@ void ClangImporter::Implementation::handleDeferredImports(SourceLoc diagLoc) {
   // officially supported with bridging headers: app targets and unit tests
   // only. Unfortunately that's not enforced.
   for (size_t i = 0; i < ImportedHeaderExports.size(); ++i) {
-    (void)finishLoadingClangModule(diagLoc, ImportedHeaderExports[i],
-                                   /*preferOverlay=*/true);
+    (void)finishLoadingClangModule(ImportedHeaderExports[i], diagLoc);
   }
 }
 
@@ -2025,7 +1994,7 @@ ClangImporter::Implementation::~Implementation() {
 }
 
 ClangModuleUnit *ClangImporter::Implementation::getWrapperForModule(
-    const clang::Module *underlying) {
+    const clang::Module *underlying, SourceLoc diagLoc) {
   auto &cacheEntry = ModuleWrappers[underlying];
   if (ClangModuleUnit *cached = cacheEntry.getPointer())
     return cached;
@@ -2040,7 +2009,7 @@ ClangModuleUnit *ClangImporter::Implementation::getWrapperForModule(
   auto file = new (SwiftContext) ClangModuleUnit(*wrapper, *this,
                                                  underlying);
   wrapper->addFile(*file);
-  SwiftContext.getClangModuleLoader()->findOverlayFiles(SourceLoc(), wrapper, file);
+  SwiftContext.getClangModuleLoader()->findOverlayFiles(diagLoc, wrapper, file);
   cacheEntry.setPointer(file);
 
   return file;

lib/ClangImporter/ImporterImpl.h

@@ -925,12 +925,12 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
 
   /// Retrieves the Swift wrapper for the given Clang module, creating
   /// it if necessary.
-  ClangModuleUnit *getWrapperForModule(const clang::Module *underlying);
+  ClangModuleUnit *getWrapperForModule(const clang::Module *underlying,
+                                       SourceLoc importLoc = SourceLoc());
 
   /// Constructs a Swift module for the given Clang module.
-  ModuleDecl *finishLoadingClangModule(SourceLoc importLoc,
-                                       const clang::Module *clangModule,
-                                       bool preferOverlay);
+  ModuleDecl *finishLoadingClangModule(const clang::Module *clangModule,
+                                       SourceLoc importLoc);
 
   /// Call finishLoadingClangModule on each deferred import collected
   /// while scanning a bridging header or PCH.

unittests/AST/ArithmeticEvaluator.cpp

@@ -16,6 +16,7 @@
 #include "swift/AST/DiagnosticEngine.h"
 #include "swift/AST/Evaluator.h"
 #include "swift/AST/SimpleRequest.h"
+#include "swift/Basic/LangOptions.h"
 #include "swift/Basic/SourceManager.h"
 #include "gtest/gtest.h"
 #include <cmath>
@@ -218,10 +219,11 @@ TEST(ArithmeticEvaluator, Simple) {
 
   SourceManager sourceMgr;
   DiagnosticEngine diags(sourceMgr);
-  Evaluator evaluator(diags,
-                      /*debugDumpCycles=*/false,
-                      /*buildDependencyGraph=*/true,
-                      /*privateDependencies*/false);
+  LangOptions opts;
+  opts.DebugDumpCycles = false;
+  opts.BuildRequestDependencyGraph = true;
+  opts.EnableExperientalPrivateIntransitiveDependencies = false;
+  Evaluator evaluator(diags, opts);
   evaluator.registerRequestFunctions(Zone::ArithmeticEvaluator,
                                      arithmeticRequestFunctions);
 
@@ -344,10 +346,11 @@ TEST(ArithmeticEvaluator, Cycle) {
 
   SourceManager sourceMgr;
   DiagnosticEngine diags(sourceMgr);
-  Evaluator evaluator(diags,
-                      /*debugDumpCycles=*/false,
-                      /*buildDependencyGraph=*/false,
-                      /*privateDependencies*/false);
+  LangOptions opts;
+  opts.DebugDumpCycles = false;
+  opts.BuildRequestDependencyGraph = false;
+  opts.EnableExperientalPrivateIntransitiveDependencies = false;
+  Evaluator evaluator(diags, opts);
   evaluator.registerRequestFunctions(Zone::ArithmeticEvaluator,
                                      arithmeticRequestFunctions);