averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-14 20:36:38 +01:00
Code Issues Packages Projects Releases Wiki Activity

[Dependency Scanning] Always record best version of discovered 'canImport'-ed modules

Browse Source 
Suppose module 'Foo' exists in the search paths and specifies user module version '1.0'.

If the first encountered 'canImport' query is unversioned:
...

Followed by a versioned one:
...

The success of the first check will record an unversioned successful canImport, which will cause the second check to evaluate to 'true', which is incorrect.

This change causes even unversioned 'canImport' checks to track and record the discovered user module version.
This commit is contained in:
Artem Chikin
2025-03-31 12:25:45 -07:00
parent d2ea34c0bd
commit c9ba79c8da
9 changed files with 163 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
include/swift/AST/ASTContext.h
View File

@@ -1185,11 +1185,13 @@ public:
bool canImportModuleImpl(ImportPath::Module ModulePath, SourceLoc loc,
llvm::VersionTuple version, bool underlyingVersion,
bool isSourceCanImport,
llvm::VersionTuple &foundVersion) const;
llvm::VersionTuple &foundVersion,
llvm::VersionTuple &foundUnderlyingClangVersion) const;
/// Add successful canImport modules.
void addSucceededCanImportModule(StringRef moduleName, bool underlyingVersion,
const llvm::VersionTuple &versionInfo);
void addSucceededCanImportModule(StringRef moduleName,
const llvm::VersionTuple &versionInfo,
const llvm::VersionTuple &underlyingVersionInfo);
public:
namelookup::ImportCache &getImportCache() const;

3
include/swift/AST/DiagnosticsSema.def
View File

