diff --git a/include/swift/AST/ASTContext.h b/include/swift/AST/ASTContext.h index a435ceff672bc..f0622adfcd0cf 100644 --- a/include/swift/AST/ASTContext.h +++ b/include/swift/AST/ASTContext.h @@ -1109,7 +1109,7 @@ class ASTContext final { /// /// Note that even if this check succeeds, errors may still occur if the /// module is loaded in full. - bool canImportModuleImpl(ImportPath::Module ModulePath, + bool canImportModuleImpl(ImportPath::Module ModulePath, SourceLoc loc, llvm::VersionTuple version, bool underlyingVersion, bool updateFailingList, llvm::VersionTuple &foundVersion) const; @@ -1146,7 +1146,7 @@ class ASTContext final { /// /// Note that even if this check succeeds, errors may still occur if the /// module is loaded in full. - bool canImportModule(ImportPath::Module ModulePath, + bool canImportModule(ImportPath::Module ModulePath, SourceLoc loc, llvm::VersionTuple version = llvm::VersionTuple(), bool underlyingVersion = false); diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 166ba4d17a584..ea0e8ca9a4347 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -830,6 +830,9 @@ ERROR(serialization_failed,none, "serialization of module %0 failed due to the errors above", (const ModuleDecl *)) +WARNING(can_import_invalid_swiftmodule,none, + "canImport() evaluated to false due to invalid swiftmodule: %0", (StringRef)) + ERROR(serialization_load_failed,Fatal, "failed to load module '%0'", (StringRef)) ERROR(module_interface_build_failed,Fatal, diff --git a/include/swift/AST/ModuleLoader.h b/include/swift/AST/ModuleLoader.h index 1e4f234abeb20..f811454e70f69 100644 --- a/include/swift/AST/ModuleLoader.h +++ b/include/swift/AST/ModuleLoader.h @@ -287,7 +287,7 @@ class ModuleLoader { /// /// If a non-null \p versionInfo is provided, the module version will be /// parsed and populated. - virtual bool canImportModule(ImportPath::Module named, + virtual bool canImportModule(ImportPath::Module named, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableImport = false) = 0; diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h index 3472cb4eff63b..434b6e744f356 100644 --- a/include/swift/ClangImporter/ClangImporter.h +++ b/include/swift/ClangImporter/ClangImporter.h @@ -227,7 +227,7 @@ class ClangImporter final : public ClangModuleLoader { /// /// If a non-null \p versionInfo is provided, the module version will be /// parsed and populated. - virtual bool canImportModule(ImportPath::Module named, + virtual bool canImportModule(ImportPath::Module named, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableImport = false) override; diff --git a/include/swift/Frontend/ModuleInterfaceLoader.h b/include/swift/Frontend/ModuleInterfaceLoader.h index 8b2c6449f7004..4f82b217ae3e6 100644 --- a/include/swift/Frontend/ModuleInterfaceLoader.h +++ b/include/swift/Frontend/ModuleInterfaceLoader.h @@ -163,7 +163,7 @@ class ExplicitSwiftModuleLoader: public SerializedModuleLoaderBase { bool SkipBuildingInterface, bool IsFramework, bool IsTestableDependencyLookup = false) override; - bool canImportModule(ImportPath::Module named, + bool canImportModule(ImportPath::Module named, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup = false) override; @@ -212,7 +212,7 @@ class ExplicitCASModuleLoader : public SerializedModuleLoaderBase { bool SkipBuildingInterface, bool IsFramework, bool IsTestableDependencyLookup = false) override; - bool canImportModule(ImportPath::Module named, + bool canImportModule(ImportPath::Module named, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup = false) override; diff --git a/include/swift/Sema/SourceLoader.h b/include/swift/Sema/SourceLoader.h index dd27fa0c1d09a..d899a011908fc 100644 --- a/include/swift/Sema/SourceLoader.h +++ b/include/swift/Sema/SourceLoader.h @@ -59,9 +59,10 @@ class SourceLoader : public ModuleLoader { /// /// If a non-null \p versionInfo is provided, the module version will be /// parsed and populated. - virtual bool canImportModule(ImportPath::Module named, - ModuleVersionInfo *versionInfo, - bool isTestableDependencyLookup = false) override; + virtual bool + canImportModule(ImportPath::Module named, SourceLoc loc, + ModuleVersionInfo *versionInfo, + bool isTestableDependencyLookup = false) override; /// Import a module with the given module path. /// diff --git a/include/swift/Serialization/SerializedModuleLoader.h b/include/swift/Serialization/SerializedModuleLoader.h index 433d7136edc33..d59620518a0ae 100644 --- a/include/swift/Serialization/SerializedModuleLoader.h +++ b/include/swift/Serialization/SerializedModuleLoader.h @@ -209,9 +209,10 @@ class SerializedModuleLoaderBase : public ModuleLoader { /// /// If a non-null \p versionInfo is provided, the module version will be /// parsed and populated. - virtual bool canImportModule(ImportPath::Module named, - ModuleVersionInfo *versionInfo, - bool isTestableDependencyLookup = false) override; + virtual bool + canImportModule(ImportPath::Module named, SourceLoc loc, + ModuleVersionInfo *versionInfo, + bool isTestableDependencyLookup = false) override; /// Import a module with the given module path. /// @@ -339,7 +340,7 @@ class MemoryBufferSerializedModuleLoader : public SerializedModuleLoaderBase { public: virtual ~MemoryBufferSerializedModuleLoader(); - bool canImportModule(ImportPath::Module named, + bool canImportModule(ImportPath::Module named, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup = false) override; diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 43156febacf8c..e1d4c711014e0 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -2404,10 +2404,11 @@ void ASTContext::addSucceededCanImportModule( } } -bool ASTContext::canImportModuleImpl( - ImportPath::Module ModuleName, llvm::VersionTuple version, - bool underlyingVersion, bool updateFailingList, - llvm::VersionTuple &foundVersion) const { +bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName, + SourceLoc loc, llvm::VersionTuple version, + bool underlyingVersion, + bool updateFailingList, + llvm::VersionTuple &foundVersion) const { SmallString<64> FullModuleName; ModuleName.getString(FullModuleName); auto ModuleNameStr = FullModuleName.str().str(); @@ -2448,7 +2449,7 @@ bool ASTContext::canImportModuleImpl( // Otherwise, ask whether any module loader can load the module. for (auto &importer : getImpl().ModuleLoaders) { - if (importer->canImportModule(ModuleName, nullptr)) + if (importer->canImportModule(ModuleName, loc, nullptr)) return true; } @@ -2465,7 +2466,7 @@ bool ASTContext::canImportModuleImpl( for (auto &importer : getImpl().ModuleLoaders) { ModuleLoader::ModuleVersionInfo versionInfo; - if (!importer->canImportModule(ModuleName, &versionInfo)) + if (!importer->canImportModule(ModuleName, loc, &versionInfo)) continue; // The loader can't find the module. if (!versionInfo.isValid()) @@ -2505,11 +2506,11 @@ void ASTContext::forEachCanImportVersionCheck( Callback(entry.first, entry.second.Version, entry.second.UnderlyingVersion); } -bool ASTContext::canImportModule(ImportPath::Module moduleName, +bool ASTContext::canImportModule(ImportPath::Module moduleName, SourceLoc loc, llvm::VersionTuple version, bool underlyingVersion) { llvm::VersionTuple versionInfo; - if (!canImportModuleImpl(moduleName, version, underlyingVersion, true, + if (!canImportModuleImpl(moduleName, loc, version, underlyingVersion, true, versionInfo)) return false; @@ -2523,8 +2524,8 @@ bool ASTContext::testImportModule(ImportPath::Module ModuleName, llvm::VersionTuple version, bool underlyingVersion) const { llvm::VersionTuple versionInfo; - return canImportModuleImpl(ModuleName, version, underlyingVersion, false, - versionInfo); + return canImportModuleImpl(ModuleName, SourceLoc(), version, + underlyingVersion, false, versionInfo); } ModuleDecl * diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 687a2b21045de..0fae5c4dafac7 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -2147,6 +2147,7 @@ static llvm::VersionTuple getCurrentVersionFromTBD(llvm::vfs::FileSystem &FS, } bool ClangImporter::canImportModule(ImportPath::Module modulePath, + SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup) { // Look up the top-level module to see if it exists. diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp index a9e55610d5d2e..9310003f6d3c7 100644 --- a/lib/ClangImporter/ImportType.cpp +++ b/lib/ClangImporter/ImportType.cpp @@ -3513,7 +3513,7 @@ ModuleDecl *ClangImporter::Implementation::tryLoadFoundationModule() { bool ClangImporter::Implementation::canImportFoundationModule() { ImportPath::Module::Builder builder(SwiftContext.Id_Foundation); auto modulePath = builder.get(); - return SwiftContext.canImportModule(modulePath); + return SwiftContext.canImportModule(modulePath, SourceLoc()); } Type ClangImporter::Implementation::getNamedSwiftType(ModuleDecl *module, diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp index 2b73ce3e38c60..38469ce00bd02 100644 --- a/lib/Frontend/ModuleInterfaceLoader.cpp +++ b/lib/Frontend/ModuleInterfaceLoader.cpp @@ -2385,7 +2385,7 @@ std::error_code ExplicitSwiftModuleLoader::findModuleFilesInDirectory( } bool ExplicitSwiftModuleLoader::canImportModule( - ImportPath::Module path, ModuleVersionInfo *versionInfo, + ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup) { // FIXME: Swift submodules? if (path.hasSubmodule()) @@ -2411,7 +2411,7 @@ bool ExplicitSwiftModuleLoader::canImportModule( auto &fs = *Ctx.SourceMgr.getFileSystem(); auto moduleBuf = fs.getBufferForFile(it->second.modulePath); if (!moduleBuf) { - Ctx.Diags.diagnose(SourceLoc(), diag::error_opening_explicit_module_file, + Ctx.Diags.diagnose(loc, diag::error_opening_explicit_module_file, it->second.modulePath); return false; } @@ -2422,8 +2422,7 @@ bool ExplicitSwiftModuleLoader::canImportModule( if (auto forwardingModule = ForwardingModule::load(**moduleBuf)) { moduleBuf = fs.getBufferForFile(forwardingModule->underlyingModulePath); if (!moduleBuf) { - Ctx.Diags.diagnose(SourceLoc(), - diag::error_opening_explicit_module_file, + Ctx.Diags.diagnose(loc, diag::error_opening_explicit_module_file, forwardingModule->underlyingModulePath); return false; } @@ -2736,7 +2735,7 @@ std::error_code ExplicitCASModuleLoader::findModuleFilesInDirectory( } bool ExplicitCASModuleLoader::canImportModule( - ImportPath::Module path, ModuleVersionInfo *versionInfo, + ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup) { // FIXME: Swift submodules? if (path.hasSubmodule()) @@ -2765,12 +2764,11 @@ bool ExplicitCASModuleLoader::canImportModule( : it->second.modulePath; auto moduleBuf = Impl.loadFileBuffer(moduleCASID, it->second.modulePath); if (!moduleBuf) { - Ctx.Diags.diagnose(SourceLoc(), diag::error_cas, - toString(moduleBuf.takeError())); + Ctx.Diags.diagnose(loc, diag::error_cas, toString(moduleBuf.takeError())); return false; } if (!*moduleBuf) { - Ctx.Diags.diagnose(SourceLoc(), diag::error_opening_explicit_module_file, + Ctx.Diags.diagnose(loc, diag::error_opening_explicit_module_file, it->second.modulePath); return false; } diff --git a/lib/Parse/ParseIfConfig.cpp b/lib/Parse/ParseIfConfig.cpp index d768f9f43f3db..09bf46eb291da 100644 --- a/lib/Parse/ParseIfConfig.cpp +++ b/lib/Parse/ParseIfConfig.cpp @@ -603,7 +603,8 @@ class EvaluateIfConfigCondition : } ImportPath::Module::Builder builder(Ctx, Str, /*separator=*/'.', Arg->getStartLoc()); - return Ctx.canImportModule(builder.get(), version, underlyingModule); + return Ctx.canImportModule(builder.get(), E->getLoc(), version, + underlyingModule); } else if (KindName == "hasFeature") { auto featureName = getDeclRefStr(Arg); return Ctx.LangOpts.hasFeature(featureName); diff --git a/lib/Sema/SourceLoader.cpp b/lib/Sema/SourceLoader.cpp index f277b1dc1a1eb..5efd2ba259c04 100644 --- a/lib/Sema/SourceLoader.cpp +++ b/lib/Sema/SourceLoader.cpp @@ -71,7 +71,7 @@ void SourceLoader::collectVisibleTopLevelModuleNames( // TODO: Implement? } -bool SourceLoader::canImportModule(ImportPath::Module path, +bool SourceLoader::canImportModule(ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup) { // FIXME: Swift submodules? diff --git a/lib/Serialization/ScanningLoaders.cpp b/lib/Serialization/ScanningLoaders.cpp index be657331dfecf..87ecfa8ca118a 100644 --- a/lib/Serialization/ScanningLoaders.cpp +++ b/lib/Serialization/ScanningLoaders.cpp @@ -311,7 +311,7 @@ ModuleDependencyVector SerializedModuleLoaderBase::getModuleDependencies( "Expected PlaceholderSwiftModuleScanner as the first dependency " "scanner loader."); for (auto &scanner : scanners) { - if (scanner->canImportModule(modulePath, nullptr, + if (scanner->canImportModule(modulePath, SourceLoc(), nullptr, isTestableDependencyLookup)) { ModuleDependencyVector moduleDependnecies; diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index 006fb055bd70c..e8ba75d7b1490 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -33,6 +33,7 @@ #include "llvm/ADT/StringSet.h" #include "llvm/Support/Debug.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/VersionTuple.h" #include "llvm/TargetParser/Host.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" @@ -1391,7 +1392,7 @@ swift::extractUserModuleVersionFromInterface(StringRef moduleInterfacePath) { } bool SerializedModuleLoaderBase::canImportModule( - ImportPath::Module path, ModuleVersionInfo *versionInfo, + ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup) { // FIXME: Swift submodules? if (path.hasSubmodule()) @@ -1414,36 +1415,55 @@ bool SerializedModuleLoaderBase::canImportModule( if (!found) return false; - // If the caller doesn't want version info we're done. - if (!versionInfo) - return true; - - assert(found); - llvm::VersionTuple swiftInterfaceVersion; if (!moduleInterfaceSourcePath.empty()) { - swiftInterfaceVersion = + // If we found interface and version is not requested, we're done. + if (!versionInfo) + return true; + + auto moduleVersion = extractUserModuleVersionFromInterface(moduleInterfaceSourcePath); + + // If version is requested and found in interface, return the version. + // Otherwise fallback to binary module handling. + if (!moduleVersion.empty()) { + versionInfo->setVersion(moduleVersion, + ModuleVersionSourceKind::SwiftInterface); + return true; + } } - // If failing to extract the user version from the interface file, try the - // binary module format, if present. - if (swiftInterfaceVersion.empty() && moduleInputBuffer) { + if (moduleInputBuffer) { auto metaData = serialization::validateSerializedAST( - moduleInputBuffer->getBuffer(), - Ctx.SILOpts.EnableOSSAModules, + moduleInputBuffer->getBuffer(), Ctx.SILOpts.EnableOSSAModules, Ctx.LangOpts.SDKName); - versionInfo->setVersion(metaData.userModuleVersion, + + // If we only found binary module, make sure that is valid. + if (metaData.status != serialization::Status::Valid && + moduleInterfaceSourcePath.empty()) { + // Emit warning if the canImport check location is known. + if (loc.isValid()) + Ctx.Diags.diagnose(loc, diag::can_import_invalid_swiftmodule, + moduleInputBuffer->getBufferIdentifier()); + + return false; + } + + if (versionInfo) + versionInfo->setVersion(metaData.userModuleVersion, + ModuleVersionSourceKind::SwiftBinaryModule); + } + + if (versionInfo && !versionInfo->isValid()) { + // If no version is found, set it to empty version. + versionInfo->setVersion(llvm::VersionTuple(), ModuleVersionSourceKind::SwiftBinaryModule); - } else { - versionInfo->setVersion(swiftInterfaceVersion, - ModuleVersionSourceKind::SwiftInterface); } return true; } bool MemoryBufferSerializedModuleLoader::canImportModule( - ImportPath::Module path, ModuleVersionInfo *versionInfo, + ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo, bool isTestableDependencyLookup) { // FIXME: Swift submodules? if (path.hasSubmodule()) diff --git a/test/ScanDependencies/can_import_version_mismatch.swift b/test/ScanDependencies/can_import_version_mismatch.swift new file mode 100644 index 0000000000000..3b1133c9cfe64 --- /dev/null +++ b/test/ScanDependencies/can_import_version_mismatch.swift @@ -0,0 +1,30 @@ +// RUN: %empty-directory(%t) +// RUN: split-file %s %t +// RUN: cp %S/../Serialization/Inputs/too-old/Library.swiftmodule %t/include/Library.swiftmodule + +/// No warning when there is a swiftinterface file even the binary module is invalid. +// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/test.swift -module-cache-path %t/clang-module-cache \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \ +// RUN: -o %t/deps.json -I %t/include 2>&1 | %FileCheck %s --check-prefix=NOWARN --allow-empty + +// NOWARN-NOT: warning: + +/// Issue a warning when only found invalid binary module. +// RUN: rm %t/include/Library.swiftinterface +// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/test.swift -module-cache-path %t/clang-module-cache \ +// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \ +// RUN: -o %t/deps.json -I %t/include 2>&1 | %FileCheck %s --check-prefix=WARN +// WARN: warning: canImport() evaluated to false due to invalid swiftmodule + +// RUN: %FileCheck %s --check-prefix=DEP --input-file=%t/deps.json +// DEP-NOT: Library + +//--- test.swift +#if canImport(Library) +import Library +#endif + +//--- include/Library.swiftinterface +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name Library -O -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib -user-module-version 1.0 +public func foo() {}