Skip to content

Commit f7d008e

Browse files
authored
Merge pull request #84929 from artemcm/RevertRevertOnDemandBridging
[Dependency Scanning] Refactor batch clang dependency query to preserve only partial results
2 parents 28535a6 + 8335a03 commit f7d008e

File tree

4 files changed

+306
-169
lines changed

4 files changed

+306
-169
lines changed

include/swift/AST/ModuleDependencies.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1187,6 +1187,12 @@ class ModuleDependenciesCache {
11871187
DiagnosticEngine &diags,
11881188
BridgeClangDependencyCallback bridgeClangModule);
11891189

1190+
/// Record Clang module dependency.
1191+
void recordClangDependency(
1192+
const clang::tooling::dependencies::ModuleDeps &dependency,
1193+
DiagnosticEngine &diags,
1194+
BridgeClangDependencyCallback bridgeClangModule);
1195+
11901196
/// Update stored dependencies for the given module.
11911197
void updateDependency(ModuleDependencyID moduleID,
11921198
ModuleDependencyInfo dependencyInfo);

include/swift/DependencyScan/ModuleDependencyScanner.h

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ using LookupModuleOutputCallback = llvm::function_ref<std::string(
3232
clang::tooling::dependencies::ModuleOutputKind)>;
3333
using RemapPathCallback = llvm::function_ref<std::string(StringRef)>;
3434

