Skip to content

Commit 15119e8

Browse files
[ScanDependency][canImport] Improve canImport handling in explicit build
Teach dependency scanner to report all the module canImport check result to swift-frontend, so swift-frontend doesn't need to parse swiftmodule or parse TBD file to determine the versions. This ensures dependency scanner and swift-frontend will have the same resolution for all canImport checks. This also fixes two related issues: * Previously, in order to get consistant results between scanner and frontend, scanner will request building the module in canImport check even it is not imported later. This slightly alters the definition of the canImport to only succeed when the module can be found AND be built. This also can affect the auto-link in such cases. * For caching build, the location of the clang module is abstracted away so swift-frontend cannot locate the TBD file to resolve underlyingVersion. rdar://128067152
1 parent c46fe92 commit 15119e8

File tree

11 files changed

+355
-45
lines changed

11 files changed

+355
-45
lines changed

include/swift/AST/ASTContext.h

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include "llvm/ADT/TinyPtrVector.h"
4848
#include "llvm/Support/Allocator.h"
4949
#include "llvm/Support/DataTypes.h"
50+
#include "llvm/Support/VersionTuple.h"
5051
#include "llvm/Support/VirtualOutputBackend.h"
5152
#include <functional>
5253
#include <memory>
@@ -411,7 +412,11 @@ class ASTContext final {
411412

412413
/// Cache of module names that passed the 'canImport' test. This cannot be
413414
/// mutable since it needs to be queried for dependency discovery.
414-
llvm::StringSet<> SucceededModuleImportNames;
415+
struct ImportedModuleVersionInfo {
416+
llvm::VersionTuple Version;
417+
llvm::VersionTuple UnderlyingVersion;
418+
};
419+
std::map<std::string, ImportedModuleVersionInfo> SucceededModuleImportNames;
415420

416421
/// Set if a `-module-alias` was passed. Used to store mapping between module aliases and
417422
/// their corresponding real names, and vice versa for a reverse lookup, which is needed to check
@@ -1102,7 +1107,12 @@ class ASTContext final {
11021107
/// module is loaded in full.
11031108
bool canImportModuleImpl(ImportPath::Module ModulePath,
11041109
llvm::VersionTuple version, bool underlyingVersion,
1105-
bool updateFailingList) const;
1110+
bool updateFailingList,
1111+
llvm::VersionTuple &foundVersion) const;
1112+
1113+
/// Add successful canImport modules.
1114+
void addSucceededCanImportModule(StringRef moduleName, bool underlyingVersion,
1115+
const llvm::VersionTuple &versionInfo);
11061116

11071117
public:
11081118
namelookup::ImportCache &getImportCache() const;
@@ -1144,10 +1154,10 @@ class ASTContext final {
11441154
llvm::VersionTuple version = llvm::VersionTuple(),
11451155
bool underlyingVersion = false) const;
11461156

1147-
/// \returns a set of names from all successfully canImport module checks.
1148-
const llvm::StringSet<> &getSuccessfulCanImportCheckNames() const {
1149-
return SucceededModuleImportNames;
1150-
}
1157+
/// Callback on each successful imported.
1158+
void forEachSuccessfulCanImportCheck(
1159+
std::function<void(StringRef, const llvm::VersionTuple &,
1160+
const llvm::VersionTuple &)>) const;
11511161

11521162
/// \returns a module with a given name that was already loaded. If the
11531163
/// module was not loaded, returns nullptr.

include/swift/AST/DiagnosticsFrontend.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,8 @@ ERROR(unknown_forced_module_loading_mode,none,
481481
(StringRef))
482482
ERROR(error_creating_remark_serializer,none,
483483
"error while creating remark serializer: '%0'", (StringRef))
484+
ERROR(invalid_can_import_module_version,none,
485+
"invalid version passed to -module-can-import-version: '%0'", (StringRef))
484486

485487
REMARK(interface_file_lock_failure,none,
486488
"building module interface without lock file", ())

include/swift/AST/SearchPathOptions.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "llvm/ADT/IntrusiveRefCntPtr.h"
2121
#include "llvm/ADT/StringMap.h"
2222
#include "llvm/Support/Error.h"
23+
#include "llvm/Support/VersionTuple.h"
2324
#include "llvm/Support/VirtualFileSystem.h"
2425
#include <optional>
2526

@@ -509,6 +510,14 @@ class SearchPathOptions {
509510
using CrossImportMap = llvm::StringMap<std::vector<std::string>>;
510511
CrossImportMap CrossImportInfo;
511512

513+
/// CanImport information passed from scanning.
514+
struct CanImportInfo {
515+
std::string ModuleName;
516+
llvm::VersionTuple Version;
517+
llvm::VersionTuple UnderlyingVersion;
518+
};
519+
std::vector<CanImportInfo> CanImportModuleInfo;
520+
512521
/// Whether to search for cross import overlay on file system.
513522
bool DisableCrossImportOverlaySearch = false;
514523

include/swift/Option/FrontendOptions.td

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,14 @@ def swift_module_cross_import: MultiArg<["-"], "swift-module-cross-import", 2>,
224224
MetaVarName<"<moduleName> <crossImport.swiftoverlay>">,
225225
HelpText<"Specify the cross import module">;
226226

227+
def module_can_import: Separate<["-"], "module-can-import">,
228+
MetaVarName<"<moduleName>">,
229+
HelpText<"Specify canImport module name">;
230+
231+
def module_can_import_version: MultiArg<["-"], "module-can-import-version", 3>,
232+
MetaVarName<"<moduleName> <version> <underlyingVersion>">,
233+
HelpText<"Specify canImport module and versions">;
234+
227235
def disable_cross_import_overlay_search: Flag<["-"], "disable-cross-import-overlay-search">,
228236
HelpText<"Disable searching for cross import overlay file">;
229237

lib/AST/ASTContext.cpp

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
#include "llvm/Support/Allocator.h"
7575
#include "llvm/Support/Compiler.h"
7676
#include "llvm/Support/FormatVariadic.h"
77+
#include "llvm/Support/VersionTuple.h"
7778
#include "llvm/Support/VirtualOutputBackend.h"
7879
#include "llvm/Support/VirtualOutputBackends.h"
7980
#include <algorithm>
@@ -786,6 +787,12 @@ ASTContext::ASTContext(
786787
registerAccessRequestFunctions(evaluator);
787788
registerNameLookupRequestFunctions(evaluator);
788789

790+
// Register canImport module info.
791+
for (auto &info: SearchPathOpts.CanImportModuleInfo) {
792+
addSucceededCanImportModule(info.ModuleName, false, info.Version);
793+
addSucceededCanImportModule(info.ModuleName, true, info.UnderlyingVersion);
794+
}
795+
789796
// Provide a default OnDiskOutputBackend if user didn't supply one.
790797
if (!OutputBackend)
791798
OutputBackend = llvm::makeIntrusiveRefCnt<llvm::vfs::OnDiskOutputBackend>();
@@ -2386,21 +2393,52 @@ getModuleVersionKindString(const ModuleLoader::ModuleVersionInfo &info) {
23862393
}
23872394
}
23882395

2389-
bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
2390-
llvm::VersionTuple version,
2391-
bool underlyingVersion,
2392-
bool updateFailingList) const {
2396+
void ASTContext::addSucceededCanImportModule(
2397+
StringRef moduleName, bool underlyingVersion,
2398+
const llvm::VersionTuple &versionInfo) {
2399+
auto &entry = SucceededModuleImportNames[moduleName.str()];
2400+
if (!versionInfo.empty()) {
2401+
if (underlyingVersion)
2402+
entry.UnderlyingVersion = versionInfo;
2403+
else
2404+
entry.Version = versionInfo;
2405+
}
2406+
}
2407+
2408+
bool ASTContext::canImportModuleImpl(
2409+
ImportPath::Module ModuleName, llvm::VersionTuple version,
2410+
bool underlyingVersion, bool updateFailingList,
2411+
llvm::VersionTuple &foundVersion) const {
23932412
SmallString<64> FullModuleName;
23942413
ModuleName.getString(FullModuleName);
2395-
auto ModuleNameStr = FullModuleName.str();
2414+
auto ModuleNameStr = FullModuleName.str().str();
23962415

23972416
// If we've failed loading this module before, don't look for it again.
23982417
if (FailedModuleImportNames.count(ModuleNameStr))
23992418
return false;
24002419

2420+
// If this module has already been checked, use the information.
2421+
auto Found = SucceededModuleImportNames.find(ModuleNameStr);
2422+
if (Found != SucceededModuleImportNames.end()) {
2423+
if (version.empty())
2424+
return true;
2425+
2426+
if (underlyingVersion) {
2427+
if (!Found->second.UnderlyingVersion.empty())
2428+
return version <= Found->second.UnderlyingVersion;
2429+
} else {
2430+
if (!Found->second.Version.empty())
2431+
return version <= Found->second.Version;
2432+
}
2433+
2434+
// If the canImport information is coming from the command-line, then no
2435+
// need to continue the search, return false.
2436+
if (!SearchPathOpts.CanImportModuleInfo.empty())
2437+
return false;
2438+
}
2439+
24012440
if (version.empty()) {
2402-
// If this module has already been checked successfully, it is importable.
2403-
if (SucceededModuleImportNames.count(ModuleNameStr))
2441+
if (Found != SucceededModuleImportNames.end())
24042442
return true;
24052443

24062444
// If this module has already been successfully imported, it is importable.
@@ -2454,28 +2492,38 @@ bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
24542492
return true;
24552493
}
24562494

2495+
foundVersion = bestVersionInfo.getVersion();
24572496
return version <= bestVersionInfo.getVersion();
24582497
}
24592498

2460-
bool ASTContext::canImportModule(ImportPath::Module ModuleName,
2499+
void ASTContext::forEachSuccessfulCanImportCheck(
2500+
std::function<void(StringRef, const llvm::VersionTuple &,
2501+
const llvm::VersionTuple &)>
2502+
Callback) const {
2503+
for (auto &entry : SucceededModuleImportNames)
2504+
Callback(entry.first, entry.second.Version, entry.second.UnderlyingVersion);
2505+
}
2506+
2507+
bool ASTContext::canImportModule(ImportPath::Module moduleName,
24612508
llvm::VersionTuple version,
24622509
bool underlyingVersion) {
2463-
if (!canImportModuleImpl(ModuleName, version, underlyingVersion, true))
2510+
llvm::VersionTuple versionInfo;
2511+
if (!canImportModuleImpl(moduleName, version, underlyingVersion, true,
2512+
versionInfo))
24642513
return false;
24652514

2466-
// If checked successfully, add the top level name to success list as
2467-
// dependency to handle clang submodule correctly. Swift does not have
2468-
// submodule so the name should be the same.
2469-
SmallString<64> TopModuleName;
2470-
ModuleName.getTopLevelPath().getString(TopModuleName);
2471-
SucceededModuleImportNames.insert(TopModuleName.str());
2515+
SmallString<64> fullModuleName;
2516+
moduleName.getString(fullModuleName);
2517+
addSucceededCanImportModule(fullModuleName, underlyingVersion, versionInfo);
24722518
return true;
24732519
}
24742520

24752521
bool ASTContext::testImportModule(ImportPath::Module ModuleName,
24762522
llvm::VersionTuple version,
24772523
bool underlyingVersion) const {
2478-
return canImportModuleImpl(ModuleName, version, underlyingVersion, false);
2524+
llvm::VersionTuple versionInfo;
2525+
return canImportModuleImpl(ModuleName, version, underlyingVersion, false,
2526+
versionInfo);
24792527
}
24802528

24812529
ModuleDecl *

lib/ClangImporter/ClangImporter.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2120,7 +2120,8 @@ bool ClangImporter::isModuleImported(const clang::Module *M) {
21202120
return M->NameVisibility == clang::Module::NameVisibilityKind::AllVisible;
21212121
}
21222122

2123-
static llvm::VersionTuple getCurrentVersionFromTBD(StringRef path,
2123+
static llvm::VersionTuple getCurrentVersionFromTBD(llvm::vfs::FileSystem &FS,
2124+
StringRef path,
21242125
StringRef moduleName) {
21252126
std::string fwName = (moduleName + ".framework").str();
21262127
auto pos = path.find(fwName);
@@ -2130,7 +2131,7 @@ static llvm::VersionTuple getCurrentVersionFromTBD(StringRef path,
21302131
llvm::sys::path::append(buffer, moduleName + ".tbd");
21312132
auto tbdPath = buffer.str();
21322133
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> tbdBufOrErr =
2133-
llvm::MemoryBuffer::getFile(tbdPath);
2134+
FS.getBufferForFile(tbdPath);
21342135
// .tbd file doesn't exist, exit.
21352136
if (!tbdBufOrErr)
21362137
return {};
@@ -2193,8 +2194,8 @@ bool ClangImporter::canImportModule(ImportPath::Module modulePath,
21932194

21942195
// Look for the .tbd file inside .framework dir to get the project version
21952196
// number.
2196-
llvm::VersionTuple currentVersion =
2197-
getCurrentVersionFromTBD(path, topModule.Item.str());
2197+
llvm::VersionTuple currentVersion = getCurrentVersionFromTBD(
2198+
Impl.Instance->getVirtualFileSystem(), path, topModule.Item.str());
21982199
versionInfo->setVersion(currentVersion,
21992200
ModuleVersionSourceKind::ClangModuleTBD);
22002201
return true;

lib/DependencyScan/ModuleDependencyScanner.cpp

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "llvm/CAS/CachingOnDiskFileSystem.h"
3232
#include "llvm/Support/Error.h"
3333
#include "llvm/Support/Threading.h"
34+
#include "llvm/Support/VersionTuple.h"
3435
#include "llvm/Support/VirtualFileSystem.h"
3536
#include <algorithm>
3637

@@ -464,15 +465,26 @@ ModuleDependencyScanner::getMainModuleDependencyInfo(ModuleDecl *mainModule) {
464465
&ScanASTContext.SourceMgr);
465466
}
466467

467-
// Add all the successful canImport checks from the ASTContext as part of
468-
// the dependency since only mainModule can have `canImport` check. This
469-
// needs to happen after visiting all the top-level decls from all
468+
// Pass all the successful canImport checks from the ASTContext as part of
469+
// build command to main module to ensure frontend gets the same result.
470+
// This needs to happen after visiting all the top-level decls from all
470471
// SourceFiles.
471-
for (auto &Module :
472-
mainModule->getASTContext().getSuccessfulCanImportCheckNames())
473-
mainDependencies.addModuleImport(Module.first(),
474-
&alreadyAddedModules);
475-
}
472+
auto buildArgs = mainDependencies.getCommandline();
473+
mainModule->getASTContext().forEachSuccessfulCanImportCheck(
474+
[&](StringRef moduleName, const llvm::VersionTuple &Version,
475+
const llvm::VersionTuple &UnderlyingVersion) {
476+
if (Version.empty() && UnderlyingVersion.empty()) {
477+
buildArgs.push_back("-module-can-import");
478+
buildArgs.push_back(moduleName.str());
479+
} else {
480+
buildArgs.push_back("-module-can-import-version");
481+
buildArgs.push_back(moduleName.str());
482+
buildArgs.push_back(Version.getAsString());
483+
buildArgs.push_back(UnderlyingVersion.getAsString());
484+
}
485+
});
486+
mainDependencies.updateCommandLine(buildArgs);
487+
}
476488

477489
return mainDependencies;
478490
}

lib/Frontend/CompilerInvocation.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "swift/Strings.h"
2525
#include "swift/SymbolGraphGen/SymbolGraphOptions.h"
2626
#include "llvm/ADT/STLExtras.h"
27+
#include "llvm/Support/VersionTuple.h"
2728
#include "llvm/TargetParser/Triple.h"
2829
#include "llvm/Option/Arg.h"
2930
#include "llvm/Option/ArgList.h"
@@ -2135,6 +2136,21 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
21352136
for (auto *A : Args.filtered(OPT_swift_module_cross_import))
21362137
Opts.CrossImportInfo[A->getValue(0)].push_back(A->getValue(1));
21372138

2139+
for (auto &Name : Args.getAllArgValues(OPT_module_can_import))
2140+
Opts.CanImportModuleInfo.push_back({Name, {}, {}});
2141+
2142+
for (auto *A: Args.filtered(OPT_module_can_import_version)) {
2143+
llvm::VersionTuple Version, UnderlyingVersion;
2144+
if (Version.tryParse(A->getValue(1)))
2145+
Diags.diagnose(SourceLoc(), diag::invalid_can_import_module_version,
2146+
A->getValue(1));
2147+
if (UnderlyingVersion.tryParse(A->getValue(2)))
2148+
Diags.diagnose(SourceLoc(), diag::invalid_can_import_module_version,
2149+
A->getValue(2));
2150+
Opts.CanImportModuleInfo.push_back(
2151+
{A->getValue(0), Version, UnderlyingVersion});
2152+
}
2153+
21382154
Opts.DisableCrossImportOverlaySearch |=
21392155
Args.hasArg(OPT_disable_cross_import_overlay_search);
21402156

0 commit comments

Comments
 (0)