diff --git a/include/swift/AST/DiagnosticsCommon.def b/include/swift/AST/DiagnosticsCommon.def index 63bdee0da4d..400fbd6af8f 100644 --- a/include/swift/AST/DiagnosticsCommon.def +++ b/include/swift/AST/DiagnosticsCommon.def @@ -199,7 +199,10 @@ WARNING(cross_imported_by_both_modules, none, //------------------------------------------------------------------------------ ERROR(scanner_find_cycle, none, - "dependency scanner detected dependency cycle: '%0'", (StringRef)) + "module dependency cycle: '%0'", (StringRef)) + +NOTE(scanner_find_cycle_swift_overlay_path, none, + "Swift Overlay dependency of '%0' on '%1' via Clang module dependency: '%2'", (StringRef, StringRef, StringRef)) ERROR(scanner_arguments_invalid, none, "dependencies scanner cannot be configured with arguments: '%0'", (StringRef)) diff --git a/include/swift/AST/ModuleDependencies.h b/include/swift/AST/ModuleDependencies.h index 47fc2bf7b2c..6a6d0dd9257 100644 --- a/include/swift/AST/ModuleDependencies.h +++ b/include/swift/AST/ModuleDependencies.h @@ -1036,6 +1036,14 @@ public: std::vector getAllDependencies(const ModuleDependencyID &moduleID) const; + /// Query only direct import dependencies + llvm::ArrayRef + getOnlyDirectDependencies(const ModuleDependencyID &moduleID) const; + + /// Query only Swift overlay dependencies + llvm::ArrayRef + getOnlyOverlayDependencies(const ModuleDependencyID &moduleID) const; + /// Look for module dependencies for a module with the given ID /// /// \returns the cached result, or \c None if there is no cached entry. diff --git a/lib/AST/ModuleDependencies.cpp b/lib/AST/ModuleDependencies.cpp index 338ede4ec31..a669ebbdfc9 100644 --- a/lib/AST/ModuleDependencies.cpp +++ b/lib/AST/ModuleDependencies.cpp @@ -795,12 +795,12 @@ void ModuleDependenciesCache::setSwiftOverlayDependencies(ModuleDependencyID mod std::vector ModuleDependenciesCache::getAllDependencies(const ModuleDependencyID &moduleID) const { - const auto &optionalModuleInfo = findDependency(moduleID); - assert(optionalModuleInfo.has_value()); + const auto &moduleInfo = findDependency(moduleID); + assert(moduleInfo.has_value()); auto directDependenciesRef = - optionalModuleInfo.value()->getDirectModuleDependencies(); + moduleInfo.value()->getDirectModuleDependencies(); auto overlayDependenciesRef = - optionalModuleInfo.value()->getSwiftOverlayDependencies(); + moduleInfo.value()->getSwiftOverlayDependencies(); std::vector result; result.insert(std::end(result), directDependenciesRef.begin(), directDependenciesRef.end()); @@ -808,3 +808,17 @@ ModuleDependenciesCache::getAllDependencies(const ModuleDependencyID &moduleID) overlayDependenciesRef.end()); return result; } + +ArrayRef +ModuleDependenciesCache::getOnlyOverlayDependencies(const ModuleDependencyID &moduleID) const { + const auto &moduleInfo = findDependency(moduleID); + assert(moduleInfo.has_value()); + return moduleInfo.value()->getSwiftOverlayDependencies(); +} + +ArrayRef +ModuleDependenciesCache::getOnlyDirectDependencies(const ModuleDependencyID &moduleID) const { + const auto &moduleInfo = findDependency(moduleID); + assert(moduleInfo.has_value()); + return moduleInfo.value()->getDirectModuleDependencies(); +} diff --git a/lib/DependencyScan/ScanDependencies.cpp b/lib/DependencyScan/ScanDependencies.cpp index c14a20b1856..4365f3d3ac8 100644 --- a/lib/DependencyScan/ScanDependencies.cpp +++ b/lib/DependencyScan/ScanDependencies.cpp @@ -63,6 +63,7 @@ #include #include #include +#include using namespace swift; using namespace swift::dependencies; @@ -1244,47 +1245,129 @@ computeTransitiveClosureOfExplicitDependencies( return result; } +static std::vector +findClangDepPath(const ModuleDependencyID &from, const ModuleDependencyID &to, + ModuleDependenciesCache &cache) { + std::unordered_set visited; + std::vector result; + std::stack> stack; + + // Must be explicitly-typed to allow recursion + std::function visit; + + visit = [&visit, &cache, &visited, &result, &stack, + to](const ModuleDependencyID &moduleID) { + if (!visited.insert(moduleID).second) + return; + + if (moduleID == to) { + // Copy stack contents to the result + auto end = &stack.top() + 1; + auto begin = end - stack.size(); + result.assign(begin, end); + return; + } + + // Otherwise, visit each child node. + for (const auto &succID : cache.getAllDependencies(moduleID)) { + stack.push(succID); + visit(succID); + stack.pop(); + } + }; + + stack.push(from); + visit(from); + return result; +} + static bool diagnoseCycle(CompilerInstance &instance, ModuleDependenciesCache &cache, ModuleDependencyID mainId) { ModuleDependencyIDSetVector openSet; ModuleDependencyIDSetVector closeSet; - // Start from the main module. + + auto kindIsSwiftDependency = [&](const ModuleDependencyID &ID) { + return ID.Kind == swift::ModuleDependencyKind::SwiftInterface || + ID.Kind == swift::ModuleDependencyKind::SwiftBinary; + }; + + auto emitModulePath = [&](const std::vector path, + llvm::SmallString<64> &buffer) { + llvm::interleave( + path, + [&buffer](const ModuleDependencyID &id) { + buffer.append(id.ModuleName); + switch (id.Kind) { + case swift::ModuleDependencyKind::SwiftInterface: + buffer.append(".swiftinterface"); + break; + case swift::ModuleDependencyKind::SwiftBinary: + buffer.append(".swiftmodule"); + break; + case swift::ModuleDependencyKind::Clang: + buffer.append(".pcm"); + break; + default: + llvm::report_fatal_error( + Twine("Invalid Module Dependency Kind in cycle: ") + + id.ModuleName); + break; + } + }, + [&buffer] { buffer.append(" -> "); }); + }; + + auto emitCycleDiagnostic = [&](const ModuleDependencyID &dep) { + auto startIt = std::find(openSet.begin(), openSet.end(), dep); + assert(startIt != openSet.end()); + std::vector cycleNodes(startIt, openSet.end()); + cycleNodes.push_back(*startIt); + llvm::SmallString<64> errorBuffer; + emitModulePath(cycleNodes, errorBuffer); + instance.getASTContext().Diags.diagnose( + SourceLoc(), diag::scanner_find_cycle, errorBuffer.str()); + + // TODO: for (std::tuple v : cycleNodes | std::views::adjacent<2>) + for (auto it = cycleNodes.begin(), end = cycleNodes.end(); it != end; + it++) { + if (it + 1 == cycleNodes.end()) + continue; + + const auto &thisID = *it; + const auto &nextID = *(it + 1); + if (kindIsSwiftDependency(thisID) && kindIsSwiftDependency(nextID) && + llvm::any_of( + cache.getOnlyOverlayDependencies(thisID), + [&](const ModuleDependencyID id) { return id == nextID; })) { + llvm::SmallString<64> noteBuffer; + auto clangDepPath = findClangDepPath( + thisID, + ModuleDependencyID{nextID.ModuleName, ModuleDependencyKind::Clang}, + cache); + emitModulePath(clangDepPath, noteBuffer); + instance.getASTContext().Diags.diagnose( + SourceLoc(), diag::scanner_find_cycle_swift_overlay_path, + thisID.ModuleName, nextID.ModuleName, noteBuffer.str()); + } + } + }; + + // Start from the main module and check direct and overlay dependencies openSet.insert(mainId); while (!openSet.empty()) { - auto &lastOpen = openSet.back(); + auto lastOpen = openSet.back(); auto beforeSize = openSet.size(); assert(cache.findDependency(lastOpen).has_value() && "Missing dependency info during cycle diagnosis."); - for (const auto &dep : cache.getAllDependencies(lastOpen)) { if (closeSet.count(dep)) continue; if (openSet.insert(dep)) { break; } else { - // Find a cycle, diagnose. - auto startIt = std::find(openSet.begin(), openSet.end(), dep); - assert(startIt != openSet.end()); - llvm::SmallString<64> buffer; - for (auto it = startIt; it != openSet.end(); ++it) { - buffer.append(it->ModuleName); - buffer.append((it->Kind == ModuleDependencyKind::SwiftInterface || - it->Kind == ModuleDependencyKind::SwiftSource || - it->Kind == ModuleDependencyKind::SwiftBinary) - ? ".swiftmodule" - : ".pcm"); - buffer.append(" -> "); - } - buffer.append(startIt->ModuleName); - buffer.append( - (startIt->Kind == ModuleDependencyKind::SwiftInterface || - startIt->Kind == ModuleDependencyKind::SwiftSource || - startIt->Kind == ModuleDependencyKind::SwiftBinary) - ? ".swiftmodule" - : ".pcm"); - instance.getASTContext().Diags.diagnose( - SourceLoc(), diag::scanner_find_cycle, buffer.str()); + emitCycleDiagnostic(dep); return true; } } @@ -1297,6 +1380,7 @@ static bool diagnoseCycle(CompilerInstance &instance, } } assert(openSet.empty()); + closeSet.clear(); return false; } diff --git a/test/ScanDependencies/Inputs/CHeaders/CycleClangMiddle.h b/test/ScanDependencies/Inputs/CHeaders/CycleClangMiddle.h new file mode 100644 index 00000000000..79cc8cd59d2 --- /dev/null +++ b/test/ScanDependencies/Inputs/CHeaders/CycleClangMiddle.h @@ -0,0 +1 @@ +#include "CycleOverlay.h" diff --git a/test/ScanDependencies/Inputs/CHeaders/CycleOverlay.h b/test/ScanDependencies/Inputs/CHeaders/CycleOverlay.h new file mode 100644 index 00000000000..3c0f1bbdf30 --- /dev/null +++ b/test/ScanDependencies/Inputs/CHeaders/CycleOverlay.h @@ -0,0 +1 @@ +void funcOverlay(void); diff --git a/test/ScanDependencies/Inputs/CHeaders/module.modulemap b/test/ScanDependencies/Inputs/CHeaders/module.modulemap index 53f00f6befe..c02240190d4 100644 --- a/test/ScanDependencies/Inputs/CHeaders/module.modulemap +++ b/test/ScanDependencies/Inputs/CHeaders/module.modulemap @@ -56,4 +56,12 @@ module Y { module ClangModuleWithOverlayedDep { header "ClangModuleWithOverlayedDep.h" export * -} \ No newline at end of file +} +module CycleClangMiddle { + header "CycleClangMiddle.h" + export * +} +module CycleOverlay { + header "CycleOverlay.h" + export * +} diff --git a/test/ScanDependencies/Inputs/Swift/CycleOverlay.swiftinterface b/test/ScanDependencies/Inputs/Swift/CycleOverlay.swiftinterface new file mode 100644 index 00000000000..83896ac28a3 --- /dev/null +++ b/test/ScanDependencies/Inputs/Swift/CycleOverlay.swiftinterface @@ -0,0 +1,4 @@ +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name CycleOverlay +import Swift +import CycleSwiftMiddle diff --git a/test/ScanDependencies/Inputs/Swift/CycleSwiftMiddle.swiftinterface b/test/ScanDependencies/Inputs/Swift/CycleSwiftMiddle.swiftinterface new file mode 100644 index 00000000000..64a10b851b1 --- /dev/null +++ b/test/ScanDependencies/Inputs/Swift/CycleSwiftMiddle.swiftinterface @@ -0,0 +1,4 @@ +// swift-interface-format-version: 1.0 +// swift-module-flags: -module-name CycleSwiftMiddle +import Swift +import CycleClangMiddle diff --git a/test/ScanDependencies/diagnose_dependency_cycle.swift b/test/ScanDependencies/diagnose_dependency_cycle.swift index 609fd331646..a69dd137752 100644 --- a/test/ScanDependencies/diagnose_dependency_cycle.swift +++ b/test/ScanDependencies/diagnose_dependency_cycle.swift @@ -5,6 +5,6 @@ // RUN: %FileCheck %s < %t/out.txt -// CHECK: dependency scanner detected dependency cycle: 'CycleOne.swiftmodule -> CycleTwo.swiftmodule -> CycleThree.swiftmodule -> CycleOne.swiftmodule' +// CHECK: module dependency cycle: 'CycleOne.swiftinterface -> CycleTwo.swiftinterface -> CycleThree.swiftinterface -> CycleOne.swiftinterface' import CycleOne diff --git a/test/ScanDependencies/diagnose_dependency_cycle_overlay.swift b/test/ScanDependencies/diagnose_dependency_cycle_overlay.swift new file mode 100644 index 00000000000..7027df32b33 --- /dev/null +++ b/test/ScanDependencies/diagnose_dependency_cycle_overlay.swift @@ -0,0 +1,10 @@ +// RUN: %empty-directory(%t) +// RUN: mkdir -p %t/clang-module-cache + +// RUN: not %target-swift-frontend -scan-dependencies -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift &> %t/out.txt +// RUN: %FileCheck %s < %t/out.txt + +// CHECK: error: module dependency cycle: 'CycleOverlay.swiftinterface -> CycleSwiftMiddle.swiftinterface -> CycleOverlay.swiftinterface' +// CHECK: note: Swift Overlay dependency of 'CycleSwiftMiddle' on 'CycleOverlay' via Clang module dependency: 'CycleSwiftMiddle.swiftinterface -> CycleClangMiddle.pcm -> CycleOverlay.pcm' + +import CycleOverlay