[ScanDependency] Move binary module validation into scanner

Improve swift dependency scanner by validating and selecting dependency
module into scanner. This provides benefits that:
* Build system does not need to schedule interface compilation task if
  the candidate module is picked, it can just use the candidate module
  directly.
* There is no need for forwarding module in the explicit module build.
  Since the build system is coordinating the build, there is no need for
  the forwarding module in the module cache to avoid duplicated work,
* This also correctly supports all the module loading modes in the
  dependency scanner.

This is achieved by only adding validate and up-to-date binary module as
the candidate module for swift interface module dependency. This allows
caching build to construct the correct dependency in the CAS. If there
is a candidate module for the interface module, dependency scanner will
return a binary module dependency in the dependency graph.

The legacy behavior is mostly preserved with a hidden frontend flag
`-no-scanner-module-validation`, while the scanner output is mostly
interchangeable with new scanner behavior with `prefer-interface` module
loading mode except the candidate module will not be returned.

rdar://123711823

Author: cachemeifyoucan

include/swift/AST/ModuleDependencies.h

@@ -1115,6 +1115,12 @@ class ModuleDependenciesCache {
   std::optional<const ModuleDependencyInfo *>
   findDependency(StringRef moduleName) const;
 
+  /// Look for known existing dependencies.
+  ///
+  /// \returns the cached result.
+  const ModuleDependencyInfo &
+  findKnownDependency(const ModuleDependencyID &moduleID) const;
+
   /// Record dependencies for the given module.
   void recordDependency(StringRef moduleName,
                         ModuleDependencyInfo dependencies);

include/swift/AST/SearchPathOptions.h

@@ -38,6 +38,15 @@ enum class ModuleSearchPathKind {
   RuntimeLibrary,
 };
 
+/// Specifies how to load modules when both a module interface and serialized
+/// AST are present, or whether to disallow one format or the other altogether.
+enum class ModuleLoadingMode {
+  PreferInterface,
+  PreferSerialized,
+  OnlyInterface,
+  OnlySerialized
+};
+
 /// A single module search path that can come from different sources, e.g.
 /// framework search paths, import search path etc.
 class ModuleSearchPath : public llvm::RefCountedBase<ModuleSearchPath> {
@@ -499,6 +508,12 @@ class SearchPathOptions {
   /// original form.
   PathObfuscator DeserializedPathRecoverer;
 
+  /// Specify the module loading behavior of the compilation.
+  ModuleLoadingMode ModuleLoadMode = ModuleLoadingMode::PreferSerialized;
+
+  /// Legacy scanner search behavior.
+  bool NoScannerModuleValidation = false;
+
   /// Return all module search paths that (non-recursively) contain a file whose
   /// name is in \p Filenames.
   SmallVector<const ModuleSearchPath *, 4>
@@ -546,7 +561,9 @@ class SearchPathOptions {
                         RuntimeResourcePath,
                         hash_combine_range(RuntimeLibraryImportPaths.begin(),
                                            RuntimeLibraryImportPaths.end()),
-                        DisableModulesValidateSystemDependencies);
+                        DisableModulesValidateSystemDependencies,
+                        NoScannerModuleValidation,
+                        ModuleLoadMode);
   }
 
   /// Return a hash code of any components from these options that should

include/swift/Option/FrontendOptions.td

@@ -1343,4 +1343,10 @@ def disable_experimental_parser_round_trip : Flag<["-"],
   "disable-experimental-parser-round-trip">,
   HelpText<"Disable round trip through the new swift parser">;
 
+def no_scanner_module_validation: Flag<["-"], "no-scanner-module-validation">,
+    HelpText<"Do not validate binary modules in scanner and delegate the validation to swift-frontend">;
+def module_load_mode: Separate<["-"], "module-load-mode">,
+    MetaVarName<"only-interface|prefer-interface|prefer-serialized|only-serialized">,
+    HelpText<"Module loading mode">;
+
 } // end let Flags = [FrontendOption, NoDriverOption, HelpHidden]

include/swift/Serialization/SerializedModuleLoader.h

@@ -16,6 +16,7 @@
 #include "swift/AST/FileUnit.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/ModuleLoader.h"
+#include "swift/AST/SearchPathOptions.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/PrefixMapper.h"
 #include "llvm/TargetParser/Triple.h"
