Skip to content

Commit b4f2821

Browse files
artemcmartemcm
authored
Merge pull request #83050 from artemcm/AdoptVisibleClangModulesForOverlayDiscovery
[Dependency Scanning] Restrict Swift overlay lookup to "visible" Clang modules only
2 parents 328dd1f + 27305d6 commit b4f2821

File tree

8 files changed

+345
-168
lines changed

8 files changed

+345
-168
lines changed

‎include/swift/AST/ModuleDependencies.h

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include "swift/AST/Import.h"
2222
#include "swift/AST/LinkLibrary.h"
23+
#include "swift/Basic/Assertions.h"
2324
#include "swift/Basic/CXXStdlibKind.h"
2425
#include "swift/Basic/LLVM.h"
2526
#include "swift/Serialization/Validation.h"
@@ -212,6 +213,11 @@ class ModuleDependencyInfoStorageBase {
212213
/// The macro dependencies.
213214
std::map<std::string, MacroPluginDependency> macroDependencies;
214215

216+
/// A list of Clang modules that are visible to this Swift module. This
217+
/// includes both direct Clang modules as well as transitive Clang
218+
/// module dependencies when they are exported
219+
llvm::StringSet<> visibleClangModules;
220+
215221
/// ModuleDependencyInfo is finalized (with all transitive dependencies
216222
/// and inputs).
217223
bool finalized;
@@ -816,7 +822,17 @@ class ModuleDependencyInfo {
816822
cast<ClangModuleDependencyStorage>(storage.get())->CASFileSystemRootID =
817823
rootID;
818824
else
819-
llvm_unreachable("Unexpected type");
825+
llvm_unreachable("Unexpected module dependency kind");
826+
}
827+
828+
llvm::StringSet<> &getVisibleClangModules() const {
829+
return storage->visibleClangModules;
830+
}
831+
832+
void
833+
addVisibleClangModules(const std::vector<std::string> &moduleNames) const {
834+
storage->visibleClangModules.insert(moduleNames.begin(),
835+
moduleNames.end());
820836
}
821837

822838
/// Whether explicit input paths of all the module dependencies
@@ -1057,6 +1073,9 @@ class ModuleDependenciesCache {
10571073
/// Query all cross-import overlay dependencies
10581074
llvm::ArrayRef<ModuleDependencyID>
10591075
getCrossImportOverlayDependencies(const ModuleDependencyID &moduleID) const;
1076+
/// Query all visible Clang modules for a given Swift dependency
1077+
llvm::StringSet<>&
1078+
getVisibleClangModules(ModuleDependencyID moduleID) const;
10601079

10611080
/// Look for module dependencies for a module with the given ID
10621081
///
@@ -1087,9 +1106,9 @@ class ModuleDependenciesCache {
10871106
void recordDependency(StringRef moduleName,
10881107
ModuleDependencyInfo dependencies);
10891108

1090-
/// Record dependencies for the given module collection.
1091-
void recordDependencies(ModuleDependencyVector moduleDependencies,
1092-
DiagnosticEngine &diags);
1109+
/// Record dependencies for the given collection of Clang modules.
1110+
void recordClangDependencies(ModuleDependencyVector moduleDependencies,
1111+
DiagnosticEngine &diags);
10931112

10941113
/// Update stored dependencies for the given module.
10951114
void updateDependency(ModuleDependencyID moduleID,
@@ -1122,6 +1141,10 @@ class ModuleDependenciesCache {
11221141
void
11231142
setCrossImportOverlayDependencies(ModuleDependencyID moduleID,
11241143
const ArrayRef<ModuleDependencyID> dependencyIDs);
1144+
/// Add to this module's set of visible Clang modules
1145+
void
1146+
addVisibleClangModules(ModuleDependencyID moduleID,
1147+
const std::vector<std::string> &moduleNames);
11251148

11261149
StringRef getMainModuleName() const { return mainScanModuleName; }
11271150

‎include/swift/DependencyScan/ModuleDependencyScanner.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class ModuleDependencyScanningWorker {
4040

4141
private:
4242
/// Retrieve the module dependencies for the Clang module with the given name.
43-
ModuleDependencyVector scanFilesystemForClangModuleDependency(
43+
ClangModuleScannerQueryResult scanFilesystemForClangModuleDependency(
4444
Identifier moduleName,
4545
const llvm::DenseSet<clang::tooling::dependencies::ModuleID>
4646
&alreadySeenModules);
@@ -72,6 +72,7 @@ class ModuleDependencyScanningWorker {
7272
ModuleDependencyIDSetVector &headerClangModuleDependencies,
7373
std::vector<std::string> &headerFileInputs,
7474
std::vector<std::string> &bridgingHeaderCommandLine,
75+
std::vector<std::string> &visibleClangModules,
7576
std::optional<std::string> &includeTreeID);
7677

7778

‎include/swift/Serialization/ScanningLoaders.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,18 @@ struct SwiftModuleScannerQueryResult {
4040
std::vector<IncompatibleCandidate> incompatibleCandidates;
4141
};
4242

43+
/// Result of looking up a Clang module on the current filesystem
44+
/// search paths.
45+
struct ClangModuleScannerQueryResult {
46+
ClangModuleScannerQueryResult(const ModuleDependencyVector &dependencyModuleGraph,
47+
const std::vector<std::string> &visibleModuleIdentifiers)
48+
: foundDependencyModuleGraph(dependencyModuleGraph),
49+
visibleModuleIdentifiers(visibleModuleIdentifiers) {}
50+
51+
ModuleDependencyVector foundDependencyModuleGraph;
52+
std::vector<std::string> visibleModuleIdentifiers;
53+
};
54+
4355
/// A module "loader" that looks for .swiftinterface and .swiftmodule files
4456
/// for the purpose of determining dependencies, but does not attempt to
4557
/// load the module files.

‎lib/AST/ModuleDependencies.cpp

Lines changed: 68 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -778,65 +778,62 @@ void ModuleDependenciesCache::recordDependency(
778778
map.insert({moduleName, dependency});
779779
}
780780

781-
void ModuleDependenciesCache::recordDependencies(
781+
void ModuleDependenciesCache::recordClangDependencies(
782782
ModuleDependencyVector dependencies, DiagnosticEngine &diags) {
783783
for (const auto &dep : dependencies) {
784+
ASSERT(dep.first.Kind == ModuleDependencyKind::Clang);
785+
auto newClangModuleDetails = dep.second.getAsClangModule();
784786
if (hasDependency(dep.first)) {
785-
if (dep.first.Kind == ModuleDependencyKind::Clang) {
786-
auto priorClangModuleDetails =
787-
findKnownDependency(dep.first).getAsClangModule();
788-
auto newClangModuleDetails = dep.second.getAsClangModule();
789-
auto priorContextHash = priorClangModuleDetails->contextHash;
790-
auto newContextHash = newClangModuleDetails->contextHash;
791-
if (priorContextHash != newContextHash) {
792-
// This situation means that within the same scanning action, Clang
793-
// Dependency Scanner has produced two different variants of the same
794-
// module. This is not supposed to happen, but we are currently
795-
// hunting down the rare cases where it does, seemingly due to
796-
// differences in Clang Scanner direct by-name queries and transitive
797-
// header lookup queries.
798-
//
799-
// Emit a failure diagnostic here that is hopefully more actionable
800-
// for the time being.
801-
diags.diagnose(SourceLoc(), diag::dependency_scan_unexpected_variant,
802-
dep.first.ModuleName);
803-
diags.diagnose(
804-
SourceLoc(),
805-
diag::dependency_scan_unexpected_variant_context_hash_note,
806-
priorContextHash, newContextHash);
807-
diags.diagnose(
808-
SourceLoc(),
809-
diag::dependency_scan_unexpected_variant_module_map_note,
810-
priorClangModuleDetails->moduleMapFile,
811-
newClangModuleDetails->moduleMapFile);
812-
813-
auto diagnoseExtraCommandLineFlags =
814-
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
815-
const ClangModuleDependencyStorage *baseModuleDetails,
816-
bool isNewlyDiscovered) -> void {
817-
std::unordered_set<std::string> baseCommandLineSet(
818-
baseModuleDetails->buildCommandLine.begin(),
819-
baseModuleDetails->buildCommandLine.end());
820-
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
821-
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
822-
diags.diagnose(
823-
SourceLoc(),
824-
diag::dependency_scan_unexpected_variant_extra_arg_note,
825-
isNewlyDiscovered, checkArg);
826-
};
827-
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
828-
newClangModuleDetails, true);
829-
diagnoseExtraCommandLineFlags(newClangModuleDetails,
830-
priorClangModuleDetails, false);
831-
}
787+
auto priorClangModuleDetails =
788+
findKnownDependency(dep.first).getAsClangModule();
789+
DEBUG_ASSERT(priorClangModuleDetails && newClangModuleDetails);
790+
auto priorContextHash = priorClangModuleDetails->contextHash;
791+
auto newContextHash = newClangModuleDetails->contextHash;
792+
if (priorContextHash != newContextHash) {
793+
// This situation means that within the same scanning action, Clang
794+
// Dependency Scanner has produced two different variants of the same
795+
// module. This is not supposed to happen, but we are currently
796+
// hunting down the rare cases where it does, seemingly due to
797+
// differences in Clang Scanner direct by-name queries and transitive
798+
// header lookup queries.
799+
//
800+
// Emit a failure diagnostic here that is hopefully more actionable
801+
// for the time being.
802+
diags.diagnose(SourceLoc(), diag::dependency_scan_unexpected_variant,
803+
dep.first.ModuleName);
804+
diags.diagnose(
805+
SourceLoc(),
806+
diag::dependency_scan_unexpected_variant_context_hash_note,
807+
priorContextHash, newContextHash);
808+
diags.diagnose(
809+
SourceLoc(),
810+
diag::dependency_scan_unexpected_variant_module_map_note,
811+
priorClangModuleDetails->moduleMapFile,
812+
newClangModuleDetails->moduleMapFile);
813+
814+
auto diagnoseExtraCommandLineFlags =
815+
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
816+
const ClangModuleDependencyStorage *baseModuleDetails,
817+
bool isNewlyDiscovered) -> void {
818+
std::unordered_set<std::string> baseCommandLineSet(
819+
baseModuleDetails->buildCommandLine.begin(),
820+
baseModuleDetails->buildCommandLine.end());
821+
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
822+
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
823+
diags.diagnose(
824+
SourceLoc(),
825+
diag::dependency_scan_unexpected_variant_extra_arg_note,
826+
isNewlyDiscovered, checkArg);
827+
};
828+
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
829+
newClangModuleDetails, true);
830+
diagnoseExtraCommandLineFlags(newClangModuleDetails,
831+
priorClangModuleDetails, false);
832832
}
833-
} else
833+
} else {
834834
recordDependency(dep.first.ModuleName, dep.second);
835-
836-
if (dep.first.Kind == ModuleDependencyKind::Clang) {
837-
auto clangModuleDetails = dep.second.getAsClangModule();
838835
addSeenClangModule(clang::tooling::dependencies::ModuleID{
839-
dep.first.ModuleName, clangModuleDetails->contextHash});
836+
dep.first.ModuleName, newClangModuleDetails->contextHash});
840837
}
841838
}
842839
}
@@ -918,6 +915,24 @@ ModuleDependenciesCache::setCrossImportOverlayDependencies(ModuleDependencyID mo
918915
updateDependency(moduleID, updatedDependencyInfo);
919916
}
920917

918+
void
919+
ModuleDependenciesCache::addVisibleClangModules(ModuleDependencyID moduleID,
920+
const std::vector<std::string> &moduleNames) {
921+
if (moduleNames.empty())
922+
return;
923+
auto dependencyInfo = findKnownDependency(moduleID);
924+
auto updatedDependencyInfo = dependencyInfo;
925+
updatedDependencyInfo.addVisibleClangModules(moduleNames);
926+
updateDependency(moduleID, updatedDependencyInfo);
927+
}
928+
929+
llvm::StringSet<> &ModuleDependenciesCache::getVisibleClangModules(ModuleDependencyID moduleID) const {
930+
ASSERT(moduleID.Kind == ModuleDependencyKind::SwiftSource ||
931+
moduleID.Kind == ModuleDependencyKind::SwiftInterface ||
932+
moduleID.Kind == ModuleDependencyKind::SwiftBinary);
933+
return findKnownDependency(moduleID).getVisibleClangModules();
934+
}
935+
921936
ModuleDependencyIDSetVector
922937
ModuleDependenciesCache::getAllDependencies(const ModuleDependencyID &moduleID) const {
923938
const auto &moduleInfo = findKnownDependency(moduleID);

0 commit comments

Comments
 (0)