Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions include/swift/AST/ModuleDependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,12 @@ class ModuleDependenciesCache {
DiagnosticEngine &diags,
BridgeClangDependencyCallback bridgeClangModule);

/// Record Clang module dependency.
void recordClangDependency(
const clang::tooling::dependencies::ModuleDeps &dependency,
DiagnosticEngine &diags,
BridgeClangDependencyCallback bridgeClangModule);

/// Update stored dependencies for the given module.
void updateDependency(ModuleDependencyID moduleID,
ModuleDependencyInfo dependencyInfo);
Expand Down
55 changes: 55 additions & 0 deletions include/swift/DependencyScan/ModuleDependencyScanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ using LookupModuleOutputCallback = llvm::function_ref<std::string(
clang::tooling::dependencies::ModuleOutputKind)>;
using RemapPathCallback = llvm::function_ref<std::string(StringRef)>;

/// A map from a module id to a collection of import statement infos.
using ImportStatementInfoMap =
std::unordered_map<ModuleDependencyID,
std::vector<ScannerImportStatementInfo>>;

/// A dependency scanning worker which performs filesystem lookup
/// of a named module dependency.
class ModuleDependencyScanningWorker {
Expand Down Expand Up @@ -329,6 +334,56 @@ class ModuleDependencyScanner {
/// for a given module name.
Identifier getModuleImportIdentifier(StringRef moduleName);

private:
struct BatchClangModuleLookupResult {
llvm::StringMap<clang::tooling::dependencies::ModuleDeps>
discoveredDependencyInfos;
llvm::StringMap<std::vector<std::string>> visibleModules;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringMap is not thread-safe. A normal insert might be but it can expand when full. We need a lock for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have a lock for this, it is only ever modified in performParallelClangModuleLookup in a critical section guarded with a lock.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add a helper function to the class to ensure the correct lock usage. It might even be cleaner to move the lock here to avoid introduce regression for future refactoring.

};

/// For the provided collection of unresolved imports
/// belonging to identified Swift dependnecies, execute a parallel
/// query to the Clang dependency scanner for each import's module identifier.
void performParallelClangModuleLookup(
const ImportStatementInfoMap &unresolvedImportsMap,
const ImportStatementInfoMap &unresolvedOptionalImportsMap,
ModuleDependenciesCache &cache,
BatchClangModuleLookupResult &result);

/// Given a result of a batch Clang module dependency lookup,
/// record its results in the cache:
/// 1. Record all discovered Clang module dependency infos
/// in the \c cache.
/// 1. Update the set of visible Clang modules from each Swift module
/// in the \c cache.
/// 2. Update the total collection of all disovered clang modules
/// in \c allDiscoveredClangModules.
/// 3. Record all import identifiers which the scan failed to resolve
/// in \c failedToResolveImports.
/// 4. Update the set of resolved Clang dependencies for each Swift
/// module dependency in \c resolvedClangDependenciesMap.
void cacheComputedClangModuleLookupResults(
const BatchClangModuleLookupResult &lookupResult,
const ImportStatementInfoMap &unresolvedImportsMap,
const ImportStatementInfoMap &unresolvedOptionalImportsMap,
ArrayRef<ModuleDependencyID> swiftModuleDependents,
ModuleDependenciesCache &cache,
ModuleDependencyIDSetVector &allDiscoveredClangModules,
std::vector<std::pair<ModuleDependencyID, ScannerImportStatementInfo>>
&failedToResolveImports,
std::unordered_map<ModuleDependencyID, ModuleDependencyIDSetVector>
&resolvedClangDependenciesMap);

/// Re-query some failed-to-resolve Clang imports from cache
/// in chance they were brought in as transitive dependencies.
void reQueryMissedModulesFromCache(
ModuleDependenciesCache &cache,
const std::vector<
std::pair<ModuleDependencyID, ScannerImportStatementInfo>>
&failedToResolveImports,
std::unordered_map<ModuleDependencyID, ModuleDependencyIDSetVector>
&resolvedClangDependenciesMap);

/// Assuming the \c `moduleImport` failed to resolve,
/// iterate over all binary Swift module dependencies with serialized
/// search paths and attempt to diagnose if the failed-to-resolve module
Expand Down
117 changes: 62 additions & 55 deletions lib/AST/ModuleDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -788,61 +788,68 @@ void ModuleDependenciesCache::recordClangDependencies(
const clang::tooling::dependencies::ModuleDepsGraph &dependencies,
DiagnosticEngine &diags,
BridgeClangDependencyCallback bridgeClangModule) {
for (const auto &dep : dependencies) {
auto depID =
ModuleDependencyID{dep.ID.ModuleName, ModuleDependencyKind::Clang};
if (hasDependency(depID)) {
auto priorClangModuleDetails =
findKnownDependency(depID).getAsClangModule();
DEBUG_ASSERT(priorClangModuleDetails);
auto priorContextHash = priorClangModuleDetails->contextHash;
auto newContextHash = dep.ID.ContextHash;
if (priorContextHash != newContextHash) {
// This situation means that within the same scanning action, Clang
// Dependency Scanner has produced two different variants of the same
// module. This is not supposed to happen, but we are currently
// hunting down the rare cases where it does, seemingly due to
// differences in Clang Scanner direct by-name queries and transitive
// header lookup queries.
//
// Emit a failure diagnostic here that is hopefully more actionable
// for the time being.
diags.diagnose(SourceLoc(),
diag::dependency_scan_unexpected_variant,
dep.ID.ModuleName);
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_context_hash_note,
priorContextHash, newContextHash);
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_module_map_note,
priorClangModuleDetails->moduleMapFile, dep.ClangModuleMapFile);

auto newClangModuleDetails = bridgeClangModule(dep).getAsClangModule();
auto diagnoseExtraCommandLineFlags =
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
const ClangModuleDependencyStorage *baseModuleDetails,
bool isNewlyDiscovered) -> void {
std::unordered_set<std::string> baseCommandLineSet(
baseModuleDetails->buildCommandLine.begin(),
baseModuleDetails->buildCommandLine.end());
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_extra_arg_note,
isNewlyDiscovered, checkArg);
};
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
newClangModuleDetails, true);
diagnoseExtraCommandLineFlags(newClangModuleDetails,
priorClangModuleDetails, false);
}
} else {
recordDependency(dep.ID.ModuleName, bridgeClangModule(dep));
addSeenClangModule(dep.ID);
}
for (const auto &dep : dependencies)
recordClangDependency(dep, diags, bridgeClangModule);
}

