Skip to content

Commit 026fcd2

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 3b4a3a5 commit 026fcd2

File tree

11 files changed

+381
-47
lines changed

11 files changed

+381
-47
lines changed

include/swift/AST/ASTContext.h

Lines changed: 21 additions & 7 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>
@@ -409,9 +410,17 @@ class ASTContext final {
409410
/// Cache of module names that fail the 'canImport' test in this context.
410411
mutable llvm::StringSet<> FailedModuleImportNames;
411412

413+
/// Versions of the modules found during versioned canImport checks.
414+
struct ImportedModuleVersionInfo {
415+
llvm::VersionTuple Version;
416+
llvm::VersionTuple UnderlyingVersion;
417+
};
418+
412419
/// Cache of module names that passed the 'canImport' test. This cannot be
413-
/// mutable since it needs to be queried for dependency discovery.
414-
llvm::StringSet<> SucceededModuleImportNames;
420+
/// mutable since it needs to be queried for dependency discovery. Keep sorted
421+
/// so caller of `forEachCanImportVersionCheck` can expect deterministic
422+
/// ordering.
423+
std::map<std::string, ImportedModuleVersionInfo> CanImportModuleVersions;
415424

416425
/// Set if a `-module-alias` was passed. Used to store mapping between module aliases and
417426
/// their corresponding real names, and vice versa for a reverse lookup, which is needed to check
@@ -1102,7 +1111,12 @@ class ASTContext final {
11021111
/// module is loaded in full.
11031112
bool canImportModuleImpl(ImportPath::Module ModulePath,
11041113
llvm::VersionTuple version, bool underlyingVersion,
1105-
bool updateFailingList) const;
1114+
bool updateFailingList,
1115+
llvm::VersionTuple &foundVersion) const;
1116+
1117+
/// Add successful canImport modules.
1118+
void addSucceededCanImportModule(StringRef moduleName, bool underlyingVersion,
1119+
const llvm::VersionTuple &versionInfo);
11061120

11071121
public:
11081122
namelookup::ImportCache &getImportCache() const;
@@ -1144,10 +1158,10 @@ class ASTContext final {
11441158
llvm::VersionTuple version = llvm::VersionTuple(),
11451159
bool underlyingVersion = false) const;
11461160

1147-
/// \returns a set of names from all successfully canImport module checks.
1148-
const llvm::StringSet<> &getSuccessfulCanImportCheckNames() const {
1149-
return SucceededModuleImportNames;
1150-
}
1161+
/// Callback on each successful imported.
1162+
void forEachCanImportVersionCheck(
1163+
std::function<void(StringRef, const llvm::VersionTuple &,
1164+
const llvm::VersionTuple &)>) const;
11511165

11521166
/// \returns a module with a given name that was already loaded. If the
11531167
/// 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: 67 additions & 17 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>();
@@ -2385,23 +2392,56 @@ getModuleVersionKindString(const ModuleLoader::ModuleVersionInfo &info) {
23852392
}
23862393
}
23872394

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

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

2400-
if (version.empty()) {
2401-
// If this module has already been checked successfully, it is importable.
2402-
if (SucceededModuleImportNames.count(ModuleNameStr))
2419+
// If this module has already been checked or there is information for the
2420+
// module from commandline, use that information instead of loading the
2421+
// module.
2422+
auto Found = CanImportModuleVersions.find(ModuleNameStr);
2423+
if (Found != CanImportModuleVersions.end()) {
2424+
if (version.empty())
24032425
return true;
24042426

2427+
if (underlyingVersion) {
2428+
if (!Found->second.UnderlyingVersion.empty())
2429+
return version <= Found->second.UnderlyingVersion;
2430+
} else {
2431+
if (!Found->second.Version.empty())
2432+
return version <= Found->second.Version;
2433+
}
2434+
2435+
// If the canImport information is coming from the command-line, then no
2436+
// need to continue the search, return false. For checking modules that are
2437+
// not passed from command-line, allow fallback to the module loading since
2438+
// this is not in a canImport request context that has already been resolved
2439+
// by scanner.
2440+
if (!SearchPathOpts.CanImportModuleInfo.empty())
2441+
return false;
2442+
}
2443+
2444+
if (version.empty()) {
24052445
// If this module has already been successfully imported, it is importable.
24062446
if (getLoadedModule(ModuleName) != nullptr)
24072447
return true;
@@ -2453,28 +2493,38 @@ bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
24532493
return true;
24542494
}
24552495

2496+
foundVersion = bestVersionInfo.getVersion();
24562497
return version <= bestVersionInfo.getVersion();
24572498
}
24582499

