[Dependency Scanning] Emit a detailed error diagnostic on Clang module variant discovery

In expectation, this should never happen. Such a situation means that within the same scanning action, Clang Dependency Scanner has produced two different variants of the same module. This is not supposed to happen, but we are currently hunting down the rare cases where it does, seemingly due to differences in Clang Scanner direct by-name queries and transitive header lookup queries.

Author: artemcm

include/swift/AST/DiagnosticsFrontend.def

@@ -624,5 +624,15 @@ ERROR(ast_format_requires_dump_ast,none,
 ERROR(unknown_dump_ast_format,none,
       "unknown format '%0' requested with '-dump-ast-format'", (StringRef))
 
+ERROR(dependency_scan_unexpected_variant, none,
+      "unexpected variant during dependency scanning on module '%0'", (StringRef))
+NOTE(dependency_scan_unexpected_variant_context_hash_note, none,
+     "first module context hash: '%0', second module context hash: '%1'", (StringRef, StringRef))
+NOTE(dependency_scan_unexpected_variant_module_map_note, none,
+     "first module module map: '%0', second module module map: '%1'", (StringRef, StringRef))
+NOTE(dependency_scan_unexpected_variant_extra_arg_note, none,
+     "%select{first|second}0 module command-line has extra argument: '%1'", (bool, StringRef))
+
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/AST/ModuleDependencies.h

@@ -55,6 +55,7 @@ class Identifier;
 class CompilerInstance;
 class IRGenOptions;
 class CompilerInvocation;
+class DiagnosticEngine;
 
 /// Which kind of module dependencies we are looking for.
 enum class ModuleDependencyKind : int8_t {
@@ -1254,7 +1255,8 @@ class ModuleDependenciesCache {
                         ModuleDependencyInfo dependencies);
 
   /// Record dependencies for the given module collection.
-  void recordDependencies(ModuleDependencyVector moduleDependencies);
+  void recordDependencies(ModuleDependencyVector moduleDependencies,
+                          DiagnosticEngine &diags);
 
   /// Update stored dependencies for the given module.
   void updateDependency(ModuleDependencyID moduleID,

lib/AST/ASTContext.cpp

@@ -2201,13 +2201,6 @@ Identifier ASTContext::getRealModuleName(Identifier key, ModuleAliasLookupOption
   return value.first;
 }
 
-namespace {
-  static StringRef pathStringFromSearchPath(
-      const SearchPathOptions::SearchPath &next) {
-    return next.Path;
-  }
-}
-
 std::vector<std::string> ASTContext::getDarwinImplicitFrameworkSearchPaths()
 const {
   assert(LangOpts.Target.isOSDarwin());

lib/AST/ModuleDependencies.cpp

@@ -888,11 +888,61 @@ void ModuleDependenciesCache::recordDependency(
 }
 
 void ModuleDependenciesCache::recordDependencies(
-    ModuleDependencyVector dependencies) {
+    ModuleDependencyVector dependencies, DiagnosticEngine &diags) {
   for (const auto &dep : dependencies) {
-    if (!hasDependency(dep.first))
+    if (hasDependency(dep.first)) {
+      if (dep.first.Kind == ModuleDependencyKind::Clang) {
+        auto priorClangModuleDetails =
+            findKnownDependency(dep.first).getAsClangModule();
+        auto newClangModuleDetails = dep.second.getAsClangModule();
+        auto priorContextHash = priorClangModuleDetails->contextHash;
+        auto newContextHash = newClangModuleDetails->contextHash;
+        if (priorContextHash != newContextHash) {
+          // This situation means that within the same scanning action, Clang
+          // Dependency Scanner has produced two different variants of the same
+          // module. This is not supposed to happen, but we are currently
+          // hunting down the rare cases where it does, seemingly due to
+          // differences in Clang Scanner direct by-name queries and transitive
+          // header lookup queries.
+          //
+          // Emit a failure diagnostic here that is hopefully more actionable
+          // for the time being.
+          diags.diagnose(SourceLoc(), diag::dependency_scan_unexpected_variant,
+                         dep.first.ModuleName);
+          diags.diagnose(
+              SourceLoc(),
+              diag::dependency_scan_unexpected_variant_context_hash_note,
+              priorContextHash, newContextHash);
+          diags.diagnose(
+              SourceLoc(),
+              diag::dependency_scan_unexpected_variant_module_map_note,
+              priorClangModuleDetails->moduleMapFile,
+              newClangModuleDetails->moduleMapFile);
+
+          auto diagnoseExtraCommandLineFlags =
+              [&diags](const ClangModuleDependencyStorage *checkModuleDetails,
+                       const ClangModuleDependencyStorage *baseModuleDetails,
+                       bool isNewlyDiscovered) -> void {
+            std::unordered_set<std::string> baseCommandLineSet(
+                baseModuleDetails->buildCommandLine.begin(),
+                baseModuleDetails->buildCommandLine.end());
+            for (const auto &checkArg : checkModuleDetails->buildCommandLine)
+              if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
+                diags.diagnose(
+                    SourceLoc(),
+                    diag::dependency_scan_unexpected_variant_extra_arg_note,
+                    isNewlyDiscovered, checkArg);
+          };
+          diagnoseExtraCommandLineFlags(priorClangModuleDetails,
+                                        newClangModuleDetails, true);
+          diagnoseExtraCommandLineFlags(newClangModuleDetails,
+                                        priorClangModuleDetails, false);
+        }
+      }
+    } else
       recordDependency(dep.first.ModuleName, dep.second);
-    if (dep.second.getKind() == ModuleDependencyKind::Clang) {
+
+    if (dep.first.Kind == ModuleDependencyKind::Clang) {
       auto clangModuleDetails = dep.second.getAsClangModule();
       addSeenClangModule(clang::tooling::dependencies::ModuleID{
           dep.first.ModuleName, clangModuleDetails->contextHash});

lib/ClangImporter/ClangModuleDependencyScanner.cpp

@@ -516,7 +516,7 @@ bool ClangImporter::getHeaderDependencies(
         [&cache](StringRef path) {
           return cache.getScanService().remapPath(path);
         });
-    cache.recordDependencies(bridgedDeps);
+    cache.recordDependencies(bridgedDeps, Impl.SwiftContext.Diags);
 
     llvm::copy(dependencies->FileDeps, std::back_inserter(headerFileInputs));
     auto bridgedDependencyIDs =

lib/DependencyScan/ModuleDependencyScanner.cpp

@@ -532,7 +532,7 @@ ModuleDependencyScanner::getNamedClangModuleDependencyInfo(
   for (const auto &dep : moduleDependencies)
     discoveredClangModules.insert(dep.first);
 
-  cache.recordDependencies(moduleDependencies);
+  cache.recordDependencies(moduleDependencies, Diagnostics);
   return cache.findDependency(moduleID);
 }
 
@@ -566,7 +566,7 @@ ModuleDependencyScanner::getNamedSwiftModuleDependencyInfo(
   if (moduleDependencies.empty())
     return std::nullopt;
 
-  cache.recordDependencies(moduleDependencies);
+  cache.recordDependencies(moduleDependencies, Diagnostics);
   return cache.findDependency(moduleName);
 }
 
@@ -927,7 +927,7 @@ ModuleDependencyScanner::resolveAllClangModuleDependencies(
           std::lock_guard<std::mutex> guard(cacheAccessLock);
           moduleLookupResult.insert_or_assign(moduleName, moduleDependencies);
           if (!moduleDependencies.empty())
-            cache.recordDependencies(moduleDependencies);
+            cache.recordDependencies(moduleDependencies, Diagnostics);
         }
       };
 
@@ -1138,7 +1138,7 @@ void ModuleDependencyScanner::resolveSwiftImportsForModule(
   ScanningThreadPool.wait();
 
   auto recordResolvedModuleImport =
-      [&cache, &moduleLookupResult, &importedSwiftDependencies,
+      [this, &cache, &moduleLookupResult, &importedSwiftDependencies,
        moduleID](const ScannerImportStatementInfo &moduleImport) {
         if (moduleID.ModuleName == moduleImport.importIdentifier)
           return;
@@ -1152,7 +1152,7 @@ void ModuleDependencyScanner::resolveSwiftImportsForModule(
         } else {
           // Cache discovered module dependencies.
           if (!lookupResult.value().empty()) {
-            cache.recordDependencies(lookupResult.value());
+            cache.recordDependencies(lookupResult.value(), Diagnostics);
             importedSwiftDependencies.insert({moduleImport.importIdentifier,
                                               lookupResult.value()[0].first.Kind});
           }
@@ -1306,7 +1306,7 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependenciesForModule(
   ScanningThreadPool.wait();
 
   // Aggregate both previously-cached and freshly-scanned module results
-  auto recordResult = [&cache, &swiftOverlayLookupResult,
+  auto recordResult = [this, &cache, &swiftOverlayLookupResult,
                        &swiftOverlayDependencies,
                        moduleID](const std::string &moduleName) {
     auto lookupResult = swiftOverlayLookupResult[moduleName];
@@ -1318,7 +1318,7 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependenciesForModule(
             {moduleName, cachedInfo.value()->getKind()});
       } else {
         // Cache discovered module dependencies.
-        cache.recordDependencies(lookupResult.value());
+        cache.recordDependencies(lookupResult.value(), Diagnostics);
         if (!lookupResult.value().empty())
           swiftOverlayDependencies.insert({moduleName, lookupResult.value()[0].first.Kind});
       }