@@ -28,15 +29,6 @@ namespace file_types {
   enum ID : uint8_t;
 }
 
-/// Specifies how to load modules when both a module interface and serialized
-/// AST are present, or whether to disallow one format or the other altogether.
-enum class ModuleLoadingMode {
-  PreferInterface,
-  PreferSerialized,
-  OnlyInterface,
-  OnlySerialized
-};
-
 /// How a dependency should be loaded.
 ///
 /// \sa getTransitiveLoadingBehavior

lib/AST/ModuleDependencies.cpp

@@ -740,6 +740,13 @@ ModuleDependenciesCache::findDependency(StringRef moduleName) const {
   return std::nullopt;
 }
 
+const ModuleDependencyInfo &ModuleDependenciesCache::findKnownDependency(
+    const ModuleDependencyID &moduleID) const {
+  auto dep = findDependency(moduleID);
+  assert(dep && "dependency unknown");
+  return **dep;
+}
+
 bool ModuleDependenciesCache::hasDependency(const ModuleDependencyID &moduleID) const {
   return hasDependency(moduleID.ModuleName, moduleID.Kind);
 }

lib/DependencyScan/ModuleDependencyScanner.cpp

@@ -174,7 +174,8 @@ ModuleDependencyScanningWorker::ModuleDependencyScanningWorker(
       ScanASTContext,
       *static_cast<ModuleInterfaceCheckerImpl *>(
           ScanASTContext.getModuleInterfaceChecker()),
-      &DependencyTracker, ModuleLoadingMode::OnlyInterface);
+      &DependencyTracker,
+      ScanCompilerInvocation.getSearchPathOptions().ModuleLoadMode);
 }
 
 ModuleDependencyVector

lib/DependencyScan/ScanDependencies.cpp

