Skip to content

Commit bdb58a3

Browse files
swift-ciswift-ci
authored
Merge pull request #60981 from tshortli/can-import-version-prefer-swiftinterface
AST: Make the versioned variants of `#if canImport()` more reliable and consistent
2 parents 237ff4c + bbf189c commit bdb58a3

20 files changed

+338
-113
lines changed

‎include/swift/AST/ModuleLoader.h

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,51 @@ class ModuleLoader {
196196
virtual void collectVisibleTopLevelModuleNames(
197197
SmallVectorImpl<Identifier> &names) const = 0;
198198

199+
/// The kind of source for a module's version.
200+
///
201+
/// NOTE: The order of the members is significant; they are declared in
202+
/// ascending priority order.
203+
enum class ModuleVersionSourceKind {
204+
ClangModuleTBD,
205+
SwiftBinaryModule,
206+
SwiftInterface,
207+
};
208+
209+
/// Represents a module version and the source it was parsed from.
210+
class ModuleVersionInfo {
211+
llvm::VersionTuple Version;
212+
llvm::Optional<ModuleVersionSourceKind> SourceKind;
213+
214+
public:
215+
/// Returns true if the version has a valid source kind.
216+
bool isValid() const { return SourceKind.hasValue(); }
217+
218+
/// Returns the version, which may be empty if a version was not present or
219+
/// was unparsable.
220+
llvm::VersionTuple getVersion() const { return Version; }
221+
222+
/// Returns the kind of source of the module version. Do not call if
223+
/// \c isValid() returns false.
224+
ModuleVersionSourceKind getSourceKind() const {
225+
return SourceKind.getValue();
226+
}
227+
228+
void setVersion(llvm::VersionTuple version, ModuleVersionSourceKind kind) {
229+
Version = version;
230+
SourceKind = kind;
231+
}
232+
};
233+
199234
/// Check whether the module with a given name can be imported without
200235
/// importing it.
201236
///
202237
/// Note that even if this check succeeds, errors may still occur if the
203238
/// module is loaded in full.
239+
///
240+
/// If a non-null \p versionInfo is provided, the module version will be
241+
/// parsed and populated.
204242
virtual bool canImportModule(ImportPath::Module named,
205-
llvm::VersionTuple version,
206-
bool underlyingVersion) = 0;
243+
ModuleVersionInfo *versionInfo) = 0;
207244

208245
/// Import a module with the given module path.
209246
///

‎include/swift/ClangImporter/ClangImporter.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,11 @@ class ClangImporter final : public ClangModuleLoader {
204204
///
205205
/// Note that even if this check succeeds, errors may still occur if the
206206
/// module is loaded in full.
207+
///
208+
/// If a non-null \p versionInfo is provided, the module version will be
209+
/// parsed and populated.
207210
virtual bool canImportModule(ImportPath::Module named,
208-
llvm::VersionTuple version,
209-
bool underlyingVersion) override;
211+
ModuleVersionInfo *versionInfo) override;
210212

211213
/// Import a module with the given module path.
212214
///

‎include/swift/Frontend/ModuleInterfaceLoader.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ class ExplicitSwiftModuleLoader: public SerializedModuleLoaderBase {
151151
std::unique_ptr<llvm::MemoryBuffer> *ModuleSourceInfoBuffer,
152152
bool skipBuildingInterface, bool IsFramework) override;
153153

154-
bool canImportModule(ImportPath::Module named, llvm::VersionTuple version,
155-
bool underlyingVersion) override;
154+
bool canImportModule(ImportPath::Module named,
155+
ModuleVersionInfo *versionInfo) override;
156156

157157
bool isCached(StringRef DepPath) override { return false; };
158158

‎include/swift/Sema/SourceLoader.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,11 @@ class SourceLoader : public ModuleLoader {
5656
///
5757
/// Note that even if this check succeeds, errors may still occur if the
5858
/// module is loaded in full.
59+
///
60+
/// If a non-null \p versionInfo is provided, the module version will be
61+
/// parsed and populated.
5962
virtual bool canImportModule(ImportPath::Module named,
60-
llvm::VersionTuple version,
61-
bool underlyingVersion) override;
63+
ModuleVersionInfo *versionInfo) override;
6264

6365
/// Import a module with the given module path.
6466
///

‎include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,11 @@ class SerializedModuleLoaderBase : public ModuleLoader {
168168
///
169169
/// Note that even if this check succeeds, errors may still occur if the
170170
/// module is loaded in full.
171+
///
172+
/// If a non-null \p versionInfo is provided, the module version will be
173+
/// parsed and populated.
171174
virtual bool canImportModule(ImportPath::Module named,
172-
llvm::VersionTuple version,
173-
bool underlyingVersion) override;
175+
ModuleVersionInfo *versionInfo) override;
174176