void ModuleDependenciesCache::recordClangDependency(
const clang::tooling::dependencies::ModuleDeps &dependency,
DiagnosticEngine &diags,
BridgeClangDependencyCallback bridgeClangModule) {
auto depID =
ModuleDependencyID{dependency.ID.ModuleName, ModuleDependencyKind::Clang};
if (!hasDependency(depID)) {
recordDependency(dependency.ID.ModuleName, bridgeClangModule(dependency));
addSeenClangModule(dependency.ID);
return;
}

auto priorClangModuleDetails =
findKnownDependency(depID).getAsClangModule();
DEBUG_ASSERT(priorClangModuleDetails);
auto priorContextHash = priorClangModuleDetails->contextHash;
auto newContextHash = dependency.ID.ContextHash;
if (priorContextHash != newContextHash) {
// This situation means that within the same scanning action, Clang
// Dependency Scanner has produced two different variants of the same
// module. This is not supposed to happen, but we are currently
// hunting down the rare cases where it does, seemingly due to
// differences in Clang Scanner direct by-name queries and transitive
// header lookup queries.
//
// Emit a failure diagnostic here that is hopefully more actionable
// for the time being.
diags.diagnose(SourceLoc(),
diag::dependency_scan_unexpected_variant,
dependency.ID.ModuleName);
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_context_hash_note,
priorContextHash, newContextHash);
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_module_map_note,
priorClangModuleDetails->moduleMapFile, dependency.ClangModuleMapFile);

auto newClangModuleDetails = bridgeClangModule(dependency).getAsClangModule();
auto diagnoseExtraCommandLineFlags =
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
const ClangModuleDependencyStorage *baseModuleDetails,
bool isNewlyDiscovered) -> void {
std::unordered_set<std::string> baseCommandLineSet(
baseModuleDetails->buildCommandLine.begin(),
baseModuleDetails->buildCommandLine.end());
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
diags.diagnose(
SourceLoc(),
diag::dependency_scan_unexpected_variant_extra_arg_note,
isNewlyDiscovered, checkArg);
};
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
newClangModuleDetails, true);
diagnoseExtraCommandLineFlags(newClangModuleDetails,
priorClangModuleDetails, false);
}
}

Expand Down
Loading