Skip to content

[6.2 🍒][Dependency Scanning] Emit a detailed warning diagnostic on Clang module variant discovery #81337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 8, 2025
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
11 changes: 11 additions & 0 deletions include/swift/AST/DiagnosticsFrontend.def
Original file line number Diff line number Diff line change
Expand Up @@ -618,5 +618,16 @@ ERROR(ast_format_requires_dump_ast,none,
ERROR(unknown_dump_ast_format,none,
"unknown format '%0' requested with '-dump-ast-format'", (StringRef))

WARNING(dependency_scan_unexpected_variant, none,
"unexpected module variant during dependency scanning on module '%0', "
"compilation of this target is likely to fail or succeed in a way that is not deterministic", (StringRef))
NOTE(dependency_scan_unexpected_variant_context_hash_note, none,
"first module context hash: '%0', second module context hash: '%1'", (StringRef, StringRef))
NOTE(dependency_scan_unexpected_variant_module_map_note, none,
"first module module map: '%0', second module module map: '%1'", (StringRef, StringRef))
NOTE(dependency_scan_unexpected_variant_extra_arg_note, none,
"%select{first|second}0 module command-line has extra argument: '%1'", (bool, StringRef))


#define UNDEFINE_DIAGNOSTIC_MACROS
#include "DefineDiagnosticMacros.h"
4 changes: 3 additions & 1 deletion include/swift/AST/ModuleDependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class Identifier;
class CompilerInstance;
class IRGenOptions;
class CompilerInvocation;
class DiagnosticEngine;

/// Which kind of module dependencies we are looking for.
enum class ModuleDependencyKind : int8_t {
Expand Down Expand Up @@ -1254,7 +1255,8 @@ class ModuleDependenciesCache {
ModuleDependencyInfo dependencies);

/// Record dependencies for the given module collection.
void recordDependencies(ModuleDependencyVector moduleDependencies);
void recordDependencies(ModuleDependencyVector moduleDependencies,
DiagnosticEngine &diags);

/// Update stored dependencies for the given module.
void updateDependency(ModuleDependencyID moduleID,
Expand Down
7 changes: 0 additions & 7 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2199,13 +2199,6 @@ Identifier ASTContext::getRealModuleName(Identifier key, ModuleAliasLookupOption
return value.first;
}

namespace {
static StringRef pathStringFromSearchPath(
const SearchPathOptions::SearchPath &next) {
return next.Path;
}
}

std::vector<std::string> ASTContext::getDarwinImplicitFrameworkSearchPaths()
const {
assert(LangOpts.Target.isOSDarwin());
Expand Down
56 changes: 53 additions & 3 deletions lib/AST/ModuleDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -886,11 +886,61 @@ void ModuleDependenciesCache::recordDependency(
}

void ModuleDependenciesCache::recordDependencies(
ModuleDependencyVector dependencies) {
ModuleDependencyVector dependencies, DiagnosticEngine &diags) {
for (const auto &dep : dependencies) {
if (!hasDependency(dep.first))
if (hasDependency(dep.first)) {
if (dep.first.Kind == ModuleDependencyKind::Clang) {
auto priorClangModuleDetails =
findKnownDependency(dep.first).getAsClangModule();
auto newClangModuleDetails = dep.second.getAsClangModule();
auto priorContextHash = priorClangModuleDetails->contextHash;
auto newContextHash = newClangModuleDetails->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.first.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,
newClangModuleDetails->moduleMapFile);

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.first.ModuleName, dep.second);
if (dep.second.getKind() == ModuleDependencyKind::Clang) {

if (dep.first.Kind == ModuleDependencyKind::Clang) {
auto clangModuleDetails = dep.second.getAsClangModule();
addSeenClangModule(clang::tooling::dependencies::ModuleID{
dep.first.ModuleName, clangModuleDetails->contextHash});
Expand Down
2 changes: 1 addition & 1 deletion lib/ClangImporter/ClangModuleDependencyScanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ bool ClangImporter::getHeaderDependencies(
[&cache](StringRef path) {
return cache.getScanService().remapPath(path);
});
cache.recordDependencies(bridgedDeps);
cache.recordDependencies(bridgedDeps, Impl.SwiftContext.Diags);

llvm::copy(dependencies->FileDeps, std::back_inserter(headerFileInputs));
auto bridgedDependencyIDs =
Expand Down
14 changes: 7 additions & 7 deletions lib/DependencyScan/ModuleDependencyScanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ ModuleDependencyScanner::getNamedClangModuleDependencyInfo(
for (const auto &dep : moduleDependencies)
discoveredClangModules.insert(dep.first);

cache.recordDependencies(moduleDependencies);
cache.recordDependencies(moduleDependencies, Diagnostics);
return cache.findDependency(moduleID);
}

Expand Down Expand Up @@ -566,7 +566,7 @@ ModuleDependencyScanner::getNamedSwiftModuleDependencyInfo(
if (moduleDependencies.empty())
return std::nullopt;

cache.recordDependencies(moduleDependencies);
cache.recordDependencies(moduleDependencies, Diagnostics);
return cache.findDependency(moduleName);
}

Expand Down Expand Up @@ -927,7 +927,7 @@ ModuleDependencyScanner::resolveAllClangModuleDependencies(
std::lock_guard<std::mutex> guard(cacheAccessLock);
moduleLookupResult.insert_or_assign(moduleName, moduleDependencies);
if (!moduleDependencies.empty())
cache.recordDependencies(moduleDependencies);
cache.recordDependencies(moduleDependencies, Diagnostics);
}
};

Expand Down Expand Up @@ -1138,7 +1138,7 @@ void ModuleDependencyScanner::resolveSwiftImportsForModule(
ScanningThreadPool.wait();

auto recordResolvedModuleImport =
[&cache, &moduleLookupResult, &importedSwiftDependencies,
[this, &cache, &moduleLookupResult, &importedSwiftDependencies,
moduleID](const ScannerImportStatementInfo &moduleImport) {
if (moduleID.ModuleName == moduleImport.importIdentifier)
return;
Expand All @@ -1152,7 +1152,7 @@ void ModuleDependencyScanner::resolveSwiftImportsForModule(
} else {
// Cache discovered module dependencies.
if (!lookupResult.value().empty()) {
cache.recordDependencies(lookupResult.value());
cache.recordDependencies(lookupResult.value(), Diagnostics);
importedSwiftDependencies.insert({moduleImport.importIdentifier,
lookupResult.value()[0].first.Kind});
}
Expand Down Expand Up @@ -1306,7 +1306,7 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependenciesForModule(
ScanningThreadPool.wait();

// Aggregate both previously-cached and freshly-scanned module results
auto recordResult = [&cache, &swiftOverlayLookupResult,
auto recordResult = [this, &cache, &swiftOverlayLookupResult,
&swiftOverlayDependencies,
moduleID](const std::string &moduleName) {
auto lookupResult = swiftOverlayLookupResult[moduleName];
Expand All @@ -1318,7 +1318,7 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependenciesForModule(
{moduleName, cachedInfo.value()->getKind()});
} else {
// Cache discovered module dependencies.
cache.recordDependencies(lookupResult.value());
cache.recordDependencies(lookupResult.value(), Diagnostics);
if (!lookupResult.value().empty())
swiftOverlayDependencies.insert({moduleName, lookupResult.value()[0].first.Kind});
}
Expand Down