Skip to content

Commit 674dfb3

Browse files
artemcmartemcm
committed
[Dependency Scanning] Move generation of a named import path 'Identifier' out of the individual scanning workers up into the parent scanner. This operation mutates the scanner ASTContext by potentially adding new identifiers to it and is therefore not thread-safe.
1 parent ff63a90 commit 674dfb3

File tree

10 files changed

+48
-35
lines changed

10 files changed

+48
-35
lines changed

‎include/swift/AST/ModuleLoader.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ class ModuleLoader {
341341
/// Retrieve the dependencies for the given, named module, or \c None
342342
/// if no such module exists.
343343
virtual llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
344-
getModuleDependencies(StringRef moduleName,
344+
getModuleDependencies(Identifier moduleName,
345345
StringRef moduleOutputPath,
346346
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
347347
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,

‎include/swift/ClangImporter/ClangImporter.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ class ClangImporter final : public ClangModuleLoader {
440440
StringRef moduleOutputPath, RemapPathCallback remapPath = nullptr);
441441

442442
llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
443-
getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
443+
getModuleDependencies(Identifier moduleName, StringRef moduleOutputPath,
444444
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
445445
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
446446
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,

‎include/swift/DependencyScan/ModuleDependencyScanner.h

+6-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "swift/AST/ASTContext.h"
15+
#include "swift/AST/Identifier.h"
1516
#include "swift/AST/ModuleDependencies.h"
1617
#include "swift/Frontend/ModuleInterfaceLoader.h"
1718
#include "swift/Serialization/SerializedModuleLoader.h"
@@ -36,18 +37,18 @@ class ModuleDependencyScanningWorker {
3637
private:
3738
/// Retrieve the module dependencies for the module with the given name.
3839
ModuleDependencyVector
39-
scanFilesystemForModuleDependency(StringRef moduleName,
40+
scanFilesystemForModuleDependency(Identifier moduleName,
4041
const ModuleDependenciesCache &cache,
4142
bool isTestableImport = false);
4243

4344
/// Retrieve the module dependencies for the Clang module with the given name.
4445
ModuleDependencyVector
45-
scanFilesystemForClangModuleDependency(StringRef moduleName,
46+
scanFilesystemForClangModuleDependency(Identifier moduleName,
4647
const ModuleDependenciesCache &cache);
4748

4849
/// Retrieve the module dependencies for the Swift module with the given name.
4950
ModuleDependencyVector
50-
scanFilesystemForSwiftModuleDependency(StringRef moduleName,
51+
scanFilesystemForSwiftModuleDependency(Identifier moduleName,
5152
const ModuleDependenciesCache &cache);
5253

5354
// An AST delegate for interface scanning.
@@ -135,6 +136,8 @@ class ModuleDependencyScanner {
135136
template <typename Function, typename... Args>
136137
auto withDependencyScanningWorker(Function &&F, Args &&...ArgList);
137138

139+
Identifier getModuleImportIdentifier(StringRef moduleName);
140+
138141
private:
139142
const CompilerInvocation &ScanCompilerInvocation;
140143
ASTContext &ScanASTContext;

‎include/swift/Sema/SourceLoader.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class SourceLoader : public ModuleLoader {
9898
}
9999

100100
llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
101-
getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
101+
getModuleDependencies(Identifier moduleName, StringRef moduleOutputPath,
102102
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
103103
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
104104
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,

‎include/swift/Serialization/SerializedModuleLoader.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
250250
virtual void verifyAllModules() override;
251251

252252
virtual llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
253-
getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
253+
getModuleDependencies(Identifier moduleName, StringRef moduleOutputPath,
254254
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
255255
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
256256
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,

‎lib/AST/SearchPathOptions.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,12 @@ ModuleSearchPathLookup::searchPathsContainingFile(
101101
llvm::SmallSet<std::pair<ModuleSearchPathKind, unsigned>, 4> ResultIds;
102102

103103
for (auto &Filename : Filenames) {
104-
for (auto &Entry : LookupTable.lookup(Filename)) {
105-
if (ResultIds.insert(std::make_pair(Entry->getKind(), Entry->getIndex()))
106-
.second) {
107-
Result.push_back(Entry.get());
104+
if (LookupTable.contains(Filename)) {
105+
for (auto &Entry : LookupTable.at(Filename)) {
106+
if (ResultIds.insert(std::make_pair(Entry->getKind(), Entry->getIndex()))
107+
.second) {
108+
Result.push_back(Entry.get());
109+
}
108110
}
109111
}
110112
}

‎lib/ClangImporter/ClangModuleDependencyScanner.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ computeClangWorkingDirectory(const std::vector<std::string> &commandLineArgs,
407407
}
408408

409409
ModuleDependencyVector
410-
ClangImporter::getModuleDependencies(StringRef moduleName,
410+
ClangImporter::getModuleDependencies(Identifier moduleName,
411411
StringRef moduleOutputPath,
412412
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
413413
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
@@ -435,14 +435,14 @@ ClangImporter::getModuleDependencies(StringRef moduleName,
435435

436436
auto clangModuleDependencies =
437437
clangScanningTool.getModuleDependencies(
438-
moduleName, commandLineArgs, workingDir,
438+
moduleName.str(), commandLineArgs, workingDir,
439439
alreadySeenClangModules, lookupModuleOutput);
440440
if (!clangModuleDependencies) {
441441
auto errorStr = toString(clangModuleDependencies.takeError());
442442
// We ignore the "module 'foo' not found" error, the Swift dependency
443443
// scanner will report such an error only if all of the module loaders
444444
// fail as well.
445-
if (errorStr.find("fatal error: module '" + moduleName.str() +
445+
if (errorStr.find("fatal error: module '" + moduleName.str().str() +
446446
"' not found") == std::string::npos)
447447
ctx.Diags.diagnose(SourceLoc(), diag::clang_dependency_scan_error,
448448
errorStr);

‎lib/DependencyScan/ModuleDependencyScanner.cpp

+25-17
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ ModuleDependencyScanningWorker::ModuleDependencyScanningWorker(
179179

180180
ModuleDependencyVector
181181
ModuleDependencyScanningWorker::scanFilesystemForModuleDependency(
182-
StringRef moduleName, const ModuleDependenciesCache &cache,
182+
Identifier moduleName, const ModuleDependenciesCache &cache,
183183
bool isTestableImport) {
184184
// First query a Swift module, otherwise lookup a Clang module
185185
ModuleDependencyVector moduleDependencies =
@@ -203,7 +203,7 @@ ModuleDependencyScanningWorker::scanFilesystemForModuleDependency(
203203

204204
ModuleDependencyVector
205205
ModuleDependencyScanningWorker::scanFilesystemForSwiftModuleDependency(
206-
StringRef moduleName, const ModuleDependenciesCache &cache) {
206+
Identifier moduleName, const ModuleDependenciesCache &cache) {
207207
return swiftScannerModuleLoader->getModuleDependencies(
208208
moduleName, cache.getModuleOutputPath(),
209209
cache.getScanService().getCachingFS(), cache.getAlreadySeenClangModules(),
@@ -213,7 +213,7 @@ ModuleDependencyScanningWorker::scanFilesystemForSwiftModuleDependency(
213213

214214
ModuleDependencyVector
215215
ModuleDependencyScanningWorker::scanFilesystemForClangModuleDependency(
216-
StringRef moduleName, const ModuleDependenciesCache &cache) {
216+
Identifier moduleName, const ModuleDependenciesCache &cache) {
217217
return clangScannerModuleLoader->getModuleDependencies(
218218
moduleName, cache.getModuleOutputPath(),
219219
cache.getScanService().getCachingFS(), cache.getAlreadySeenClangModules(),
@@ -252,6 +252,10 @@ auto ModuleDependencyScanner::withDependencyScanningWorker(Function &&F,
252252
return result;
253253
}
254254

255+
Identifier ModuleDependencyScanner::getModuleImportIdentifier(StringRef moduleName) {
256+
return ScanASTContext.getIdentifier(moduleName);
257+
}
258+
255259
ModuleDependencyScanner::ModuleDependencyScanner(
256260
SwiftDependencyScanningService &ScanningService,
257261
const CompilerInvocation &ScanCompilerInvocation,
@@ -443,10 +447,11 @@ ModuleDependencyScanner::getNamedClangModuleDependencyInfo(
443447
return found;
444448

445449
// Otherwise perform filesystem scan
450+
auto moduleIdentifier = getModuleImportIdentifier(moduleName);
446451
auto moduleDependencies = withDependencyScanningWorker(
447-
[&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
452+
[&cache, moduleIdentifier](ModuleDependencyScanningWorker *ScanningWorker) {
448453
return ScanningWorker->scanFilesystemForClangModuleDependency(
449-
moduleName, cache);
454+
moduleIdentifier, cache);
450455
});
451456
if (moduleDependencies.empty())
452457
return llvm::None;
@@ -474,10 +479,11 @@ ModuleDependencyScanner::getNamedSwiftModuleDependencyInfo(
474479
return found;
475480

476481
// Otherwise perform filesystem scan
482+
auto moduleIdentifier = getModuleImportIdentifier(moduleName);
477483
auto moduleDependencies = withDependencyScanningWorker(
478-
[&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
484+
[&cache, moduleIdentifier](ModuleDependencyScanningWorker *ScanningWorker) {
479485
return ScanningWorker->scanFilesystemForSwiftModuleDependency(
480-
moduleName, cache);
486+
moduleIdentifier, cache);
481487
});
482488
if (moduleDependencies.empty())
483489
return llvm::None;
@@ -552,8 +558,9 @@ void ModuleDependencyScanner::resolveImportDependencies(
552558
// A scanning task to query a module by-name. If the module already exists
553559
// in the cache, do nothing and return.
554560
auto scanForModuleDependency = [this, &cache, &moduleLookupResult](
555-
StringRef moduleName, bool onlyClangModule,
561+
Identifier moduleIdentifier, bool onlyClangModule,
556562
bool isTestable) {
563+
auto moduleName = moduleIdentifier.str();
557564
// If this is already in the cache, no work to do here
558565
if (onlyClangModule) {
559566
if (cache.hasDependency(moduleName, ModuleDependencyKind::Clang))
@@ -564,13 +571,13 @@ void ModuleDependencyScanner::resolveImportDependencies(
564571
}
565572

566573
auto moduleDependencies = withDependencyScanningWorker(
567-
[&cache, moduleName, onlyClangModule,
574+
[&cache, moduleIdentifier, onlyClangModule,
568575
isTestable](ModuleDependencyScanningWorker *ScanningWorker) {
569576
return onlyClangModule
570577
? ScanningWorker->scanFilesystemForClangModuleDependency(
571-
moduleName, cache)
578+
moduleIdentifier, cache)
572579
: ScanningWorker->scanFilesystemForModuleDependency(
573-
moduleName, cache, isTestable);
580+
moduleIdentifier, cache, isTestable);
574581
});
575582
moduleLookupResult.insert_or_assign(moduleName, moduleDependencies);
576583
};
@@ -579,14 +586,14 @@ void ModuleDependencyScanner::resolveImportDependencies(
579586
for (const auto &dependsOn : moduleDependencyInfo->getModuleImports()) {
580587
bool underlyingClangModuleLookup = moduleID.ModuleName == dependsOn;
581588
bool isTestable = moduleDependencyInfo->isTestableImport(dependsOn);
582-
ScanningThreadPool.async(scanForModuleDependency, dependsOn,
589+
ScanningThreadPool.async(scanForModuleDependency, getModuleImportIdentifier(dependsOn),
583590
underlyingClangModuleLookup, isTestable);
584591
}
585592
for (const auto &dependsOn :
586593
moduleDependencyInfo->getOptionalModuleImports()) {
587594
bool underlyingClangModuleLookup = moduleID.ModuleName == dependsOn;
588595
bool isTestable = moduleDependencyInfo->isTestableImport(dependsOn);
589-
ScanningThreadPool.async(scanForModuleDependency, dependsOn,
596+
ScanningThreadPool.async(scanForModuleDependency, getModuleImportIdentifier(dependsOn),
590597
underlyingClangModuleLookup, isTestable);
591598
}
592599
ScanningThreadPool.wait();
@@ -726,23 +733,24 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependencies(
726733
// A scanning task to query a Swift module by-name. If the module already
727734
// exists in the cache, do nothing and return.
728735
auto scanForSwiftDependency = [this, &cache, &swiftOverlayLookupResult](
729-
StringRef moduleName) {
736+
Identifier moduleIdentifier) {
737+
auto moduleName = moduleIdentifier.str();
730738
if (cache.hasDependency(moduleName, ModuleDependencyKind::SwiftInterface) ||
731739
cache.hasDependency(moduleName, ModuleDependencyKind::SwiftBinary) ||
732740
cache.hasDependency(moduleName, ModuleDependencyKind::SwiftPlaceholder))
733741
return;
734742

735743
auto moduleDependencies = withDependencyScanningWorker(
736-
[&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
737-
return ScanningWorker->scanFilesystemForSwiftModuleDependency(moduleName,
744+
[&cache, moduleIdentifier](ModuleDependencyScanningWorker *ScanningWorker) {
745+
return ScanningWorker->scanFilesystemForSwiftModuleDependency(moduleIdentifier,
738746
cache);
739747
});
740748
swiftOverlayLookupResult.insert_or_assign(moduleName, moduleDependencies);
741749
};
742750

743751
// Enque asynchronous lookup tasks
744752
for (const auto &clangDep : clangDependencies)
745-
ScanningThreadPool.async(scanForSwiftDependency, clangDep);
753+
ScanningThreadPool.async(scanForSwiftDependency, getModuleImportIdentifier(clangDep));
746754
ScanningThreadPool.wait();
747755

748756
// Aggregate both previously-cached and freshly-scanned module results

‎lib/Sema/SourceLoader.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ void SourceLoader::loadExtensions(NominalTypeDecl *nominal,
155155
}
156156

157157
ModuleDependencyVector
158-
SourceLoader::getModuleDependencies(StringRef moduleName,
158+
SourceLoader::getModuleDependencies(Identifier moduleName,
159159
StringRef moduleOutputPath,
160160
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
161161
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,

‎lib/Serialization/ScanningLoaders.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -305,14 +305,14 @@ SwiftModuleScanner::scanInterfaceFile(Twine moduleInterfacePath,
305305
}
306306

307307
ModuleDependencyVector SerializedModuleLoaderBase::getModuleDependencies(
308-
StringRef moduleName, StringRef moduleOutputPath,
308+
Identifier moduleName, StringRef moduleOutputPath,
309309
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
310310
const llvm::DenseSet<clang::tooling::dependencies::ModuleID>
311311
&alreadySeenClangModules,
312312
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,
313313
InterfaceSubContextDelegate &delegate, llvm::TreePathPrefixMapper *mapper,
314314
bool isTestableDependencyLookup) {
315-
ImportPath::Module::Builder builder(Ctx, moduleName, /*separator=*/'.');
315+
ImportPath::Module::Builder builder(moduleName);
316316
auto modulePath = builder.get();
317317
auto moduleId = modulePath.front().Item;
318318
llvm::Optional<SwiftDependencyTracker> tracker = llvm::None;
@@ -344,7 +344,7 @@ ModuleDependencyVector SerializedModuleLoaderBase::getModuleDependencies(
344344

345345
ModuleDependencyVector moduleDependnecies;
346346
moduleDependnecies.push_back(
347-
std::make_pair(ModuleDependencyID{moduleName.str(),
347+
std::make_pair(ModuleDependencyID{moduleName.str().str(),
348348
scanner->dependencies->getKind()},
349349
*(scanner->dependencies)));
350350
return moduleDependnecies;

0 commit comments

Comments
 (0)