Skip to content

Commit bdb63ed

Browse files
cachemeifyoucancachemeifyoucan
authored
Merge pull request #74519 from cachemeifyoucan/eng/PR-128876895-swift-6.0
[6.0][ScanDependencies] Make sure `canImport` resolution agrees with `import`
2 parents 0de64e0 + 59cf32f commit bdb63ed

16 files changed

+109
-53
lines changed

‎include/swift/AST/ASTContext.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,7 +1109,7 @@ class ASTContext final {
11091109
///
11101110
/// Note that even if this check succeeds, errors may still occur if the
11111111
/// module is loaded in full.
1112-
bool canImportModuleImpl(ImportPath::Module ModulePath,
1112+
bool canImportModuleImpl(ImportPath::Module ModulePath, SourceLoc loc,
11131113
llvm::VersionTuple version, bool underlyingVersion,
11141114
bool updateFailingList,
11151115
llvm::VersionTuple &foundVersion) const;
@@ -1146,7 +1146,7 @@ class ASTContext final {
11461146
///
11471147
/// Note that even if this check succeeds, errors may still occur if the
11481148
/// module is loaded in full.
1149-
bool canImportModule(ImportPath::Module ModulePath,
1149+
bool canImportModule(ImportPath::Module ModulePath, SourceLoc loc,
11501150
llvm::VersionTuple version = llvm::VersionTuple(),
11511151
bool underlyingVersion = false);
11521152

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,9 @@ ERROR(serialization_failed,none,
827827
"serialization of module %0 failed due to the errors above",
828828
(const ModuleDecl *))
829829

830+
WARNING(can_import_invalid_swiftmodule,none,
831+
"canImport() evaluated to false due to invalid swiftmodule: %0", (StringRef))
832+
830833
ERROR(serialization_load_failed,Fatal,
831834
"failed to load module '%0'", (StringRef))
832835
ERROR(module_interface_build_failed,Fatal,

‎include/swift/AST/ModuleLoader.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ class ModuleLoader {
287287
///
288288
/// If a non-null \p versionInfo is provided, the module version will be
289289
/// parsed and populated.
290-
virtual bool canImportModule(ImportPath::Module named,
290+
virtual bool canImportModule(ImportPath::Module named, SourceLoc loc,
291291
ModuleVersionInfo *versionInfo,
292292
bool isTestableImport = false) = 0;
293293

‎include/swift/ClangImporter/ClangImporter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ class ClangImporter final : public ClangModuleLoader {
227227
///
228228
/// If a non-null \p versionInfo is provided, the module version will be
229229
/// parsed and populated.
230-
virtual bool canImportModule(ImportPath::Module named,
230+
virtual bool canImportModule(ImportPath::Module named, SourceLoc loc,
231231
ModuleVersionInfo *versionInfo,
232232
bool isTestableImport = false) override;
233233

‎include/swift/Frontend/ModuleInterfaceLoader.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ class ExplicitSwiftModuleLoader: public SerializedModuleLoaderBase {
163163
bool SkipBuildingInterface, bool IsFramework,
164164
bool IsTestableDependencyLookup = false) override;
165165

166-
bool canImportModule(ImportPath::Module named,
166+
bool canImportModule(ImportPath::Module named, SourceLoc loc,
167167
ModuleVersionInfo *versionInfo,
168168
bool isTestableDependencyLookup = false) override;
169169

@@ -212,7 +212,7 @@ class ExplicitCASModuleLoader : public SerializedModuleLoaderBase {
212212
bool SkipBuildingInterface, bool IsFramework,
213213
bool IsTestableDependencyLookup = false) override;
214214

215-
bool canImportModule(ImportPath::Module named,
215+
bool canImportModule(ImportPath::Module named, SourceLoc loc,
216216
ModuleVersionInfo *versionInfo,
217217
bool isTestableDependencyLookup = false) override;
218218

‎include/swift/Sema/SourceLoader.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,10 @@ class SourceLoader : public ModuleLoader {
5959
///
6060
/// If a non-null \p versionInfo is provided, the module version will be
6161
/// parsed and populated.
62-
virtual bool canImportModule(ImportPath::Module named,
63-
ModuleVersionInfo *versionInfo,
64-
bool isTestableDependencyLookup = false) override;
62+
virtual bool
63+
canImportModule(ImportPath::Module named, SourceLoc loc,
64+
ModuleVersionInfo *versionInfo,
65+
bool isTestableDependencyLookup = false) override;
6566

6667
/// Import a module with the given module path.
6768
///

‎include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,10 @@ class SerializedModuleLoaderBase : public ModuleLoader {
209209
///
210210
/// If a non-null \p versionInfo is provided, the module version will be
211211
/// parsed and populated.
212-
virtual bool canImportModule(ImportPath::Module named,
213-
ModuleVersionInfo *versionInfo,
214-
bool isTestableDependencyLookup = false) override;
212+
virtual bool
213+
canImportModule(ImportPath::Module named, SourceLoc loc,
214+
ModuleVersionInfo *versionInfo,
215+
bool isTestableDependencyLookup = false) override;
215216

216217
/// Import a module with the given module path.
217218
///
@@ -339,7 +340,7 @@ class MemoryBufferSerializedModuleLoader : public SerializedModuleLoaderBase {
339340
public:
340341
virtual ~MemoryBufferSerializedModuleLoader();
341342

342-
bool canImportModule(ImportPath::Module named,
343+
bool canImportModule(ImportPath::Module named, SourceLoc loc,
343344
ModuleVersionInfo *versionInfo,
344345
bool isTestableDependencyLookup = false) override;
345346

‎lib/AST/ASTContext.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2403,10 +2403,11 @@ void ASTContext::addSucceededCanImportModule(
24032403
}
24042404
}
24052405

2406-
bool ASTContext::canImportModuleImpl(
2407-
ImportPath::Module ModuleName, llvm::VersionTuple version,
2408-
bool underlyingVersion, bool updateFailingList,
2409-
llvm::VersionTuple &foundVersion) const {
2406+
bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
2407+
SourceLoc loc, llvm::VersionTuple version,
2408+
bool underlyingVersion,
2409+
bool updateFailingList,
2410+
llvm::VersionTuple &foundVersion) const {
24102411
SmallString<64> FullModuleName;
24112412
ModuleName.getString(FullModuleName);
24122413
auto ModuleNameStr = FullModuleName.str().str();
@@ -2447,7 +2448,7 @@ bool ASTContext::canImportModuleImpl(
24472448

24482449
// Otherwise, ask whether any module loader can load the module.
24492450
for (auto &importer : getImpl().ModuleLoaders) {
2450-
if (importer->canImportModule(ModuleName, nullptr))
2451+
if (importer->canImportModule(ModuleName, loc, nullptr))
24512452
return true;
24522453
}
24532454

@@ -2464,7 +2465,7 @@ bool ASTContext::canImportModuleImpl(
24642465
for (auto &importer : getImpl().ModuleLoaders) {
24652466
ModuleLoader::ModuleVersionInfo versionInfo;
24662467

2467-
if (!importer->canImportModule(ModuleName, &versionInfo))
2468+
if (!importer->canImportModule(ModuleName, loc, &versionInfo))
24682469
continue; // The loader can't find the module.
24692470

24702471
if (!versionInfo.isValid())
@@ -2504,11 +2505,11 @@ void ASTContext::forEachCanImportVersionCheck(
25042505
Callback(entry.first, entry.second.Version, entry.second.UnderlyingVersion);
25052506
}
25062507

2507-
bool ASTContext::canImportModule(ImportPath::Module moduleName,
2508+
bool ASTContext::canImportModule(ImportPath::Module moduleName, SourceLoc loc,
25082509
llvm::VersionTuple version,
25092510
bool underlyingVersion) {
25102511
llvm::VersionTuple versionInfo;
2511-
if (!canImportModuleImpl(moduleName, version, underlyingVersion, true,
2512+
if (!canImportModuleImpl(moduleName, loc, version, underlyingVersion, true,
25122513
versionInfo))
25132514
return false;
25142515

@@ -2522,8 +2523,8 @@ bool ASTContext::testImportModule(ImportPath::Module ModuleName,
25222523
llvm::VersionTuple version,
25232524
bool underlyingVersion) const {
25242525
llvm::VersionTuple versionInfo;
2525-
return canImportModuleImpl(ModuleName, version, underlyingVersion, false,
2526-
versionInfo);
2526+
return canImportModuleImpl(ModuleName, SourceLoc(), version,
2527+
underlyingVersion, false, versionInfo);
25272528
}
25282529

25292530
ModuleDecl *

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2133,6 +2133,7 @@ static llvm::VersionTuple getCurrentVersionFromTBD(llvm::vfs::FileSystem &FS,
21332133
}
21342134

21352135
bool ClangImporter::canImportModule(ImportPath::Module modulePath,
2136+
SourceLoc loc,
21362137
ModuleVersionInfo *versionInfo,
21372138
bool isTestableDependencyLookup) {
21382139
// Look up the top-level module to see if it exists.

‎lib/ClangImporter/ImportType.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3512,7 +3512,7 @@ ModuleDecl *ClangImporter::Implementation::tryLoadFoundationModule() {
35123512
bool ClangImporter::Implementation::canImportFoundationModule() {
35133513
ImportPath::Module::Builder builder(SwiftContext.Id_Foundation);
35143514
auto modulePath = builder.get();
3515-
return SwiftContext.canImportModule(modulePath);
3515+
return SwiftContext.canImportModule(modulePath, SourceLoc());
35163516
}
35173517

35183518
Type ClangImporter::Implementation::getNamedSwiftType(ModuleDecl *module,

‎lib/Frontend/ModuleInterfaceLoader.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2383,7 +2383,7 @@ std::error_code ExplicitSwiftModuleLoader::findModuleFilesInDirectory(
23832383
}
23842384

23852385
bool ExplicitSwiftModuleLoader::canImportModule(
2386-
ImportPath::Module path, ModuleVersionInfo *versionInfo,
2386+
ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo,
23872387
bool isTestableDependencyLookup) {
23882388
// FIXME: Swift submodules?
23892389
if (path.hasSubmodule())
@@ -2409,7 +2409,7 @@ bool ExplicitSwiftModuleLoader::canImportModule(
24092409
auto &fs = *Ctx.SourceMgr.getFileSystem();
24102410
auto moduleBuf = fs.getBufferForFile(it->second.modulePath);
24112411
if (!moduleBuf) {
2412-
Ctx.Diags.diagnose(SourceLoc(), diag::error_opening_explicit_module_file,
2412+
Ctx.Diags.diagnose(loc, diag::error_opening_explicit_module_file,
24132413
it->second.modulePath);
24142414
return false;
24152415
}
@@ -2420,8 +2420,7 @@ bool ExplicitSwiftModuleLoader::canImportModule(
24202420
if (auto forwardingModule = ForwardingModule::load(**moduleBuf)) {
24212421
moduleBuf = fs.getBufferForFile(forwardingModule->underlyingModulePath);
24222422
if (!moduleBuf) {
2423-
Ctx.Diags.diagnose(SourceLoc(),
2424-
diag::error_opening_explicit_module_file,
2423+
Ctx.Diags.diagnose(loc, diag::error_opening_explicit_module_file,
24252424
forwardingModule->underlyingModulePath);
24262425
return false;
24272426
}
@@ -2734,7 +2733,7 @@ std::error_code ExplicitCASModuleLoader::findModuleFilesInDirectory(
27342733
}
27352734

27362735
bool ExplicitCASModuleLoader::canImportModule(
2737-
ImportPath::Module path, ModuleVersionInfo *versionInfo,
2736+
ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo,
27382737
bool isTestableDependencyLookup) {
27392738
// FIXME: Swift submodules?
27402739
if (path.hasSubmodule())
@@ -2763,12 +2762,11 @@ bool ExplicitCASModuleLoader::canImportModule(
27632762
: it->second.modulePath;
27642763
auto moduleBuf = Impl.loadFileBuffer(moduleCASID, it->second.modulePath);
27652764
if (!moduleBuf) {
2766-
Ctx.Diags.diagnose(SourceLoc(), diag::error_cas,
2767-
toString(moduleBuf.takeError()));
2765+
Ctx.Diags.diagnose(loc, diag::error_cas, toString(moduleBuf.takeError()));
27682766
return false;
27692767
}
27702768
if (!*moduleBuf) {
2771-
Ctx.Diags.diagnose(SourceLoc(), diag::error_opening_explicit_module_file,
2769+
Ctx.Diags.diagnose(loc, diag::error_opening_explicit_module_file,
27722770
it->second.modulePath);
27732771
return false;
27742772
}

‎lib/Parse/ParseIfConfig.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,8 @@ class EvaluateIfConfigCondition :
603603
}
604604
ImportPath::Module::Builder builder(Ctx, Str, /*separator=*/'.',
605605
Arg->getStartLoc());
606-
return Ctx.canImportModule(builder.get(), version, underlyingModule);
606+
return Ctx.canImportModule(builder.get(), E->getLoc(), version,
607+
underlyingModule);
607608
} else if (KindName == "hasFeature") {
608609
auto featureName = getDeclRefStr(Arg);
609610
return Ctx.LangOpts.hasFeature(featureName);

‎lib/Sema/SourceLoader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ void SourceLoader::collectVisibleTopLevelModuleNames(
7171
// TODO: Implement?
7272
}
7373

74-
bool SourceLoader::canImportModule(ImportPath::Module path,
74+
bool SourceLoader::canImportModule(ImportPath::Module path, SourceLoc loc,
7575
ModuleVersionInfo *versionInfo,
7676
bool isTestableDependencyLookup) {
7777
// FIXME: Swift submodules?

‎lib/Serialization/ScanningLoaders.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ ModuleDependencyVector SerializedModuleLoaderBase::getModuleDependencies(
295295
"Expected PlaceholderSwiftModuleScanner as the first dependency "
296296
"scanner loader.");
297297
for (auto &scanner : scanners) {
298-
if (scanner->canImportModule(modulePath, nullptr,
298+
if (scanner->canImportModule(modulePath, SourceLoc(), nullptr,
299299
isTestableDependencyLookup)) {
300300

301301
ModuleDependencyVector moduleDependnecies;

‎lib/Serialization/SerializedModuleLoader.cpp

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include "llvm/ADT/StringSet.h"
3434
#include "llvm/Support/Debug.h"
3535
#include "llvm/Support/FileSystem.h"
36+
#include "llvm/Support/VersionTuple.h"
3637
#include "llvm/TargetParser/Host.h"
3738
#include "llvm/Support/MemoryBuffer.h"
3839
#include "llvm/Support/Path.h"
@@ -1375,7 +1376,7 @@ swift::extractUserModuleVersionFromInterface(StringRef moduleInterfacePath) {
13751376
}
13761377

13771378
bool SerializedModuleLoaderBase::canImportModule(
1378-
ImportPath::Module path, ModuleVersionInfo *versionInfo,
1379+
ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo,
13791380
bool isTestableDependencyLookup) {
13801381
// FIXME: Swift submodules?
13811382
if (path.hasSubmodule())
@@ -1398,36 +1399,55 @@ bool SerializedModuleLoaderBase::canImportModule(
13981399
if (!found)
13991400
return false;
14001401

1401-
// If the caller doesn't want version info we're done.
1402-
if (!versionInfo)
1403-
return true;
1404-
1405-
assert(found);
1406-
llvm::VersionTuple swiftInterfaceVersion;
14071402
if (!moduleInterfaceSourcePath.empty()) {
1408-
swiftInterfaceVersion =
1403+
// If we found interface and version is not requested, we're done.
1404+
if (!versionInfo)
1405+
return true;
1406+
1407+
auto moduleVersion =
14091408
extractUserModuleVersionFromInterface(moduleInterfaceSourcePath);
1409+
1410+
// If version is requested and found in interface, return the version.
1411+
// Otherwise fallback to binary module handling.
1412+
if (!moduleVersion.empty()) {
1413+
versionInfo->setVersion(moduleVersion,
1414+
ModuleVersionSourceKind::SwiftInterface);
1415+
return true;
1416+
}
14101417
}
14111418

1412-
// If failing to extract the user version from the interface file, try the
1413-
// binary module format, if present.
1414-
if (swiftInterfaceVersion.empty() && moduleInputBuffer) {
1419+
if (moduleInputBuffer) {
14151420
auto metaData = serialization::validateSerializedAST(
1416-
moduleInputBuffer->getBuffer(),
1417-
Ctx.SILOpts.EnableOSSAModules,
1421+
moduleInputBuffer->getBuffer(), Ctx.SILOpts.EnableOSSAModules,
14181422
Ctx.LangOpts.SDKName);
1419-
versionInfo->setVersion(metaData.userModuleVersion,
1423+
1424+
// If we only found binary module, make sure that is valid.
1425+
if (metaData.status != serialization::Status::Valid &&
1426+
moduleInterfaceSourcePath.empty()) {
1427+
// Emit warning if the canImport check location is known.
1428+
if (loc.isValid())
1429+
Ctx.Diags.diagnose(loc, diag::can_import_invalid_swiftmodule,
1430+
moduleInputBuffer->getBufferIdentifier());
1431+
1432+
return false;
1433+
}
1434+
1435+
if (versionInfo)
1436+
versionInfo->setVersion(metaData.userModuleVersion,
1437+
ModuleVersionSourceKind::SwiftBinaryModule);
1438+
}
1439+
1440+
if (versionInfo && !versionInfo->isValid()) {
1441+
// If no version is found, set it to empty version.
1442+
versionInfo->setVersion(llvm::VersionTuple(),
14201443
ModuleVersionSourceKind::SwiftBinaryModule);
1421-
} else {
1422-
versionInfo->setVersion(swiftInterfaceVersion,
1423-
ModuleVersionSourceKind::SwiftInterface);
14241444
}
14251445

14261446
return true;
14271447
}
14281448

14291449
bool MemoryBufferSerializedModuleLoader::canImportModule(
1430-
ImportPath::Module path, ModuleVersionInfo *versionInfo,
1450+
ImportPath::Module path, SourceLoc loc, ModuleVersionInfo *versionInfo,
14311451
bool isTestableDependencyLookup) {
14321452
// FIXME: Swift submodules?
14331453
if (path.hasSubmodule())

‎test/ScanDependencies/can_import_version_mismatch.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
// RUN: cp %S/../Serialization/Inputs/too-old/Library.swiftmodule %t/include/Library.swiftmodule
4+
5+
/// No warning when there is a swiftinterface file even the binary module is invalid.
6+
// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/test.swift -module-cache-path %t/clang-module-cache \
7+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \
8+
// RUN: -o %t/deps.json -I %t/include 2>&1 | %FileCheck %s --check-prefix=NOWARN --allow-empty
9+
10+
// NOWARN-NOT: warning:
11+
12+
/// Issue a warning when only found invalid binary module.
13+
// RUN: rm %t/include/Library.swiftinterface
14+
// RUN: %target-swift-frontend -scan-dependencies -module-name Test %t/test.swift -module-cache-path %t/clang-module-cache \
15+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \
16+
// RUN: -o %t/deps.json -I %t/include 2>&1 | %FileCheck %s --check-prefix=WARN
17+
// WARN: warning: canImport() evaluated to false due to invalid swiftmodule
18+
19+
// RUN: %FileCheck %s --check-prefix=DEP --input-file=%t/deps.json
20+
// DEP-NOT: Library
21+
22+
//--- test.swift
23+
#if canImport(Library)
24+
import Library
25+
#endif
26+
27+
//--- include/Library.swiftinterface
28+
// swift-interface-format-version: 1.0
29+
// 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
30+
public func foo() {}

0 commit comments

Comments
 (0)