@@ -1272,7 +1272,8 @@ REMARK(explicit_interface_build_skipped,none,
(StringRef))
WARNING(cannot_find_project_version,none,
"cannot find user version number for %0 module '%1'; version number ignored", (StringRef, StringRef))
"cannot find user version number for%select{| Clang}1 module '%0';"
" version number ignored", (StringRef, bool))
// Operator decls
ERROR(ambiguous_operator_decls,none,

204
lib/AST/ASTContext.cpp
View File

@@ -847,18 +847,16 @@ ASTContext::ASTContext(
registerNameLookupRequestFunctions(evaluator);
// Register canImport module info.
for (auto &info: SearchPathOpts.CanImportModuleInfo) {
addSucceededCanImportModule(info.ModuleName, false, info.Version);
addSucceededCanImportModule(info.ModuleName, true, info.UnderlyingVersion);
}
for (auto &info: SearchPathOpts.CanImportModuleInfo)
addSucceededCanImportModule(info.ModuleName, info.Version, info.UnderlyingVersion);
// Provide a default OnDiskOutputBackend if user didn't supply one.
if (!OutputBackend)
OutputBackend = llvm::makeIntrusiveRefCnt<llvm::vfs::OnDiskOutputBackend>();
// Insert all block list config paths.
for (auto path: langOpts.BlocklistConfigFilePaths) {
for (auto path: langOpts.BlocklistConfigFilePaths)
blockListConfig.addConfigureFilePath(path);
}
}
void ASTContext::Implementation::dump(llvm::raw_ostream &os) const {
@@ -2648,34 +2646,25 @@ static bool isClangModuleVersion(const ModuleLoader::ModuleVersionInfo &info) {
}
}
static StringRef
getModuleVersionKindString(const ModuleLoader::ModuleVersionInfo &info) {
switch (info.getSourceKind()) {
case ModuleLoader::ModuleVersionSourceKind::ClangModuleTBD:
return "Clang";
case ModuleLoader::ModuleVersionSourceKind::SwiftBinaryModule:
case ModuleLoader::ModuleVersionSourceKind::SwiftInterface:
return "Swift";
}
}
void ASTContext::addSucceededCanImportModule(
StringRef moduleName, bool underlyingVersion,
const llvm::VersionTuple &versionInfo) {
StringRef moduleName,
const llvm::VersionTuple &versionInfo,
const llvm::VersionTuple &underlyingVersionInfo) {
// We have previously recorded a successful canImport
// information for this module.
if (CanImportModuleVersions.count(moduleName.str()))
return;
auto &entry = CanImportModuleVersions[moduleName.str()];
if (!versionInfo.empty()) {
if (underlyingVersion)
entry.UnderlyingVersion = versionInfo;
else
entry.Version = versionInfo;
}
entry.Version = versionInfo;
entry.UnderlyingVersion = underlyingVersionInfo;
}
bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
SourceLoc loc, llvm::VersionTuple version,
bool underlyingVersion,
bool isSourceCanImport,
llvm::VersionTuple &foundVersion) const {
bool ASTContext::canImportModuleImpl(
ImportPath::Module ModuleName, SourceLoc loc, llvm::VersionTuple version,
bool underlyingVersion, bool isSourceCanImport,
llvm::VersionTuple &foundVersion,
llvm::VersionTuple &foundUnderlyingClangVersion) const {
SmallString<64> FullModuleName;
ModuleName.getString(FullModuleName);
auto ModuleNameStr = FullModuleName.str().str();
@@ -2684,6 +2673,20 @@ bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
if (FailedModuleImportNames.count(ModuleNameStr))
return false;
auto missingVersion = [this, &loc, &ModuleName,
&underlyingVersion]() -> bool {
// The module version could not be parsed from the preferred source for
// this query. Diagnose and return `true` to indicate that the unversioned module
// will satisfy the query.
auto mID = ModuleName[0];
auto diagLoc = mID.Loc;
if (mID.Loc.isInvalid())
diagLoc = loc;
Diags.diagnose(diagLoc, diag::cannot_find_project_version, mID.Item.str(),
underlyingVersion);
return true;
};
// If this module has already been checked or there is information for the
// module from commandline, use that information instead of loading the
// module.
@@ -2692,26 +2695,93 @@ bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
if (version.empty())
return true;
if (underlyingVersion)
return Found->second.UnderlyingVersion.empty()
? true
: version <= Found->second.UnderlyingVersion;
const auto &foundComparisonVersion = underlyingVersion
? Found->second.UnderlyingVersion
: Found->second.Version;
if (!foundComparisonVersion.empty())
return version <= foundComparisonVersion;
else
return Found->second.Version.empty()
? true
: version <= Found->second.Version;
return missingVersion();
}
// When looking up a module, each module importer will report back
// if it finds a module with a specified version. This routine verifies
// whether said version is valid and if it superceeds the best
// previously-discovered version of this module found.
auto validateVersion =
[](const ModuleLoader::ModuleVersionInfo &bestVersionInfo,
const ModuleLoader::ModuleVersionInfo &versionInfo,
bool underlyingVersion) {
if (!versionInfo.isValid())
return false; // The loader didn't attempt to parse a version.
if (underlyingVersion && !isClangModuleVersion(versionInfo))
return false; // We're only matching Clang module versions.
if (bestVersionInfo.isValid() &&
versionInfo.getSourceKind() <= bestVersionInfo.getSourceKind())
return false; // This module version's source is lower priority.
return true;
};
// For each module loader, attempt to discover queried module,
// along the way record the discovered module's version as well as
// the discovered module's underlying Clang module's version.
auto lookupVersionedModule =
[&](ModuleLoader::ModuleVersionInfo &bestVersionInfo,
ModuleLoader::ModuleVersionInfo &bestUnderlyingVersionInfo) -> bool {
for (auto &importer : getImpl().ModuleLoaders) {
ModuleLoader::ModuleVersionInfo versionInfo;
if (!importer->canImportModule(ModuleName, loc, &versionInfo))
continue; // The loader can't find the module.
if (validateVersion(bestVersionInfo, versionInfo,
/* underlyingVersion */ false))
bestVersionInfo = versionInfo;
if (validateVersion(bestUnderlyingVersionInfo, versionInfo,
/* underlyingVersion */ true))
bestUnderlyingVersionInfo = versionInfo;
}
if (!underlyingVersion && !bestVersionInfo.isValid())
return false;
if (underlyingVersion && !bestUnderlyingVersionInfo.isValid())
return false;
foundVersion = bestVersionInfo.getVersion();
foundUnderlyingClangVersion = bestUnderlyingVersionInfo.getVersion();
return true;
};
// For queries which do not care about any kind of module information
// such as e.g. `testImportModule`, simply return `true` as soon
// as *any* loader can find the queried module.
auto lookupModule = [&]() -> bool {
for (auto &importer : getImpl().ModuleLoaders) {
ModuleLoader::ModuleVersionInfo versionInfo;
if (!importer->canImportModule(ModuleName, loc, &versionInfo))
continue; // The loader can't find the module.
return true;
}
return false;
};
if (version.empty()) {
// If this module has already been successfully imported, it is importable.
if (getLoadedModule(ModuleName) != nullptr)
return true;
if (!isSourceCanImport)
return lookupModule();
// Otherwise, ask whether any module loader can load the module.
for (auto &importer : getImpl().ModuleLoaders) {
if (importer->canImportModule(ModuleName, loc, nullptr))
return true;
}
// Otherwise, ask whether any module loader can load the module,
// and record the module version that the succeeding loader
// observed.
ModuleLoader::ModuleVersionInfo versionInfo, underlyingVersionInfo;
if (lookupVersionedModule(versionInfo, underlyingVersionInfo))
return true;
if (isSourceCanImport)
FailedModuleImportNames.insert(ModuleNameStr);
@@ -2722,43 +2792,15 @@ bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
// We need to check whether the version of the module is high enough.
// Retrieve a module version from each module loader that can find the module
// and use the best source available for the query.
ModuleLoader::ModuleVersionInfo bestVersionInfo;
for (auto &importer : getImpl().ModuleLoaders) {
ModuleLoader::ModuleVersionInfo versionInfo;
if (!importer->canImportModule(ModuleName, loc, &versionInfo))
continue; // The loader can't find the module.
if (!versionInfo.isValid())
continue; // The loader didn't attempt to parse a version.
if (underlyingVersion && !isClangModuleVersion(versionInfo))
continue; // We're only matching Clang module versions.
if (bestVersionInfo.isValid() &&
versionInfo.getSourceKind() <= bestVersionInfo.getSourceKind())
continue; // This module version's source is lower priority.
bestVersionInfo = versionInfo;
}
if (!bestVersionInfo.isValid())
ModuleLoader::ModuleVersionInfo versionInfo, underlyingVersionInfo;
if (!lookupVersionedModule(versionInfo, underlyingVersionInfo))
return false;
if (bestVersionInfo.getVersion().empty()) {
// The module version could not be parsed from the preferred source for
// this query. Diagnose and treat the query as if it succeeded.
auto mID = ModuleName[0];
auto diagLoc = mID.Loc;
if (mID.Loc.isInvalid())
diagLoc = loc;
Diags.diagnose(diagLoc, diag::cannot_find_project_version,
getModuleVersionKindString(bestVersionInfo), mID.Item.str());
return true;
}
const auto &queryVersion = underlyingVersion ? underlyingVersionInfo : versionInfo;
if (queryVersion.getVersion().empty())
return missingVersion();
foundVersion = bestVersionInfo.getVersion();
return version <= bestVersionInfo.getVersion();
return version <= queryVersion.getVersion();
}
void ASTContext::forEachCanImportVersionCheck(
@@ -2773,13 +2815,15 @@ bool ASTContext::canImportModule(ImportPath::Module moduleName, SourceLoc loc,
llvm::VersionTuple version,
bool underlyingVersion) {
llvm::VersionTuple versionInfo;
llvm::VersionTuple underlyingVersionInfo;
if (!canImportModuleImpl(moduleName, loc, version, underlyingVersion, true,
versionInfo))
versionInfo, underlyingVersionInfo))
return false;
SmallString<64> fullModuleName;
moduleName.getString(fullModuleName);
addSucceededCanImportModule(fullModuleName, underlyingVersion, versionInfo);
addSucceededCanImportModule(fullModuleName, versionInfo, underlyingVersionInfo);
return true;
}
@@ -2787,8 +2831,10 @@ bool ASTContext::testImportModule(ImportPath::Module ModuleName,
llvm::VersionTuple version,
bool underlyingVersion) const {
llvm::VersionTuple versionInfo;
llvm::VersionTuple underlyingVersionInfo;
return canImportModuleImpl(ModuleName, SourceLoc(), version,
underlyingVersion, false, versionInfo);
underlyingVersion, false, versionInfo,
underlyingVersionInfo);
}
ModuleDecl *

