Skip to content

Commit 9fcd202

Browse files
artemcmartemcm
committed
[Dependency Scanning] Do not add underlying Clang moduel to persistent (global) scanner cache
When we are building a Swift module which has an underlying Clang module, and which generates an ObjC interface ('-Swift.h'), the mechanism for building the latter involves a VFS redirect of its modulemap to one that does not yet have the generated Swift code, because it must be built before the Swift portion is built because the Swift portion depends on it. This means that the invocation to build this module is different to one used by the clients which depend on this module. To avoid the subsequent client scans from re-using the partial (VFS-redirected) module, ensure that we do not store dependency info of the underlying Clang module into the global scanner cache. This will cause subsequent client scans to re-scan for this module, and find the fully-resolved modulemap without a VFS redirect. Resolves rdar://88309064
1 parent 445cbd7 commit 9fcd202

File tree

4 files changed

+53
-25
lines changed

4 files changed

+53
-25
lines changed

‎include/swift/AST/ModuleDependencies.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,15 @@ class ModuleDependenciesCache {
610610
/// Additional information needed for Clang dependency scanning.
611611
ClangModuleDependenciesCacheImpl *clangImpl = nullptr;
612612

613+
/// Name of the main Swift module of this scan
614+
StringRef mainModuleName;
615+
/// Underlying Clang module is seen differently by the main
616+
/// module and by module clients. For this reason, we do not wish subsequent
617+
/// scans to be able to re-use this dependency info and therefore we avoid
618+
/// adding it to the global cache. The dependency storage is therefore tied
619+
/// to this, local, cache.
620+
std::unique_ptr<ModuleDependencies> underlyingClangModuleDependency = nullptr;
621+
613622
/// Function that will delete \c clangImpl properly.
614623
void (*clangImplDeleter)(ClangModuleDependenciesCacheImpl *) = nullptr;
615624

@@ -638,7 +647,8 @@ class ModuleDependenciesCache {
638647
Optional<ModuleDependenciesKind> kind) const;
639648

640649
public:
641-
ModuleDependenciesCache(GlobalModuleDependenciesCache &globalCache);
650+
ModuleDependenciesCache(GlobalModuleDependenciesCache &globalCache,
651+
StringRef mainModuleName);
642652
ModuleDependenciesCache(const ModuleDependenciesCache &) = delete;
643653
ModuleDependenciesCache &operator=(const ModuleDependenciesCache &) = delete;
644654
virtual ~ModuleDependenciesCache() { destroyClangImpl(); }

‎lib/AST/ModuleDependencies.cpp

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,9 @@ ModuleDependenciesCache::getDependencyReferencesMap(
517517
}
518518

519519
ModuleDependenciesCache::ModuleDependenciesCache(
520-
GlobalModuleDependenciesCache &globalCache)
521-
: globalCache(globalCache) {
520+
GlobalModuleDependenciesCache &globalCache,
521+
StringRef mainModuleName)
522+
: globalCache(globalCache), mainModuleName(mainModuleName) {
522523
for (auto kind = ModuleDependenciesKind::FirstKind;
523524
kind != ModuleDependenciesKind::LastKind; ++kind) {
524525
ModuleDependenciesMap.insert(
@@ -566,11 +567,21 @@ bool ModuleDependenciesCache::hasDependencies(
566567

567568
void ModuleDependenciesCache::recordDependencies(
568569
StringRef moduleName, ModuleDependencies dependencies) {
569-
auto globalDepPtr = globalCache.recordDependencies(moduleName, dependencies);
570-
auto kind = globalDepPtr->getKind();
571-
auto &map = getDependencyReferencesMap(kind);
570+
auto dependenciesKind = dependencies.getKind();
571+
572+
// The underlying Clang module needs to be cached in this invocation,
573+
// but should not make it to the global cache since it will look slightly
574+
// differently for clients of this module than it does for the module itself.
575+
const ModuleDependencies* recordedDependencies;
576+
if (moduleName == mainModuleName && dependencies.isClangModule()) {
577+
underlyingClangModuleDependency = std::make_unique<ModuleDependencies>(std::move(dependencies));
578+
recordedDependencies = underlyingClangModuleDependency.get();
579+
} else
580+
recordedDependencies = globalCache.recordDependencies(moduleName, dependencies);
581+
582+
auto &map = getDependencyReferencesMap(dependenciesKind);
572583
assert(map.count(moduleName) == 0 && "Already added to map");
573-
map.insert({moduleName, globalDepPtr});
584+
map.insert({moduleName, recordedDependencies});
574585
}
575586

576587
void ModuleDependenciesCache::updateDependencies(

‎lib/DependencyScan/DependencyScanningTool.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ DependencyScanningTool::getDependencies(
7373
auto Instance = std::move(*InstanceOrErr);
7474

7575
// Local scan cache instance, wrapping the shared global cache.
76-
ModuleDependenciesCache cache(*SharedCache);
76+
ModuleDependenciesCache cache(*SharedCache,
77+
Instance->getMainModule()->getNameStr());
7778
// Execute the scanning action, retrieving the in-memory result
7879
auto DependenciesOrErr = performModuleScan(*Instance.get(), cache);
7980
if (DependenciesOrErr.getError())
@@ -113,7 +114,8 @@ DependencyScanningTool::getDependencies(
113114
auto Instance = std::move(*InstanceOrErr);
114115

115116
// Local scan cache instance, wrapping the shared global cache.
116-
ModuleDependenciesCache cache(*SharedCache);
117+
ModuleDependenciesCache cache(*SharedCache,
118+
Instance->getMainModule()->getNameStr());
117119
auto BatchScanResults = performBatchModuleScan(
118120
*Instance.get(), cache, VersionedPCMInstanceCacheCache.get(),
119121
Saver, BatchInput);

‎lib/DependencyScan/ScanDependencies.cpp

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,32 +1089,33 @@ forEachBatchEntry(CompilerInstance &invocationInstance,
10891089

10901090
// Create a new instance by the arguments and save it in the map.
10911091
auto newGlobalCache = std::make_unique<GlobalModuleDependenciesCache>();
1092-
subInstanceMap->insert(
1093-
{entry.arguments,
1094-
std::make_tuple(std::make_unique<CompilerInstance>(),
1095-
std::move(newGlobalCache),
1096-
std::make_unique<ModuleDependenciesCache>(*newGlobalCache))});
1092+
auto newInstance = std::make_unique<CompilerInstance>();
10971093

1098-
pInstance = std::get<0>((*subInstanceMap)[entry.arguments]).get();
1099-
auto globalCache = std::get<1>((*subInstanceMap)[entry.arguments]).get();
1100-
pCache = std::get<2>((*subInstanceMap)[entry.arguments]).get();
11011094
SmallVector<const char *, 4> args;
11021095
llvm::cl::TokenizeGNUCommandLine(entry.arguments, saver, args);
11031096
CompilerInvocation subInvoke = invoke;
1104-
pInstance->addDiagnosticConsumer(&FDC);
1097+
newInstance->addDiagnosticConsumer(&FDC);
11051098
if (subInvoke.parseArgs(args, diags)) {
11061099
invocationInstance.getDiags().diagnose(
11071100
SourceLoc(), diag::scanner_arguments_invalid, entry.arguments);
11081101
return true;
11091102
}
11101103
std::string InstanceSetupError;
1111-
if (pInstance->setup(subInvoke, InstanceSetupError)) {
1104+
if (newInstance->setup(subInvoke, InstanceSetupError)) {
11121105
invocationInstance.getDiags().diagnose(
11131106
SourceLoc(), diag::scanner_arguments_invalid, entry.arguments);
11141107
return true;
11151108
}
1116-
globalCache->configureForTriple(pInstance->getInvocation()
1117-
.getLangOptions().Target.str());
1109+
newGlobalCache->configureForTriple(
1110+
newInstance->getInvocation().getLangOptions().Target.str());
1111+
auto newLocalCache = std::make_unique<ModuleDependenciesCache>(
1112+
*newGlobalCache, newInstance->getMainModule()->getNameStr());
1113+
pInstance = newInstance.get();
1114+
pCache = newLocalCache.get();
1115+
subInstanceMap->insert(
1116+
{entry.arguments,
1117+
std::make_tuple(std::move(newInstance), std::move(newGlobalCache),
1118+
std::move(newLocalCache))});
11181119
}
11191120
assert(pInstance);
11201121
assert(pCache);
@@ -1252,7 +1253,8 @@ bool swift::dependencies::scanDependencies(CompilerInstance &instance) {
12521253
if (opts.ReuseDependencyScannerCache)
12531254
deserializeDependencyCache(instance, globalCache);
12541255

1255-
ModuleDependenciesCache cache(globalCache);
1256+
ModuleDependenciesCache cache(globalCache,
1257+
instance.getMainModule()->getNameStr());
12561258

12571259
// Execute scan
12581260
auto dependenciesOrErr = performModuleScan(instance, cache);
@@ -1287,7 +1289,8 @@ bool swift::dependencies::prescanDependencies(CompilerInstance &instance) {
12871289
GlobalModuleDependenciesCache singleUseGlobalCache;
12881290
singleUseGlobalCache.configureForTriple(instance.getInvocation()
12891291
.getLangOptions().Target.str());
1290-
ModuleDependenciesCache cache(singleUseGlobalCache);
1292+
ModuleDependenciesCache cache(singleUseGlobalCache,
1293+
instance.getMainModule()->getNameStr());
12911294
if (out.has_error() || EC) {
12921295
Context.Diags.diagnose(SourceLoc(), diag::error_opening_output, path,
12931296
EC.message());
@@ -1318,7 +1321,8 @@ bool swift::dependencies::batchScanDependencies(
13181321
GlobalModuleDependenciesCache singleUseGlobalCache;
13191322
singleUseGlobalCache.configureForTriple(instance.getInvocation()
13201323
.getLangOptions().Target.str());
1321-
ModuleDependenciesCache cache(singleUseGlobalCache);
1324+
ModuleDependenciesCache cache(singleUseGlobalCache,
1325+
instance.getMainModule()->getNameStr());
13221326
(void)instance.getMainModule();
13231327
llvm::BumpPtrAllocator alloc;
13241328
llvm::StringSaver saver(alloc);
@@ -1353,7 +1357,8 @@ bool swift::dependencies::batchPrescanDependencies(
13531357
GlobalModuleDependenciesCache singleUseGlobalCache;
13541358
singleUseGlobalCache.configureForTriple(instance.getInvocation()
13551359
.getLangOptions().Target.str());
1356-
ModuleDependenciesCache cache(singleUseGlobalCache);
1360+
ModuleDependenciesCache cache(singleUseGlobalCache,
1361+
instance.getMainModule()->getNameStr());
13571362
(void)instance.getMainModule();
13581363
llvm::BumpPtrAllocator alloc;
13591364
llvm::StringSaver saver(alloc);

0 commit comments

Comments
 (0)