175177
/// Import a module with the given module path.
176178
///
@@ -287,8 +289,9 @@ class MemoryBufferSerializedModuleLoader : public SerializedModuleLoaderBase {
287289
public:
288290
virtual ~MemoryBufferSerializedModuleLoader();
289291

290-
bool canImportModule(ImportPath::Module named, llvm::VersionTuple version,
291-
bool underlyingVersion) override;
292+
bool canImportModule(ImportPath::Module named,
293+
ModuleVersionInfo *versionInfo) override;
294+
292295
ModuleDecl *
293296
loadModule(SourceLoc importLoc,
294297
ImportPath::Module path) override;

‎lib/AST/ASTContext.cpp

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2184,6 +2184,27 @@ bool ASTContext::shouldPerformTypoCorrection() {
21842184
return NumTypoCorrections <= LangOpts.TypoCorrectionLimit;
21852185
}
21862186

2187+
static bool isClangModuleVersion(const ModuleLoader::ModuleVersionInfo &info) {
2188+
switch (info.getSourceKind()) {
2189+
case ModuleLoader::ModuleVersionSourceKind::ClangModuleTBD:
2190+
return true;
2191+
case ModuleLoader::ModuleVersionSourceKind::SwiftBinaryModule:
2192+
case ModuleLoader::ModuleVersionSourceKind::SwiftInterface:
2193+
return false;
2194+
}
2195+
}
2196+
2197+
static StringRef
2198+
getModuleVersionKindString(const ModuleLoader::ModuleVersionInfo &info) {
2199+
switch (info.getSourceKind()) {
2200+
case ModuleLoader::ModuleVersionSourceKind::ClangModuleTBD:
2201+
return "Clang";
2202+
case ModuleLoader::ModuleVersionSourceKind::SwiftBinaryModule:
2203+
case ModuleLoader::ModuleVersionSourceKind::SwiftInterface:
2204+
return "Swift";
2205+
}
2206+
}
2207+
21872208
bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
21882209
llvm::VersionTuple version,
21892210
bool underlyingVersion,
@@ -2195,22 +2216,60 @@ bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
21952216
// If we've failed loading this module before, don't look for it again.
21962217
if (FailedModuleImportNames.count(ModuleNameStr))
21972218
return false;
2198-
// If no specific version, the module is importable if it has already been imported.
2219+
21992220
if (version.empty()) {
22002221
// If this module has already been successfully imported, it is importable.
22012222
if (getLoadedModule(ModuleName) != nullptr)
22022223
return true;
2224+
2225+
// Otherwise, ask whether any module loader can load the module.
2226+
for (auto &importer : getImpl().ModuleLoaders) {
2227+
if (importer->canImportModule(ModuleName, nullptr))
2228+
return true;
2229+
}
2230+
2231+
if (updateFailingList)
2232+
FailedModuleImportNames.insert(ModuleNameStr);
2233+
2234+
return false;
22032235
}
2204-
// Otherwise, ask the module loaders.
2236+
2237+
// We need to check whether the version of the module is high enough.
2238+
// Retrieve a module version from each module loader that can find the module
2239+
// and use the best source available for the query.
2240+
ModuleLoader::ModuleVersionInfo bestVersionInfo;
22052241
for (auto &importer : getImpl().ModuleLoaders) {
2206-
if (importer->canImportModule(ModuleName, version, underlyingVersion)) {
2207-
return true;
2208-
}
2242+
ModuleLoader::ModuleVersionInfo versionInfo;
2243+
2244+
if (!importer->canImportModule(ModuleName, &versionInfo))
2245+
continue; // The loader can't find the module.
2246+
2247+
if (!versionInfo.isValid())
2248+
continue; // The loader didn't attempt to parse a version.
2249+
2250+
if (underlyingVersion && !isClangModuleVersion(versionInfo))
2251+
continue; // We're only matching Clang module versions.
2252+
2253+
if (bestVersionInfo.isValid() &&
2254+
versionInfo.getSourceKind() <= bestVersionInfo.getSourceKind())
2255+
continue; // This module version's source is lower priority.
2256+
2257+
bestVersionInfo = versionInfo;
22092258
}
2210-
if (updateFailingList && version.empty()) {
2211-
FailedModuleImportNames.insert(ModuleNameStr);
2259+
2260+
if (!bestVersionInfo.isValid())
2261+
return false;
2262+
2263+
if (bestVersionInfo.getVersion().empty()) {
2264+
// The module version could not be parsed from the preferred source for
2265+
// this query. Diagnose and treat the query as if it succeeded.
2266+
auto mID = ModuleName[0];
2267+
Diags.diagnose(mID.Loc, diag::cannot_find_project_version,
2268+
getModuleVersionKindString(bestVersionInfo), mID.Item.str());
2269+
return true;
22122270
}
2213-
return false;
2271+
2272+
return version <= bestVersionInfo.getVersion();
22142273
}
22152274

22162275
bool ASTContext::canImportModule(ImportPath::Module ModuleName,

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1798,8 +1798,7 @@ static std::string getScalaNodeText(llvm::yaml::Node *N) {
17981798
}
17991799

18001800
bool ClangImporter::canImportModule(ImportPath::Module modulePath,
1801-
llvm::VersionTuple version,
1802-
bool underlyingVersion) {
1801+
ModuleVersionInfo *versionInfo) {
18031802
// Look up the top-level module to see if it exists.
18041803
auto &clangHeaderSearch = Impl.getClangPreprocessor().getHeaderSearchInfo();
18051804
auto topModule = modulePath.front();
@@ -1842,10 +1841,10 @@ bool ClangImporter::canImportModule(ImportPath::Module modulePath,
18421841
}
18431842
}
18441843

1845-
if (version.empty())
1844+
if (!versionInfo)
18461845
return true;
1846+
18471847
assert(available);
1848-
assert(!version.empty());
18491848
llvm::VersionTuple currentVersion;
18501849
StringRef path = getClangASTContext().getSourceManager()
18511850
.getFilename(clangModule->DefinitionLoc);
@@ -1888,16 +1887,10 @@ bool ClangImporter::canImportModule(ImportPath::Module modulePath,
18881887
}
18891888
break;
18901889
}
1891-
// Diagnose unable to checking the current version.
1892-
if (currentVersion.empty()) {
1893-
Impl.diagnose(topModule.Loc, diag::cannot_find_project_version, "Clang",
1894-
topModule.Item.str());
1895-
return true;
1896-
}
1897-
assert(!currentVersion.empty());
1898-
// Give a green light if the version on disk is greater or equal to the version
1899-
// specified in the canImport condition.
1900-
return currentVersion >= version;
1890+
1891+
versionInfo->setVersion(currentVersion,
1892+
ModuleVersionSourceKind::ClangModuleTBD);
1893+
return true;
19011894
}
19021895

