Skip to content

Commit 5fa19d4

Browse files
Merge pull request swiftlang#74105 from cachemeifyoucan/eng/PR-128067152-6.0
[6.0][ScanDependency][canImport] Improve canImport handling in explicit build
2 parents 50f7123 + a300933 commit 5fa19d4

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
@@ -73,6 +73,7 @@
7373
#include "llvm/Support/Allocator.h"
7474
#include "llvm/Support/Compiler.h"
7575
#include "llvm/Support/FormatVariadic.h"
76+
#include "llvm/Support/VersionTuple.h"
7677
#include "llvm/Support/VirtualOutputBackend.h"
7778
#include "llvm/Support/VirtualOutputBackends.h"
7879
#include <algorithm>
@@ -785,6 +786,12 @@ ASTContext::ASTContext(
785786
registerAccessRequestFunctions(evaluator);
786787
registerNameLookupRequestFunctions(evaluator);
787788

789+
// Register canImport module info.
790+
for (auto &info: SearchPathOpts.CanImportModuleInfo) {
791+
addSucceededCanImportModule(info.ModuleName, false, info.Version);
792+
addSucceededCanImportModule(info.ModuleName, true, info.UnderlyingVersion);
793+
}
794+
788795
// Provide a default OnDiskOutputBackend if user didn't supply one.
789796
if (!OutputBackend)
790797
OutputBackend = llvm::makeIntrusiveRefCnt<llvm::vfs::OnDiskOutputBackend>();
@@ -2384,23 +2391,56 @@ getModuleVersionKindString(const ModuleLoader::ModuleVersionInfo &info) {
23842391
}
23852392
}
23862393

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

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

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

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. For checking modules that are
2436+
// not passed from command-line, allow fallback to the module loading since
2437+
// this is not in a canImport request context that has already been resolved
2438+
// by scanner.
2439+
if (!SearchPathOpts.CanImportModuleInfo.empty())
2440+
return false;
2441+
}
2442+
2443+
if (version.empty()) {
24042444
// If this module has already been successfully imported, it is importable.
24052445
if (getLoadedModule(ModuleName) != nullptr)
24062446
return true;
@@ -2452,28 +2492,38 @@ bool ASTContext::canImportModuleImpl(ImportPath::Module ModuleName,
24522492
return true;
24532493
}
24542494

2495+
foundVersion = bestVersionInfo.getVersion();
24552496
return version <= bestVersionInfo.getVersion();
24562497
}
24572498

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

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

24732521
bool ASTContext::testImportModule(ImportPath::Module ModuleName,
24742522
llvm::VersionTuple version,
24752523
bool underlyingVersion) const {
2476-
return canImportModuleImpl(ModuleName, version, underlyingVersion, false);
2524+
llvm::VersionTuple versionInfo;
2525+
return canImportModuleImpl(ModuleName, version, underlyingVersion, false,
2526+
versionInfo);
24772527
}
24782528

24792529
ModuleDecl *

lib/ClangImporter/ClangImporter.cpp

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

2109-
static llvm::VersionTuple getCurrentVersionFromTBD(StringRef path,
2109+
static llvm::VersionTuple getCurrentVersionFromTBD(llvm::vfs::FileSystem &FS,
2110+
StringRef path,
21102111
StringRef moduleName) {
21112112
std::string fwName = (moduleName + ".framework").str();
21122113
auto pos = path.find(fwName);
@@ -2116,7 +2117,7 @@ static llvm::VersionTuple getCurrentVersionFromTBD(StringRef path,
21162117
llvm::sys::path::append(buffer, moduleName + ".tbd");
21172118
auto tbdPath = buffer.str();
21182119
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> tbdBufOrErr =
2119-
llvm::MemoryBuffer::getFile(tbdPath);
2120+
FS.getBufferForFile(tbdPath);
21202121
// .tbd file doesn't exist, exit.
21212122
if (!tbdBufOrErr)
21222123
return {};
@@ -2179,8 +2180,8 @@ bool ClangImporter::canImportModule(ImportPath::Module modulePath,
21792180

21802181
// Look for the .tbd file inside .framework dir to get the project version
21812182
// number.
2182-
llvm::VersionTuple currentVersion =
2183-
getCurrentVersionFromTBD(path, topModule.Item.str());
2183+
llvm::VersionTuple currentVersion = getCurrentVersionFromTBD(
2184+
Impl.Instance->getVirtualFileSystem(), path, topModule.Item.str());
21842185
versionInfo->setVersion(currentVersion,
21852186
ModuleVersionSourceKind::ClangModuleTBD);
21862187
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

@@ -453,15 +454,26 @@ ModuleDependencyScanner::getMainModuleDependencyInfo(ModuleDecl *mainModule) {
453454
&ScanASTContext.SourceMgr);
454455
}
455456

456-
// Add all the successful canImport checks from the ASTContext as part of
457-
// the dependency since only mainModule can have `canImport` check. This
458-
// needs to happen after visiting all the top-level decls from all
457+
// Pass all the successful canImport checks from the ASTContext as part of
458+
// build command to main module to ensure frontend gets the same result.
459+
// This needs to happen after visiting all the top-level decls from all
459460
// SourceFiles.
460-
for (auto &Module :
461-
mainModule->getASTContext().getSuccessfulCanImportCheckNames())
462-
mainDependencies.addModuleImport(Module.first(),
463-
&alreadyAddedModules);
464-
}
461+
auto buildArgs = mainDependencies.getCommandline();
462+
mainModule->getASTContext().forEachCanImportVersionCheck(
463+
[&](StringRef moduleName, const llvm::VersionTuple &Version,
464+
const llvm::VersionTuple &UnderlyingVersion) {
465+
if (Version.empty() && UnderlyingVersion.empty()) {
466+
buildArgs.push_back("-module-can-import");
467+
buildArgs.push_back(moduleName.str());
468+
} else {
469+
buildArgs.push_back("-module-can-import-version");
470+
buildArgs.push_back(moduleName.str());
471+
buildArgs.push_back(Version.getAsString());
472+
buildArgs.push_back(UnderlyingVersion.getAsString());
473+
}
474+
});
475+
mainDependencies.updateCommandLine(buildArgs);
476+
}
465477

466478
return mainDependencies;
467479
}

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

2084+
for (auto &Name : Args.getAllArgValues(OPT_module_can_import))
2085+
Opts.CanImportModuleInfo.push_back({Name, {}, {}});
2086+
2087+
for (auto *A: Args.filtered(OPT_module_can_import_version)) {
2088+
llvm::VersionTuple Version, UnderlyingVersion;
2089+
if (Version.tryParse(A->getValue(1)))
2090+
Diags.diagnose(SourceLoc(), diag::invalid_can_import_module_version,
2091+
A->getValue(1));
2092+
if (UnderlyingVersion.tryParse(A->getValue(2)))
2093+
Diags.diagnose(SourceLoc(), diag::invalid_can_import_module_version,
2094+
A->getValue(2));
2095+
Opts.CanImportModuleInfo.push_back(
2096+
{A->getValue(0), Version, UnderlyingVersion});
2097+
}
2098+
20832099
Opts.DisableCrossImportOverlaySearch |=
20842100
Args.hasArg(OPT_disable_cross_import_overlay_search);
20852101

0 commit comments

Comments
 (0)