From 550538f3b5cf0a62a0340e2fb9b6d64389a9d5a9 Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Wed, 15 Jan 2025 16:53:05 -0800 Subject: [PATCH] [Dependency Scanning] Emit a warning when failing to load a binary module of a non-resilient dependency This failure will most-likely result in the dependency query failure which will fail the scan. It will be helpful if the scanner emitted diagnostic for each such module it rejected to explain the reason why. Resolves rdar://142906530 --- include/swift/AST/DiagnosticsSema.def | 10 +++- .../Serialization/SerializedModuleLoader.h | 8 ++- lib/Frontend/ModuleInterfaceLoader.cpp | 43 +------------- lib/Serialization/ScanningLoaders.cpp | 9 ++- lib/Serialization/SerializedModuleLoader.cpp | 57 +++++++++++++++++-- .../invalid_binary_module_only.swift | 13 +++++ 6 files changed, 89 insertions(+), 51 deletions(-) create mode 100644 test/ScanDependencies/invalid_binary_module_only.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 2b77c58419fab..19b1a5c1bab6e 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1230,8 +1230,14 @@ REMARK(module_api_import_aliases,none, "%select{, which reexports definition from %2|}3", (const Decl *, ModuleDecl *, ModuleDecl *, bool)) -REMARK(skip_module_invalid,none,"skip invalid swiftmodule: %0", (StringRef)) -WARNING(skip_module_not_testable,none,"ignore swiftmodule built without '-enable-testing': %0", (StringRef)) +REMARK(dependency_scan_skip_module_invalid,none, + "module file '%0' skipped by the dependency scan because it is " + "incompatible with this Swift compiler: %1", (StringRef, StringRef)) +WARNING(skip_module_not_testable,none, + "ignore swiftmodule built without '-enable-testing': %0", (StringRef)) +WARNING(dependency_scan_module_incompatible, none, + "module file '%0' is incompatible with this Swift compiler: %1", + (StringRef, StringRef)) REMARK(macro_loaded,none, "loaded macro implementation module %0 from " diff --git a/include/swift/Serialization/SerializedModuleLoader.h b/include/swift/Serialization/SerializedModuleLoader.h index a80479f9589b1..e7a40b1cad171 100644 --- a/include/swift/Serialization/SerializedModuleLoader.h +++ b/include/swift/Serialization/SerializedModuleLoader.h @@ -18,6 +18,7 @@ #include "swift/AST/ModuleDependencies.h" #include "swift/AST/ModuleLoader.h" #include "swift/AST/SearchPathOptions.h" +#include "swift/Serialization/Validation.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/PrefixMapper.h" #include "llvm/TargetParser/Triple.h" @@ -165,7 +166,8 @@ class SerializedModuleLoaderBase : public ModuleLoader { /// Scan the given serialized module file to determine dependencies. llvm::ErrorOr - scanModuleFile(Twine modulePath, bool isFramework, bool isTestableImport); + scanModuleFile(Twine modulePath, bool isFramework, + bool isTestableImport, bool isCandidateForTextualModule); struct BinaryModuleImports { llvm::StringSet<> moduleImports; @@ -267,6 +269,10 @@ class SerializedModuleLoaderBase : public ModuleLoader { InterfaceSubContextDelegate &delegate, llvm::PrefixMapper *mapper, bool isTestableImport) override; + + /// A textual reason why the compiler rejected a binary module load + /// attempt with a given status, to be used for diagnostic output. + static std::optional invalidModuleReason(serialization::Status status); }; /// Imports serialized Swift modules into an ASTContext. diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp index 9516a6e027e1e..accf69b750d4e 100644 --- a/lib/Frontend/ModuleInterfaceLoader.cpp +++ b/lib/Frontend/ModuleInterfaceLoader.cpp @@ -325,45 +325,6 @@ struct ModuleRebuildInfo { return false; } - const char *invalidModuleReason(serialization::Status status) { - using namespace serialization; - switch (status) { - case Status::FormatTooOld: - return "compiled with an older version of the compiler"; - case Status::FormatTooNew: - return "compiled with a newer version of the compiler"; - case Status::RevisionIncompatible: - return "compiled with a different version of the compiler"; - case Status::ChannelIncompatible: - return "compiled for a different distribution channel"; - case Status::NotInOSSA: - return "module was not built with OSSA"; - case Status::MissingDependency: - return "missing dependency"; - case Status::MissingUnderlyingModule: - return "missing underlying module"; - case Status::CircularDependency: - return "circular dependency"; - case Status::FailedToLoadBridgingHeader: - return "failed to load bridging header"; - case Status::Malformed: - return "malformed"; - case Status::MalformedDocumentation: - return "malformed documentation"; - case Status::NameMismatch: - return "name mismatch"; - case Status::TargetIncompatible: - return "compiled for a different target platform"; - case Status::TargetTooNew: - return "target platform newer than current platform"; - case Status::SDKMismatch: - return "SDK does not match"; - case Status::Valid: - return nullptr; - } - llvm_unreachable("bad status"); - } - /// Emits a diagnostic for all out-of-date compiled or forwarding modules /// encountered while trying to load a module. template @@ -406,9 +367,9 @@ struct ModuleRebuildInfo { // If there was a compiled module that wasn't able to be read, diagnose // the reason we couldn't read it. if (auto status = mod.serializationStatus) { - if (auto reason = invalidModuleReason(*status)) { + if (auto reason = SerializedModuleLoaderBase::invalidModuleReason(*status)) { diags.diagnose(loc, diag::compiled_module_invalid_reason, - mod.path, reason); + mod.path, reason.value()); } else { diags.diagnose(loc, diag::compiled_module_invalid, mod.path); } diff --git a/lib/Serialization/ScanningLoaders.cpp b/lib/Serialization/ScanningLoaders.cpp index a9fd69cad7de1..19cfe2a6200f6 100644 --- a/lib/Serialization/ScanningLoaders.cpp +++ b/lib/Serialization/ScanningLoaders.cpp @@ -61,7 +61,9 @@ std::error_code SwiftModuleScanner::findModuleFilesInDirectory( if (fs.exists(ModPath)) { // The module file will be loaded directly. auto dependencies = - scanModuleFile(ModPath, IsFramework, isTestableDependencyLookup); + scanModuleFile(ModPath, IsFramework, + isTestableDependencyLookup, + /* isCandidateForTextualModule */ false); if (dependencies) { this->dependencies = std::move(dependencies.get()); return std::error_code(); @@ -164,8 +166,9 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath, Ctx.SearchPathOpts.ScannerModuleValidation) { assert(compiledCandidates.size() == 1 && "Should only have 1 candidate module"); - auto BinaryDep = scanModuleFile(compiledCandidates[0], isFramework, - isTestableImport); + auto BinaryDep = scanModuleFile(compiledCandidates[0], + isFramework, isTestableImport, + /* isCandidateForTextualModule */ true); if (BinaryDep) { Result = *BinaryDep; return std::error_code(); diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index b72df8d30ef57..7913586c6af07 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -304,6 +304,45 @@ SerializedModuleLoaderBase::getModuleName(ASTContext &Ctx, StringRef modulePath, return ModuleFile::getModuleName(Ctx, modulePath, Name); } +std::optional SerializedModuleLoaderBase::invalidModuleReason(serialization::Status status) { + using namespace serialization; + switch (status) { + case Status::FormatTooOld: + return "compiled with an older version of the compiler"; + case Status::FormatTooNew: + return "compiled with a newer version of the compiler"; + case Status::RevisionIncompatible: + return "compiled with a different version of the compiler"; + case Status::ChannelIncompatible: + return "compiled for a different distribution channel"; + case Status::NotInOSSA: + return "module was not built with OSSA"; + case Status::MissingDependency: + return "missing dependency"; + case Status::MissingUnderlyingModule: + return "missing underlying module"; + case Status::CircularDependency: + return "circular dependency"; + case Status::FailedToLoadBridgingHeader: + return "failed to load bridging header"; + case Status::Malformed: + return "malformed"; + case Status::MalformedDocumentation: + return "malformed documentation"; + case Status::NameMismatch: + return "name mismatch"; + case Status::TargetIncompatible: + return "compiled for a different target platform"; + case Status::TargetTooNew: + return "target platform newer than current platform"; + case Status::SDKMismatch: + return "SDK does not match"; + case Status::Valid: + return std::nullopt; + } + llvm_unreachable("bad status"); +} + llvm::ErrorOr> SerializedModuleLoaderBase::getMatchingPackageOnlyImportsOfModule( Twine modulePath, bool isFramework, bool isRequiredOSSAModules, @@ -504,7 +543,8 @@ SerializedModuleLoaderBase::resolveMacroPlugin(const ExternalMacroPlugin ¯o, llvm::ErrorOr SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework, - bool isTestableImport) { + bool isTestableImport, + bool isCandidateForTextualModule) { const std::string moduleDocPath; const std::string sourceInfoPath; @@ -521,10 +561,19 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework, if (Ctx.SearchPathOpts.ScannerModuleValidation) { // If failed to load, just ignore and return do not found. - if (loadInfo.status != serialization::Status::Valid) { + if (auto loadFailureReason = invalidModuleReason(loadInfo.status)) { + // If no textual interface was found, then for this dependency + // scanning query this was *the* module discovered, which means + // it would be helpful to let the user know why the scanner + // was not able to use it because the scan will ultimately fail to + // resolve this dependency due to this incompatibility. + if (!isCandidateForTextualModule) + Ctx.Diags.diagnose(SourceLoc(), diag::dependency_scan_module_incompatible, + modulePath.str(), loadFailureReason.value()); + if (Ctx.LangOpts.EnableModuleLoadingRemarks) - Ctx.Diags.diagnose(SourceLoc(), diag::skip_module_invalid, - modulePath.str()); + Ctx.Diags.diagnose(SourceLoc(), diag::dependency_scan_skip_module_invalid, + modulePath.str(), loadFailureReason.value()); return std::make_error_code(std::errc::no_such_file_or_directory); } diff --git a/test/ScanDependencies/invalid_binary_module_only.swift b/test/ScanDependencies/invalid_binary_module_only.swift new file mode 100644 index 0000000000000..a53a7c164d77c --- /dev/null +++ b/test/ScanDependencies/invalid_binary_module_only.swift @@ -0,0 +1,13 @@ +// RUN: %empty-directory(%t) +// RUN: mkdir -p %t/clang-module-cache +// RUN: mkdir -p %t/moduleInputs + +// RUN: echo "Not Really a module" >> %t/moduleInputs/FooBar.swiftmodule + +// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %t/moduleInputs -diagnostic-style llvm -scanner-module-validation 2>&1 | %FileCheck %s + +import FooBar + +// CHECK: warning: module file '{{.*}}{{/|\\}}moduleInputs{{/|\\}}FooBar.swiftmodule' is incompatible with this Swift compiler: malformed +// CHECK: error: Unable to find module dependency: 'FooBar' +