9
test/CAS/can-import.swift
View File

@@ -21,8 +21,10 @@
// CMD-NEXT: "C"
// CMD-NEXT: "1.0"
// CMD-NEXT: "0"
// CMD-NEXT: "-module-can-import"
// CMD-NEXT: "-module-can-import-version"
// CMD-NEXT: "D"
// CMD-NEXT: "1.0"
// CMD-NEXT: "0"
// CMD-NEXT: "-module-can-import-version"
// CMD-NEXT: "Simple"
// CMD-NEXT: "0"
@@ -111,6 +113,11 @@ public func c() { }
// swift-module-flags: -module-name D -O -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib -user-module-version 1.0
public func d() { }
//--- include/Simple.swiftinterface
// swift-interface-format-version: 1.0
// swift-module-flags: -module-name Simple -O -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib
public func simple() { }
//--- frameworks/Simple.framework/Modules/module.modulemap
framework module Simple {
umbrella header "Simple.h"

4
test/ClangImporter/can_import_underlying_version_tbd_missing_version.swift
View File

@@ -16,8 +16,8 @@ func canImportUnderlyingVersion() {
}
func canImportVersion() {
#if canImport(Simple, _version: 3.3) // expected-warning {{cannot find user version number for Clang module 'Simple'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Simple'; version number ignored}}
#if canImport(Simple, _version: 3.3) // expected-warning {{cannot find user version number for module 'Simple'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for module 'Simple'; version number ignored}}
let a = 1 // expected-warning {{initialization of immutable value 'a' was never used; consider replacing with assignment to '_' or removing it}}
#endif
}

10
test/Parse/ConditionalCompilation/can_import_options.swift
View File

@@ -9,9 +9,10 @@ func canImport() {
let basicCheck = 1 // expected-warning {{initialization of immutable value 'basicCheck' was never used; consider replacing with assignment to '_' or removing it}}
#endif
#if canImport(Foo, _version: 1)
// No actual Foo to be imported since it is not versioned.
let versionCheck = 1
#if canImport(Foo, _version: 1) // expected-warning {{cannot find user version number for module 'Foo'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for module 'Foo'; version number ignored}}
// An unversioned 'Foo' causes versioned queries to evaluate to 'true'
let versionCheck = 1 // expected-warning {{initialization of immutable value 'versionCheck' was never used; consider replacing with assignment to '_' or removing it}}
#endif
}
@@ -72,7 +73,8 @@ func canImportVersioned() {
let extraComponent = 1 // expected-warning {{initialization of immutable value 'extraComponent' was never used; consider replacing with assignment to '_' or removing it}}
#endif
#if canImport(Bar, _underlyingVersion: 113.33)
#if canImport(Bar, _underlyingVersion: 113.33) // expected-warning{{cannot find user version number for Clang module 'Bar'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Bar'; version number ignored}}
// Bar is an unversioned Swift module with no underlying clang module.
let underlyingMinorSmaller = 1 // expected-warning {{initialization of immutable value 'underlyingMinorSmaller' was never used; consider replacing with assignment to '_' or removing it}}
#endif

10
test/Parse/ConditionalCompilation/can_import_version.swift
View File

@@ -67,8 +67,9 @@ func canImportVersioned() {
let extraComponent = 1 // expected-warning {{initialization of immutable value 'extraComponent' was never used; consider replacing with assignment to '_' or removing it}}
#endif
#if canImport(Foo, _underlyingVersion: 113.33)
// Foo is an unversioned Swift module with no underlying clang module.
#if canImport(Foo, _underlyingVersion: 113.33) // expected-warning {{cannot find user version number for Clang module 'Foo'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Foo'; version number ignored}}
// Foo is a Swift module with no underlying clang module.
let underlyingMinorSmaller = 1 // expected-warning {{initialization of immutable value 'underlyingMinorSmaller' was never used; consider replacing with assignment to '_' or removing it}}
#endif
@@ -135,8 +136,9 @@ func canImportVersionedString() {
let extraComponent = 1 // expected-warning {{initialization of immutable value 'extraComponent' was never used; consider replacing with assignment to '_' or removing it}}
#endif
#if canImport(Foo, _underlyingVersion: "113.33")
// Foo is an unversioned Swift module with no underlying clang module.
#if canImport(Foo, _underlyingVersion: "113.33") // expected-warning {{cannot find user version number for Clang module 'Foo'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for Clang module 'Foo'; version number ignored}}
// Foo is a Swift module with no underlying clang module.
let underlyingMinorSmaller = 1 // expected-warning {{initialization of immutable value 'underlyingMinorSmaller' was never used; consider replacing with assignment to '_' or removing it}}
#endif
}

10
test/Parse/ConditionalCompilation/can_import_version_missing_user_module_version.swift
View File

@@ -13,18 +13,18 @@ import NoUserModuleVersion
func testCanImportNoUserModuleVersion() {
#if canImport(NoUserModuleVersion, _version: 113.331) // expected-warning {{cannot find user version number for Swift module 'NoUserModuleVersion'; version number ignored}}
#if canImport(NoUserModuleVersion, _version: 113.331) // expected-warning {{cannot find user version number for module 'NoUserModuleVersion'; version number ignored}}
// NOTE: Duplicate warning because the canImport request happens twice when parser
// validation is enabled.
// TODO(ParserValidation): expected-warning@-3 *{{cannot find user version number for Swift module 'NoUserModuleVersion'; version number ignored}}
// TODO(ParserValidation): expected-warning@-3 *{{cannot find user version number for module 'NoUserModuleVersion'; version number ignored}}
let a = 1 // expected-warning {{initialization of immutable value 'a' was never used; consider replacing with assignment to '_' or removing it}}
#endif
#if canImport(NoUserModuleVersion, _version: 2) // expected-warning {{cannot find user version number for Swift module 'NoUserModuleVersion'; version number ignored}}
let b = 1 // expected-warning {{initialization of immutable value 'b' was never used; consider replacing with assignment to '_' or removing it}}
#if canImport(NoUserModuleVersion, _version: 2) // expected-warning {{cannot find user version number for module 'NoUserModuleVersion'; version number ignored}}
// NOTE: Duplicate warning because the canImport request happens twice when parser
// validation is enabled.
// TODO(ParserValidation): expected-warning@-4 *{{cannot find user version number for Swift module 'NoUserModuleVersion'; version number ignored}}
// TODO(ParserValidation): expected-warning@-3 *{{cannot find user version number for module 'NoUserModuleVersion'; version number ignored}}
let b = 1 // expected-warning {{initialization of immutable value 'b' was never used; consider replacing with assignment to '_' or removing it}}
#endif
}

7
test/ScanDependencies/can_import_explicit_versioned_unversioned_module.swift
View File

@@ -1,4 +1,4 @@
// REQUIRES: executable_test
// REQUIRES: objc_interop
// RUN: %empty-directory(%t)
// RUN: %empty-directory(%t/inputs)
// RUN: %empty-directory(%t/module-cache)
@@ -49,9 +49,10 @@ public func Foo() {
}
//--- MyExe.swift
#if canImport(MyLib, _version: 42)
#if canImport(MyLib, _version: 42) // expected-warning {{cannot find user version number for module 'MyLib'; version number ignored}}
// TODO(ParserValidation): expected-warning@-1 *{{cannot find user version number for module 'MyLib'; version number ignored}}
#warning("versioned canImport() of unversioned module is working")
// expected-warning@-1{{versioned canImport() of unversioned module is working}}
// expected-warning@-1{{versioned canImport() of unversioned module is working}}
#else
#warning("versioned canImport() of unversioned module is broken")
#endif