2459-
bool ASTContext::canImportModule(ImportPath::Module ModuleName,
2500+
void ASTContext::forEachCanImportVersionCheck(
2501+
std::function<void(StringRef, const llvm::VersionTuple &,
2502+
const llvm::VersionTuple &)>
2503+
Callback) const {
2504+
for (auto &entry : CanImportModuleVersions)
2505+
Callback(entry.first, entry.second.Version, entry.second.UnderlyingVersion);
2506+
}
2507+
2508+
bool ASTContext::canImportModule(ImportPath::Module moduleName,
24602509
llvm::VersionTuple version,
24612510
bool underlyingVersion) {
2462-
if (!canImportModuleImpl(ModuleName, version, underlyingVersion, true))
2511+
llvm::VersionTuple versionInfo;
2512+
if (!canImportModuleImpl(moduleName, version, underlyingVersion, true,
2513+
versionInfo))
24632514
return false;
24642515

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

24742522
bool ASTContext::testImportModule(ImportPath::Module ModuleName,
24752523
llvm::VersionTuple version,
24762524
bool underlyingVersion) const {
2477-
return canImportModuleImpl(ModuleName, version, underlyingVersion, false);
2525+
llvm::VersionTuple versionInfo;
2526+
return canImportModuleImpl(ModuleName, version, underlyingVersion, false,
2527+
versionInfo);
24782528
}
24792529

24802530
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().forEachCanImportVersionCheck(
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
@@ -25,6 +25,7 @@
2525
#include "swift/Strings.h"
2626
#include "swift/SymbolGraphGen/SymbolGraphOptions.h"
2727
#include "llvm/ADT/STLExtras.h"
28+
#include "llvm/Support/VersionTuple.h"
2829
#include "llvm/TargetParser/Triple.h"
2930
#include "llvm/Option/Arg.h"
3031
#include "llvm/Option/ArgList.h"
@@ -2144,6 +2145,21 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
21442145
for (auto *A : Args.filtered(OPT_swift_module_cross_import))
21452146
Opts.CrossImportInfo[A->getValue(0)].push_back(A->getValue(1));
21462147

2148+
for (auto &Name : Args.getAllArgValues(OPT_module_can_import))
2149+
Opts.CanImportModuleInfo.push_back({Name, {}, {}});
2150+
2151+
for (auto *A: Args.filtered(OPT_module_can_import_version)) {
2152+
llvm::VersionTuple Version, UnderlyingVersion;
2153+
if (Version.tryParse(A->getValue(1)))
2154+
Diags.diagnose(SourceLoc(), diag::invalid_can_import_module_version,
2155+
A->getValue(1));
2156+
if (UnderlyingVersion.tryParse(A->getValue(2)))
2157+
Diags.diagnose(SourceLoc(), diag::invalid_can_import_module_version,
2158+
A->getValue(2));
2159+
Opts.CanImportModuleInfo.push_back(
2160+
{A->getValue(0), Version, UnderlyingVersion});
2161+
}
2162+
21472163
Opts.DisableCrossImportOverlaySearch |=
21482164
Args.hasArg(OPT_disable_cross_import_overlay_search);
21492165

0 commit comments

Comments
 (0)