[Dependency Scanning] Add support for diagnosing invalid architecture swift binary module candidates

Refactor 'maybeDiagnoseTargetMismatch' to separately collect mismatching target variant modules in 'identifyArchitectureVariants' and rename it to 'handlePossibleTargetMismatch'.

Prior uses of 'maybeDiagnoseTargetMismatch' will continue diagnosing errors/warnings on only discovering incompatible swift binary module target variants.

A new overload of 'handlePossibleTargetMismatch', in 'SwiftModuleScanner', instead collects it as a discovered incompatible candidate, for diagnosis downstream.

Author: artemcm

include/swift/Serialization/ScanningLoaders.h

@@ -70,6 +70,12 @@ class SwiftModuleScanner : public SerializedModuleLoaderBase {
                        ModuleVersionInfo *versionInfo,
                        bool isTestableImport) override;
 
+  bool handlePossibleTargetMismatch(
+      SourceLoc sourceLocation,
+      StringRef moduleName,
+      const SerializedModuleBaseName &BaseName,
+      bool isCanImportLookup) override;
+
   virtual void collectVisibleTopLevelModuleNames(
       SmallVectorImpl<Identifier> &names) const override {
     llvm_unreachable("Not used");

include/swift/Serialization/SerializedModuleLoader.h

@@ -151,14 +151,23 @@ class SerializedModuleLoaderBase : public ModuleLoader {
   /// to list the architectures that \e are present.
   ///
   /// \returns true if an error diagnostic was emitted
-  virtual bool maybeDiagnoseTargetMismatch(
+  virtual bool handlePossibleTargetMismatch(
       SourceLoc sourceLocation,
       StringRef moduleName,
-      const SerializedModuleBaseName &BaseName,
+      const SerializedModuleBaseName &baseName,
       bool isCanImportLookup) {
     return false;
   }
 
+  /// Assuming the \c baseName is a target-specific Swift module path,
+  /// for a missing target variant, collect all adjacent binary module
+  /// files to build a list of discovered modules for incompatible
+  /// architectures.
+  static void identifyArchitectureVariants(
+       ASTContext &Ctx,
+       const SerializedModuleBaseName &baseName,
+       std::vector<std::string> &incompatibleArchModules);
+
   /// Determines if the provided path is a cached artifact for dependency
   /// tracking purposes.
   virtual bool isCached(StringRef DepPath) {
@@ -282,7 +291,7 @@ class ImplicitSerializedModuleLoader : public SerializedModuleLoaderBase {
       bool IsCanImportLookup, bool IsFramework,
       bool IsTestableDependencyLookup = false) override;
 
-  bool maybeDiagnoseTargetMismatch(
+  bool handlePossibleTargetMismatch(
       SourceLoc sourceLocation,
       StringRef moduleName,
       const SerializedModuleBaseName &BaseName,

lib/Serialization/ScanningLoaders.cpp

@@ -107,6 +107,21 @@ bool SwiftModuleScanner::canImportModule(
       path, loc, versionInfo, isTestableDependencyLookup);
 }
 
+bool SwiftModuleScanner::handlePossibleTargetMismatch(
+    SourceLoc sourceLocation, StringRef moduleName,
+    const SerializedModuleBaseName &absoluteBaseName,
+    bool isCanImportLookup) {
+  std::vector<std::string> foundIncompatibleArchModules;
+  identifyArchitectureVariants(Ctx, absoluteBaseName,
+                               foundIncompatibleArchModules);
+
+  for (const auto &modulePath : foundIncompatibleArchModules)
+    incompatibleCandidates.push_back({modulePath,
+                                      "invalid architecture"});
+
+  return false;
+}
+
 static std::vector<std::string> getCompiledCandidates(ASTContext &ctx,
                                                       StringRef moduleName,
                                                       StringRef interfacePath) {

lib/Serialization/SerializedModuleLoader.cpp

@@ -577,47 +577,58 @@ std::error_code ImplicitSerializedModuleLoader::findModuleFilesInDirectory(
   return std::error_code();
 }
 
-bool ImplicitSerializedModuleLoader::maybeDiagnoseTargetMismatch(
-    SourceLoc sourceLocation, StringRef moduleName,
-    const SerializedModuleBaseName &absoluteBaseName,
-    bool isCanImportLookup) {
+void SerializedModuleLoaderBase::identifyArchitectureVariants(
+    ASTContext &Ctx, const SerializedModuleBaseName &absoluteBaseName,
+    std::vector<std::string> &incompatibleArchModules) {
   llvm::vfs::FileSystem &fs = *Ctx.SourceMgr.getFileSystem();
 
-  // Get the last component of the base name, which is the target-specific one.
-  auto target = llvm::sys::path::filename(absoluteBaseName.baseName);
-
   // Strip off the last component to get the .swiftmodule folder.
   auto dir = absoluteBaseName.baseName;
   llvm::sys::path::remove_filename(dir);
 
   std::error_code errorCode;
-  std::string foundArchs;
   for (llvm::vfs::directory_iterator directoryIterator =
            fs.dir_begin(dir, errorCode), endIterator;
        directoryIterator != endIterator;
        directoryIterator.increment(errorCode)) {
     if (errorCode)
-      return false;
+      continue;
     StringRef filePath = directoryIterator->path();
     StringRef extension = llvm::sys::path::extension(filePath);
     if (file_types::lookupTypeForExtension(extension) ==
           file_types::TY_SwiftModuleFile) {
-      if (!foundArchs.empty())
-        foundArchs += ", ";
-      foundArchs += llvm::sys::path::stem(filePath).str();
+      incompatibleArchModules.push_back(filePath.str());
     }
   }
+}
 
-  if (foundArchs.empty()) {
-    // Maybe this swiftmodule directory only contains swiftinterfaces, or
-    // maybe something else is going on. Regardless, we shouldn't emit a
-    // possibly incorrect diagnostic.
+bool ImplicitSerializedModuleLoader::handlePossibleTargetMismatch(
+    SourceLoc sourceLocation, StringRef moduleName,
+    const SerializedModuleBaseName &absoluteBaseName,
+    bool isCanImportLookup) {
+  std::string foundArchs;
+  std::vector<std::string> foundIncompatibleArchModules;
+  identifyArchitectureVariants(Ctx, absoluteBaseName,
+                               foundIncompatibleArchModules);
+
+  // Maybe this swiftmodule directory only contains swiftinterfaces, or
+  // maybe something else is going on. Regardless, we shouldn't emit a
+  // possibly incorrect diagnostic.
+  if (foundIncompatibleArchModules.empty())
     return false;
+
+  // Generate combined list of discovered architectures
+  // for the diagnostic
+  for (const auto &modulePath : foundIncompatibleArchModules) {
+    if (!foundArchs.empty())
+      foundArchs += ", ";
+    foundArchs += llvm::sys::path::stem(modulePath).str();
   }
 
   Ctx.Diags
-      .diagnose(sourceLocation, diag::sema_no_import_target, moduleName, target,
-                foundArchs, dir)
+      .diagnose(sourceLocation, diag::sema_no_import_target, moduleName,
+                llvm::sys::path::filename(absoluteBaseName.baseName),
+                foundArchs, absoluteBaseName.baseName)
       .limitBehaviorIf(isCanImportLookup, DiagnosticBehavior::Warning);
   return !isCanImportLookup;
 }
@@ -782,8 +793,8 @@ bool SerializedModuleLoaderBase::findModule(
     // We can only get here if all targetFileNamePairs failed with
     // 'std::errc::no_such_file_or_directory'.
     if (firstAbsoluteBaseName &&
-        maybeDiagnoseTargetMismatch(moduleID.Loc, moduleName,
-                                    *firstAbsoluteBaseName, isCanImportLookup))
+        handlePossibleTargetMismatch(moduleID.Loc, moduleName,
+                                     *firstAbsoluteBaseName, isCanImportLookup))
       return SearchResult::Error;
 
     return SearchResult::NotFound;

test/ScanDependencies/incompatible-arch.swift

@@ -0,0 +1,12 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/module-cache)
+// RUN: %empty-directory(%t/inputs/Foo.swiftmodule)
+// RUN: touch %t/inputs/Foo.swiftmodule/i387.swiftmodule
+// RUN: touch %t/inputs/Foo.swiftmodule/ppc65.swiftmodule
+
+// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %s -o %t/deps.json -I %t/inputs -diagnostic-style llvm -scanner-module-validation 2>&1 | %FileCheck %s
+
+// CHECK: error: unable to resolve Swift module dependency to a compatible module: 'Foo'
+// CHECK-DAG: note: found incompatible module '{{.*}}Foo.swiftmodule{{/|\\}}ppc65.swiftmodule': invalid architecture
+// CHECK-DAG: note: found incompatible module '{{.*}}Foo.swiftmodule{{/|\\}}i387.swiftmodule': invalid architecture
+import Foo