Skip to content

Commit 0aa0687

Browse files
Merge pull request swiftlang#73853 from cachemeifyoucan/eng/PR-128067152
[ScanDependency][canImport] Improve canImport handling in explicit build
2 parents 51318b7 + 026fcd2 commit 0aa0687

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"
@@ -2118,6 +2119,21 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
21182119
for (auto *A : Args.filtered(OPT_swift_module_cross_import))
21192120
Opts.CrossImportInfo[A->getValue(0)].push_back(A->getValue(1));
21202121

2122+
for (auto &Name : Args.getAllArgValues(OPT_module_can_import))
2123+
Opts.CanImportModuleInfo.push_back({Name, {}, {}});
2124+
2125+
for (auto *A: Args.filtered(OPT_module_can_import_version)) {
2126+
llvm::VersionTuple Version, UnderlyingVersion;
2127+
if (Version.tryParse(A->getValue(1)))
2128+
Diags.diagnose(SourceLoc(), diag::invalid_can_import_module_version,
2129+
A->getValue(1));
2130+
if (UnderlyingVersion.tryParse(A->getValue(2)))
2131+
Diags.diagnose(SourceLoc(), diag::invalid_can_import_module_version,
2132+
A->getValue(2));
2133+
Opts.CanImportModuleInfo.push_back(
2134+
{A->getValue(0), Version, UnderlyingVersion});
2135+
}
2136+
21212137
Opts.DisableCrossImportOverlaySearch |=
21222138
Args.hasArg(OPT_disable_cross_import_overlay_search);
21232139

0 commit comments

Comments
 (0)