19031896
ModuleDecl *ClangImporter::Implementation::loadModuleClang(

‎lib/Frontend/ModuleInterfaceLoader.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1947,9 +1947,8 @@ std::error_code ExplicitSwiftModuleLoader::findModuleFilesInDirectory(
19471947
return std::make_error_code(std::errc::not_supported);
19481948
}
19491949

1950-
bool ExplicitSwiftModuleLoader::canImportModule(ImportPath::Module path,
1951-
llvm::VersionTuple version,
1952-
bool underlyingVersion) {
1950+
bool ExplicitSwiftModuleLoader::canImportModule(
1951+
ImportPath::Module path, ModuleVersionInfo *versionInfo) {
19531952
// FIXME: Swift submodules?
19541953
if (path.hasSubmodule())
19551954
return false;

‎lib/Sema/SourceLoader.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,7 @@ void SourceLoader::collectVisibleTopLevelModuleNames(
7171
}
7272

7373
bool SourceLoader::canImportModule(ImportPath::Module path,
74-
llvm::VersionTuple version,
75-
bool underlyingVersion) {
74+
ModuleVersionInfo *versionInfo) {
7675
// FIXME: Swift submodules?
7776
if (path.hasSubmodule())
7877
return false;
@@ -90,6 +89,7 @@ bool SourceLoader::canImportModule(ImportPath::Module path,
9089

9190
return false;
9291
}
92+
9393
return true;
9494
}
9595

‎lib/Serialization/ModuleDependencyScanner.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ Optional<ModuleDependencies> SerializedModuleLoaderBase::getModuleDependencies(
199199
assert(isa<PlaceholderSwiftModuleScanner>(scanners[0].get()) &&
200200
"Expected PlaceholderSwiftModuleScanner as the first dependency scanner loader.");
201201
for (auto &scanner : scanners) {
202-
if (scanner->canImportModule(modulePath, llvm::VersionTuple(), false)) {
202+
if (scanner->canImportModule(modulePath, nullptr)) {
203203
// Record the dependencies.
204204
cache.recordDependencies(moduleName, *(scanner->dependencies));
205205
return std::move(scanner->dependencies);

0 commit comments

Comments
 (0)