@@ -192,14 +192,15 @@ updateModuleCacheKey(ModuleDependencyInfo &depInfo,
 }
 
 static llvm::Error resolveExplicitModuleInputs(
-    ModuleDependencyID moduleID, const ModuleDependencyInfo &resolvingDepInfo,
+    ModuleDependencyID moduleID,
     const std::set<ModuleDependencyID> &dependencies,
     ModuleDependenciesCache &cache, CompilerInstance &instance,
     std::optional<std::set<ModuleDependencyID>> bridgingHeaderDeps) {
   // Only need to resolve dependency for following dependencies.
   if (moduleID.Kind == ModuleDependencyKind::SwiftPlaceholder)
     return llvm::Error::success();
 
+  auto &resolvingDepInfo = cache.findKnownDependency(moduleID);
   // If the dependency is already finalized, nothing needs to be done.
   if (resolvingDepInfo.isFinalized())
     return llvm::Error::success();
@@ -242,13 +243,10 @@ static llvm::Error resolveExplicitModuleInputs(
 
   std::vector<std::string> commandLine = resolvingDepInfo.getCommandline();
   for (const auto &depModuleID : dependencies) {
-    const auto optionalDepInfo =
-        cache.findDependency(depModuleID);
-    assert(optionalDepInfo.has_value());
-    const auto depInfo = optionalDepInfo.value();
+    const auto &depInfo = cache.findKnownDependency(depModuleID);
     switch (depModuleID.Kind) {
     case swift::ModuleDependencyKind::SwiftInterface: {
-      auto interfaceDepDetails = depInfo->getAsSwiftInterfaceModule();
+      auto interfaceDepDetails = depInfo.getAsSwiftInterfaceModule();
       assert(interfaceDepDetails && "Expected Swift Interface dependency.");
       auto &path = interfaceDepDetails->moduleCacheKey.empty()
                        ? interfaceDepDetails->moduleOutputPath
@@ -257,7 +255,7 @@ static llvm::Error resolveExplicitModuleInputs(
                             path);
     } break;
     case swift::ModuleDependencyKind::SwiftBinary: {
-      auto binaryDepDetails = depInfo->getAsSwiftBinaryModule();
+      auto binaryDepDetails = depInfo.getAsSwiftBinaryModule();
       assert(binaryDepDetails && "Expected Swift Binary Module dependency.");
       auto &path = binaryDepDetails->moduleCacheKey.empty()
                        ? binaryDepDetails->compiledModulePath
@@ -280,13 +278,13 @@ static llvm::Error resolveExplicitModuleInputs(
       }
     } break;
     case swift::ModuleDependencyKind::SwiftPlaceholder: {
-      auto placeholderDetails = depInfo->getAsPlaceholderDependencyModule();
+      auto placeholderDetails = depInfo.getAsPlaceholderDependencyModule();
       assert(placeholderDetails && "Expected Swift Placeholder dependency.");
       commandLine.push_back("-swift-module-file=" + depModuleID.ModuleName + "=" +
                             placeholderDetails->compiledModulePath);
     } break;
     case swift::ModuleDependencyKind::Clang: {
-      auto clangDepDetails = depInfo->getAsClangModule();
+      auto clangDepDetails = depInfo.getAsClangModule();
       assert(clangDepDetails && "Expected Clang Module dependency.");
       if (!resolvingDepInfo.isClangModule()) {
         commandLine.push_back("-Xcc");
@@ -303,13 +301,13 @@ static llvm::Error resolveExplicitModuleInputs(
       }
 
       // Only need to merge the CASFS from clang importer.
-      if (auto ID = depInfo->getCASFSRootID())
+      if (auto ID = depInfo.getCASFSRootID())
         rootIDs.push_back(*ID);
-      if (auto ID = depInfo->getClangIncludeTree())
+      if (auto ID = depInfo.getClangIncludeTree())
         includeTrees.push_back(*ID);
     } break;
     case swift::ModuleDependencyKind::SwiftSource: {
-      if (auto E = addBridgingHeaderDeps(*depInfo))
+      if (auto E = addBridgingHeaderDeps(depInfo))
         return E;
       break;
     }
@@ -390,9 +388,8 @@ static llvm::Error resolveExplicitModuleInputs(
       std::vector<std::string> newCommandLine =
           dependencyInfoCopy.getBridgingHeaderCommandline();
       for (auto bridgingDep : *bridgingHeaderDeps) {
-        auto dep = cache.findDependency(bridgingDep);
-        assert(dep && "unknown clang dependency");
-        auto *clangDep = (*dep)->getAsClangModule();
+        auto &dep = cache.findKnownDependency(bridgingDep);
+        auto *clangDep = dep.getAsClangModule();
         assert(clangDep && "wrong module dependency kind");
         if (!clangDep->moduleCacheKey.empty()) {
           newCommandLine.push_back("-Xcc");
@@ -406,19 +403,11 @@ static llvm::Error resolveExplicitModuleInputs(
       dependencyInfoCopy.updateBridgingHeaderCommandLine(newCommandLine);
     }
 
-    if (resolvingDepInfo.isClangModule() ||
-        resolvingDepInfo.isSwiftInterfaceModule()) {
-      // Compute and update module cache key.
-      auto Key = updateModuleCacheKey(dependencyInfoCopy, cache, CAS);
-      if (!Key)
-        return Key.takeError();
-    }
-
-    // For binary module, we need to make sure the lookup key is setup here in
-    // action cache. We just use the CASID of the binary module itself as key.
-    if (auto *binaryDep = dependencyInfoCopy.getAsSwiftBinaryModule()) {
-      auto Ref =
-          CASFS.getObjectRefForFileContent(binaryDep->compiledModulePath);
+    // Compute and update module cache key.
+    auto setupBinaryCacheKey = [&](StringRef path) -> llvm::Error {
+      // For binary module, we need to make sure the lookup key is setup here in
+      // action cache. We just use the CASID of the binary module itself as key.
+      auto Ref = CASFS.getObjectRefForFileContent(path);
       if (!Ref)
         return llvm::errorCodeToError(Ref.getError());
       assert(*Ref && "Binary module should be loaded into CASFS already");
@@ -432,30 +421,44 @@ static llvm::Error resolveExplicitModuleInputs(
       if (auto E = instance.getActionCache().put(CAS.getID(**Ref),
                                                  CAS.getID(*Result)))
         return E;
+      return llvm::Error::success();
+    };
+
+    if (resolvingDepInfo.isClangModule() ||
+        resolvingDepInfo.isSwiftInterfaceModule()) {
+      auto Key = updateModuleCacheKey(dependencyInfoCopy, cache, CAS);
+      if (!Key)
+        return Key.takeError();
+    } else if (auto *binaryDep = dependencyInfoCopy.getAsSwiftBinaryModule()) {
+      if (auto E = setupBinaryCacheKey(binaryDep->compiledModulePath))
+        return E;
     }
   }
+
   dependencyInfoCopy.setIsFinalized(true);
   cache.updateDependency(moduleID, dependencyInfoCopy);
 
   return llvm::Error::success();
 }
 
-static llvm::Error pruneUnusedVFSOverlays(
-    ModuleDependencyID moduleID, const ModuleDependencyInfo &resolvingDepInfo,
-    const std::set<ModuleDependencyID> &dependencies,
-    ModuleDependenciesCache &cache, CompilerInstance &instance) {
+static llvm::Error
+pruneUnusedVFSOverlays(ModuleDependencyID moduleID,
+                       const std::set<ModuleDependencyID> &dependencies,
+                       ModuleDependenciesCache &cache,
+                       CompilerInstance &instance) {
+  // Pruning of unused VFS overlay options for Clang dependencies
+  // is performed by the Clang dependency scanner.
+  if (moduleID.Kind == ModuleDependencyKind::Clang)
+    return llvm::Error::success();
+
   auto isVFSOverlayFlag = [](StringRef arg) {
     return arg == "-ivfsoverlay" || arg == "-vfsoverlay";
   };
   auto isXCCArg = [](StringRef arg) {
     return arg == "-Xcc";
   };
 
-  // Pruning of unused VFS overlay options for Clang dependencies
-  // is performed by the Clang dependency scanner.
-  if (!resolvingDepInfo.isSwiftModule())
-    return llvm::Error::success();
-
+  auto &resolvingDepInfo = cache.findKnownDependency(moduleID);
   // If this Swift dependency contains any VFS overlay paths,
   // then attempt to prune the ones not used by any of the Clang dependencies.
   if (!llvm::any_of(resolvingDepInfo.getCommandline(),
@@ -469,10 +472,8 @@ static llvm::Error pruneUnusedVFSOverlays(
   // pruned by the Clang dependency scanner.
   llvm::StringSet<> usedVFSOverlayPaths;
   for (const auto &depModuleID : dependencies) {
-    const auto optionalDepInfo = cache.findDependency(depModuleID);
-    assert(optionalDepInfo.has_value());
-    const auto depInfo = optionalDepInfo.value();
-    if (auto clangDepDetails = depInfo->getAsClangModule()) {
+    const auto &depInfo = cache.findKnownDependency(depModuleID);
+    if (auto clangDepDetails = depInfo.getAsClangModule()) {
       const auto &depCommandLine = clangDepDetails->buildCommandLine;
       // true if the previous argument was the dash-option of an option pair
       bool getNext = false;
@@ -1158,24 +1159,17 @@ generateFullDependencyGraph(const CompilerInstance &instance,
 
   for (size_t i = 0; i < allModules.size(); ++i) {
     const auto &module = allModules[i];
-    // Grab the completed module dependencies.
-    auto moduleDepsQuery = cache.findDependency(module);
-    if (!moduleDepsQuery) {
-      llvm::report_fatal_error(Twine("Module Dependency Cache missing module") +
-                               module.ModuleName);
-    }
-
-    auto moduleDeps = *moduleDepsQuery;
+    auto &moduleDeps = cache.findKnownDependency(module);
     // Collect all the required pieces to build a ModuleInfo
-    auto swiftPlaceholderDeps = moduleDeps->getAsPlaceholderDependencyModule();
-    auto swiftTextualDeps = moduleDeps->getAsSwiftInterfaceModule();
-    auto swiftSourceDeps = moduleDeps->getAsSwiftSourceModule();
-    auto swiftBinaryDeps = moduleDeps->getAsSwiftBinaryModule();
-    auto clangDeps = moduleDeps->getAsClangModule();
+    auto swiftPlaceholderDeps = moduleDeps.getAsPlaceholderDependencyModule();
+    auto swiftTextualDeps = moduleDeps.getAsSwiftInterfaceModule();
+    auto swiftSourceDeps = moduleDeps.getAsSwiftSourceModule();
+    auto swiftBinaryDeps = moduleDeps.getAsSwiftBinaryModule();
+    auto clangDeps = moduleDeps.getAsClangModule();
 
     // ModulePath
     const char *modulePathSuffix =
-        moduleDeps->isSwiftModule() ? ".swiftmodule" : ".pcm";
+        moduleDeps.isSwiftModule() ? ".swiftmodule" : ".pcm";
     std::string modulePath;
     if (swiftTextualDeps)
       modulePath = swiftTextualDeps->moduleOutputPath;
@@ -1196,10 +1190,8 @@ generateFullDependencyGraph(const CompilerInstance &instance,
       sourceFiles = clangDeps->fileDependencies;
     }
 
-    auto optionalDepInfo = cache.findDependency(module);
-    assert(optionalDepInfo.has_value() && "Missing dependency info during graph generation diagnosis.");
-    auto depInfo = optionalDepInfo.value();
-    auto directDependencies = depInfo->getDirectModuleDependencies();
+    auto &depInfo = cache.findKnownDependency(module);
+    auto directDependencies = depInfo.getDirectModuleDependencies();
 
     // Generate a swiftscan_clang_details_t object based on the dependency kind
     auto getModuleDetails = [&]() -> swiftscan_module_details_t {
@@ -1208,9 +1200,12 @@ generateFullDependencyGraph(const CompilerInstance &instance,
         swiftscan_string_ref_t moduleInterfacePath =
             create_clone(swiftTextualDeps->swiftInterfaceFile.c_str());
         swiftscan_string_ref_t bridgingHeaderPath =
-            swiftTextualDeps->textualModuleDetails.bridgingHeaderFile.has_value()
+            swiftTextualDeps->textualModuleDetails.bridgingHeaderFile
+                    .has_value()
                 ? create_clone(
-                      swiftTextualDeps->textualModuleDetails.bridgingHeaderFile.value().c_str())
+                      swiftTextualDeps->textualModuleDetails.bridgingHeaderFile
+                          .value()
+                          .c_str())
                 : create_null();
         details->kind = SWIFTSCAN_DEPENDENCY_INFO_SWIFT_TEXTUAL;
         // Create an overlay dependencies set according to the output format
@@ -1414,12 +1409,12 @@ computeTransitiveClosureOfExplicitDependencies(
 }
 
 static std::set<ModuleDependencyID> computeBridgingHeaderTransitiveDependencies(
-    const ModuleDependencyInfo *dep,
+    const ModuleDependencyInfo &dep,
     const std::unordered_map<ModuleDependencyID, std::set<ModuleDependencyID>>
         &transitiveClosures,
     const ModuleDependenciesCache &cache) {
   std::set<ModuleDependencyID> result;
-  auto *sourceDep = dep->getAsSwiftSourceModule();
+  auto *sourceDep = dep.getAsSwiftSourceModule();
   if (!sourceDep)
     return result;
 
@@ -1871,22 +1866,19 @@ static void resolveDependencyCommandLineArguments(
     // For main module or binary modules, no command-line to resolve.
     // For Clang modules, their dependencies are resolved by the clang Scanner
     // itself for us.
-    auto optionalDeps = cache.findDependency(modID);
-    assert(optionalDeps.has_value());
-    auto deps = optionalDeps.value();
+    auto &deps = cache.findKnownDependency(modID);
     std::optional<std::set<ModuleDependencyID>> bridgingHeaderDeps;
     if (modID.Kind == ModuleDependencyKind::SwiftSource)
       bridgingHeaderDeps = computeBridgingHeaderTransitiveDependencies(
           deps, moduleTransitiveClosures, cache);
 
-    if (auto E =
-            resolveExplicitModuleInputs(modID, *deps, dependencyClosure, cache,
-                                        instance, bridgingHeaderDeps))
+    if (auto E = resolveExplicitModuleInputs(modID, dependencyClosure, cache,
+                                             instance, bridgingHeaderDeps))
       instance.getDiags().diagnose(SourceLoc(), diag::error_cas,
                                    toString(std::move(E)));
 
-    if (auto E = pruneUnusedVFSOverlays(modID, *deps, dependencyClosure,
-                                        cache, instance))
+    if (auto E =
+            pruneUnusedVFSOverlays(modID, dependencyClosure, cache, instance))
       instance.getDiags().diagnose(SourceLoc(), diag::error_cas,
                                    toString(std::move(E)));
   }