35+
/// A map from a module id to a collection of import statement infos.
36+
using ImportStatementInfoMap =
37+
std::unordered_map<ModuleDependencyID,
38+
std::vector<ScannerImportStatementInfo>>;
39+
3540
/// A dependency scanning worker which performs filesystem lookup
3641
/// of a named module dependency.
3742
class ModuleDependencyScanningWorker {
@@ -333,6 +338,56 @@ class ModuleDependencyScanner {
333338
/// for a given module name.
334339
Identifier getModuleImportIdentifier(StringRef moduleName);
335340

341+
private:
342+
struct BatchClangModuleLookupResult {
343+
llvm::StringMap<clang::tooling::dependencies::ModuleDeps>
344+
discoveredDependencyInfos;
345+
llvm::StringMap<std::vector<std::string>> visibleModules;
346+
};
347+
348+
/// For the provided collection of unresolved imports
349+
/// belonging to identified Swift dependnecies, execute a parallel
350+
/// query to the Clang dependency scanner for each import's module identifier.
351+
void performParallelClangModuleLookup(
352+
const ImportStatementInfoMap &unresolvedImportsMap,
353+
const ImportStatementInfoMap &unresolvedOptionalImportsMap,
354+
ModuleDependenciesCache &cache,
355+
BatchClangModuleLookupResult &result);
356+
357+
/// Given a result of a batch Clang module dependency lookup,
358+
/// record its results in the cache:
359+
/// 1. Record all discovered Clang module dependency infos
360+
/// in the \c cache.
361+
/// 1. Update the set of visible Clang modules from each Swift module
362+
/// in the \c cache.
363+
/// 2. Update the total collection of all disovered clang modules
364+
/// in \c allDiscoveredClangModules.
365+
/// 3. Record all import identifiers which the scan failed to resolve
366+
/// in \c failedToResolveImports.
367+
/// 4. Update the set of resolved Clang dependencies for each Swift
368+
/// module dependency in \c resolvedClangDependenciesMap.
369+
void cacheComputedClangModuleLookupResults(
370+
const BatchClangModuleLookupResult &lookupResult,
371+
const ImportStatementInfoMap &unresolvedImportsMap,
372+
const ImportStatementInfoMap &unresolvedOptionalImportsMap,
373+
ArrayRef<ModuleDependencyID> swiftModuleDependents,
374+
ModuleDependenciesCache &cache,
375+
ModuleDependencyIDSetVector &allDiscoveredClangModules,
376+
std::vector<std::pair<ModuleDependencyID, ScannerImportStatementInfo>>
377+
&failedToResolveImports,
378+
std::unordered_map<ModuleDependencyID, ModuleDependencyIDSetVector>
379+
&resolvedClangDependenciesMap);
380+
381+
/// Re-query some failed-to-resolve Clang imports from cache
382+
/// in chance they were brought in as transitive dependencies.
383+
void reQueryMissedModulesFromCache(
384+
ModuleDependenciesCache &cache,
385+
const std::vector<
386+
std::pair<ModuleDependencyID, ScannerImportStatementInfo>>
387+
&failedToResolveImports,
388+
std::unordered_map<ModuleDependencyID, ModuleDependencyIDSetVector>
389+
&resolvedClangDependenciesMap);
390+
336391
/// Assuming the \c `moduleImport` failed to resolve,
337392
/// iterate over all binary Swift module dependencies with serialized
338393
/// search paths and attempt to diagnose if the failed-to-resolve module

lib/AST/ModuleDependencies.cpp

Lines changed: 62 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -788,61 +788,68 @@ void ModuleDependenciesCache::recordClangDependencies(
788788
const clang::tooling::dependencies::ModuleDepsGraph &dependencies,
789789
DiagnosticEngine &diags,
790790
BridgeClangDependencyCallback bridgeClangModule) {
791-
for (const auto &dep : dependencies) {
792-
auto depID =
793-
ModuleDependencyID{dep.ID.ModuleName, ModuleDependencyKind::Clang};
794-
if (hasDependency(depID)) {
795-
auto priorClangModuleDetails =
796-
findKnownDependency(depID).getAsClangModule();
797-
DEBUG_ASSERT(priorClangModuleDetails);
798-
auto priorContextHash = priorClangModuleDetails->contextHash;
799-
auto newContextHash = dep.ID.ContextHash;
800-
if (priorContextHash != newContextHash) {
801-
// This situation means that within the same scanning action, Clang
802-
// Dependency Scanner has produced two different variants of the same
803-
// module. This is not supposed to happen, but we are currently
804-
// hunting down the rare cases where it does, seemingly due to
805-
// differences in Clang Scanner direct by-name queries and transitive
806-
// header lookup queries.
807-
//
808-
// Emit a failure diagnostic here that is hopefully more actionable
809-
// for the time being.
810-
diags.diagnose(SourceLoc(),
811-
diag::dependency_scan_unexpected_variant,
812-
dep.ID.ModuleName);
813-
diags.diagnose(
814-
SourceLoc(),
815-
diag::dependency_scan_unexpected_variant_context_hash_note,
816-
priorContextHash, newContextHash);
817-
diags.diagnose(
818-
SourceLoc(),
819-
diag::dependency_scan_unexpected_variant_module_map_note,
820-
priorClangModuleDetails->moduleMapFile, dep.ClangModuleMapFile);
821-
822-
auto newClangModuleDetails = bridgeClangModule(dep).getAsClangModule();
823-
auto diagnoseExtraCommandLineFlags =
824-
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
825-
const ClangModuleDependencyStorage *baseModuleDetails,
826-
bool isNewlyDiscovered) -> void {
827-
std::unordered_set<std::string> baseCommandLineSet(
828-
baseModuleDetails->buildCommandLine.begin(),
829-
baseModuleDetails->buildCommandLine.end());
830-
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
831-
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
832-
diags.diagnose(
833-
SourceLoc(),
834-
diag::dependency_scan_unexpected_variant_extra_arg_note,
835-
isNewlyDiscovered, checkArg);
836-
};
837-
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
838-
newClangModuleDetails, true);
839-
diagnoseExtraCommandLineFlags(newClangModuleDetails,
840-
priorClangModuleDetails, false);
841-
}
842-
} else {
843-
recordDependency(dep.ID.ModuleName, bridgeClangModule(dep));
844-
addSeenClangModule(dep.ID);
845-
}
791+
for (const auto &dep : dependencies)
792+
recordClangDependency(dep, diags, bridgeClangModule);
793+
}
794+
795+
void ModuleDependenciesCache::recordClangDependency(
796+
const clang::tooling::dependencies::ModuleDeps &dependency,
797+
DiagnosticEngine &diags,
798+
BridgeClangDependencyCallback bridgeClangModule) {
799+
auto depID =
800+
ModuleDependencyID{dependency.ID.ModuleName, ModuleDependencyKind::Clang};
801+
if (!hasDependency(depID)) {
802+
recordDependency(dependency.ID.ModuleName, bridgeClangModule(dependency));
803+
addSeenClangModule(dependency.ID);
804+
return;
805+
}
806+
807+
auto priorClangModuleDetails =
808+
findKnownDependency(depID).getAsClangModule();
809+
DEBUG_ASSERT(priorClangModuleDetails);
810+
auto priorContextHash = priorClangModuleDetails->contextHash;
811+
auto newContextHash = dependency.ID.ContextHash;
812+
if (priorContextHash != newContextHash) {
813+
// This situation means that within the same scanning action, Clang
814+
// Dependency Scanner has produced two different variants of the same
815+
// module. This is not supposed to happen, but we are currently
816+
// hunting down the rare cases where it does, seemingly due to
817+
// differences in Clang Scanner direct by-name queries and transitive
818+
// header lookup queries.
819+
//
820+
// Emit a failure diagnostic here that is hopefully more actionable
821+
// for the time being.
822+
diags.diagnose(SourceLoc(),
823+
diag::dependency_scan_unexpected_variant,
824+
dependency.ID.ModuleName);
825+
diags.diagnose(
826+
SourceLoc(),
827+
diag::dependency_scan_unexpected_variant_context_hash_note,
828+
priorContextHash, newContextHash);
829+
diags.diagnose(
830+
SourceLoc(),
831+
diag::dependency_scan_unexpected_variant_module_map_note,
832+
priorClangModuleDetails->moduleMapFile, dependency.ClangModuleMapFile);
833+
834+
auto newClangModuleDetails = bridgeClangModule(dependency).getAsClangModule();
835+
auto diagnoseExtraCommandLineFlags =
836+
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
837+
const ClangModuleDependencyStorage *baseModuleDetails,
838+
bool isNewlyDiscovered) -> void {
839+
std::unordered_set<std::string> baseCommandLineSet(
840+
baseModuleDetails->buildCommandLine.begin(),
841+
baseModuleDetails->buildCommandLine.end());
842+
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
843+
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
844+
diags.diagnose(
845+
SourceLoc(),
846+
diag::dependency_scan_unexpected_variant_extra_arg_note,
847+
isNewlyDiscovered, checkArg);
848+
};
849+
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
850+
newClangModuleDetails, true);
851+
diagnoseExtraCommandLineFlags(newClangModuleDetails,
852+
priorClangModuleDetails, false);
846853
}
847854
}
848855

0 commit comments

Comments
 (0)