Skip to content

Commit f45db99

Browse files
artemcmartemcm
committed
[Dependency Scanning] Improve the cycle-detection diagnostic for Swift overlay dependencies
Add tracing of the underlying Clang module for an overlay dependency if said overlay dependency forms a cycle
1 parent 71640be commit f45db99

11 files changed

+169
-32
lines changed

‎include/swift/AST/DiagnosticsCommon.def

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,10 @@ WARNING(cross_imported_by_both_modules, none,
199199
//------------------------------------------------------------------------------
200200

201201
ERROR(scanner_find_cycle, none,
202-
"dependency scanner detected dependency cycle: '%0'", (StringRef))
202+
"module dependency cycle: '%0'", (StringRef))
203+
204+
NOTE(scanner_find_cycle_swift_overlay_path, none,
205+
"Swift Overlay dependency of '%0' on '%1' via Clang module dependency: '%2'", (StringRef, StringRef, StringRef))
203206

204207
ERROR(scanner_arguments_invalid, none,
205208
"dependencies scanner cannot be configured with arguments: '%0'", (StringRef))

‎include/swift/AST/ModuleDependencies.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,14 @@ class ModuleDependenciesCache {
10361036
std::vector<ModuleDependencyID>
10371037
getAllDependencies(const ModuleDependencyID &moduleID) const;
10381038

1039+
/// Query only direct import dependencies
1040+
llvm::ArrayRef<ModuleDependencyID>
1041+
getOnlyDirectDependencies(const ModuleDependencyID &moduleID) const;
1042+
1043+
/// Query only Swift overlay dependencies
1044+
llvm::ArrayRef<ModuleDependencyID>
1045+
getOnlyOverlayDependencies(const ModuleDependencyID &moduleID) const;
1046+
10391047
/// Look for module dependencies for a module with the given ID
10401048
///
10411049
/// \returns the cached result, or \c None if there is no cached entry.

‎lib/AST/ModuleDependencies.cpp

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -795,16 +795,30 @@ void ModuleDependenciesCache::setSwiftOverlayDependencies(ModuleDependencyID mod
795795

796796
std::vector<ModuleDependencyID>
797797
ModuleDependenciesCache::getAllDependencies(const ModuleDependencyID &moduleID) const {
798-
const auto &optionalModuleInfo = findDependency(moduleID);
799-
assert(optionalModuleInfo.has_value());
798+
const auto &moduleInfo = findDependency(moduleID);
799+
assert(moduleInfo.has_value());
800800
auto directDependenciesRef =
801-
optionalModuleInfo.value()->getDirectModuleDependencies();
801+
moduleInfo.value()->getDirectModuleDependencies();
802802
auto overlayDependenciesRef =
803-
optionalModuleInfo.value()->getSwiftOverlayDependencies();
803+
moduleInfo.value()->getSwiftOverlayDependencies();
804804
std::vector<ModuleDependencyID> result;
805805
result.insert(std::end(result), directDependenciesRef.begin(),
806806
directDependenciesRef.end());
807807
result.insert(std::end(result), overlayDependenciesRef.begin(),
808808
overlayDependenciesRef.end());
809809
return result;
810810
}
811+
812+
ArrayRef<ModuleDependencyID>
813+
ModuleDependenciesCache::getOnlyOverlayDependencies(const ModuleDependencyID &moduleID) const {
814+
const auto &moduleInfo = findDependency(moduleID);
815+
assert(moduleInfo.has_value());
816+
return moduleInfo.value()->getSwiftOverlayDependencies();
817+
}
818+
819+
ArrayRef<ModuleDependencyID>
820+
ModuleDependenciesCache::getOnlyDirectDependencies(const ModuleDependencyID &moduleID) const {
821+
const auto &moduleInfo = findDependency(moduleID);
822+
assert(moduleInfo.has_value());
823+
return moduleInfo.value()->getDirectModuleDependencies();
824+
}

‎lib/DependencyScan/ScanDependencies.cpp

Lines changed: 109 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
#include <set>
6464
#include <sstream>
6565
#include <string>
66+
#include <algorithm>
6667

6768
using namespace swift;
6869
using namespace swift::dependencies;
@@ -1244,47 +1245,129 @@ computeTransitiveClosureOfExplicitDependencies(
12441245
return result;
12451246
}
12461247

1248+
static std::vector<ModuleDependencyID>
1249+
findClangDepPath(const ModuleDependencyID &from, const ModuleDependencyID &to,
1250+
ModuleDependenciesCache &cache) {
1251+
std::unordered_set<ModuleDependencyID> visited;
1252+
std::vector<ModuleDependencyID> result;
1253+
std::stack<ModuleDependencyID, std::vector<ModuleDependencyID>> stack;
1254+
1255+
// Must be explicitly-typed to allow recursion
1256+
std::function<void(const ModuleDependencyID &)> visit;
1257+
1258+
visit = [&visit, &cache, &visited, &result, &stack,
1259+
to](const ModuleDependencyID &moduleID) {
1260+
if (!visited.insert(moduleID).second)
1261+
return;
1262+
1263+
if (moduleID == to) {
1264+
// Copy stack contents to the result
1265+
auto end = &stack.top() + 1;
1266+
auto begin = end - stack.size();
1267+
result.assign(begin, end);
1268+
return;
1269+
}
1270+
1271+
// Otherwise, visit each child node.
1272+
for (const auto &succID : cache.getAllDependencies(moduleID)) {
1273+
stack.push(succID);
1274+
visit(succID);
1275+
stack.pop();
1276+
}
1277+
};
1278+
1279+
stack.push(from);
1280+
visit(from);
1281+
return result;
1282+
}
1283+
12471284
static bool diagnoseCycle(CompilerInstance &instance,
12481285
ModuleDependenciesCache &cache,
12491286
ModuleDependencyID mainId) {
12501287
ModuleDependencyIDSetVector openSet;
12511288
ModuleDependencyIDSetVector closeSet;
1252-
// Start from the main module.
1289+
1290+
auto kindIsSwiftDependency = [&](const ModuleDependencyID &ID) {
1291+
return ID.Kind == swift::ModuleDependencyKind::SwiftInterface ||
1292+
ID.Kind == swift::ModuleDependencyKind::SwiftBinary;
1293+
};
1294+
1295+
auto emitModulePath = [&](const std::vector<ModuleDependencyID> path,
1296+
llvm::SmallString<64> &buffer) {
1297+
llvm::interleave(
1298+
path,
1299+
[&buffer](const ModuleDependencyID &id) {
1300+
buffer.append(id.ModuleName);
1301+
switch (id.Kind) {
1302+
case swift::ModuleDependencyKind::SwiftInterface:
1303+
buffer.append(".swiftinterface");
1304+
break;
1305+
case swift::ModuleDependencyKind::SwiftBinary:
1306+
buffer.append(".swiftmodule");
1307+
break;
1308+
case swift::ModuleDependencyKind::Clang:
1309+
buffer.append(".pcm");
1310+
break;
1311+
default:
1312+
llvm::report_fatal_error(
1313+
Twine("Invalid Module Dependency Kind in cycle: ") +
1314+
id.ModuleName);
1315+
break;
1316+
}
1317+
},
1318+
[&buffer] { buffer.append(" -> "); });
1319+
};
1320+
1321+
auto emitCycleDiagnostic = [&](const ModuleDependencyID &dep) {
1322+
auto startIt = std::find(openSet.begin(), openSet.end(), dep);
1323+
assert(startIt != openSet.end());
1324+
std::vector cycleNodes(startIt, openSet.end());
1325+
cycleNodes.push_back(*startIt);
1326+
llvm::SmallString<64> errorBuffer;
1327+
emitModulePath(cycleNodes, errorBuffer);
1328+
instance.getASTContext().Diags.diagnose(
1329+
SourceLoc(), diag::scanner_find_cycle, errorBuffer.str());
1330+
1331+
// TODO: for (std::tuple<const ModuleDependencyID&, const
1332+
// ModuleDependencyID&> v : cycleNodes | std::views::adjacent<2>)
1333+
for (auto it = cycleNodes.begin(), end = cycleNodes.end(); it != end;
1334+
it++) {
1335+
if (it + 1 == cycleNodes.end())
1336+
continue;
1337+
1338+
const auto &thisID = *it;
1339+
const auto &nextID = *(it + 1);
1340+
if (kindIsSwiftDependency(thisID) && kindIsSwiftDependency(nextID) &&
1341+
llvm::any_of(
1342+
cache.getOnlyOverlayDependencies(thisID),
1343+
[&](const ModuleDependencyID id) { return id == nextID; })) {
1344+
llvm::SmallString<64> noteBuffer;
1345+
auto clangDepPath = findClangDepPath(
1346+
thisID,
1347+
ModuleDependencyID{nextID.ModuleName, ModuleDependencyKind::Clang},
1348+
cache);
1349+
emitModulePath(clangDepPath, noteBuffer);
1350+
instance.getASTContext().Diags.diagnose(
1351+
SourceLoc(), diag::scanner_find_cycle_swift_overlay_path,
1352+
thisID.ModuleName, nextID.ModuleName, noteBuffer.str());
1353+
}
1354+
}
1355+
};
1356+
1357+
// Start from the main module and check direct and overlay dependencies
12531358
openSet.insert(mainId);
12541359
while (!openSet.empty()) {
1255-
auto &lastOpen = openSet.back();
1360+
auto lastOpen = openSet.back();
12561361
auto beforeSize = openSet.size();
12571362
assert(cache.findDependency(lastOpen).has_value() &&
12581363
"Missing dependency info during cycle diagnosis.");
1259-
12601364
for (const auto &dep : cache.getAllDependencies(lastOpen)) {
12611365
if (closeSet.count(dep))
12621366
continue;
12631367
if (openSet.insert(dep)) {
12641368
break;
12651369
} else {
1266-
// Find a cycle, diagnose.
1267-
auto startIt = std::find(openSet.begin(), openSet.end(), dep);
1268-
assert(startIt != openSet.end());
1269-
llvm::SmallString<64> buffer;
1270-
for (auto it = startIt; it != openSet.end(); ++it) {
1271-
buffer.append(it->ModuleName);
1272-
buffer.append((it->Kind == ModuleDependencyKind::SwiftInterface ||
1273-
it->Kind == ModuleDependencyKind::SwiftSource ||
1274-
it->Kind == ModuleDependencyKind::SwiftBinary)
1275-
? ".swiftmodule"
1276-
: ".pcm");
1277-
buffer.append(" -> ");
1278-
}
1279-
buffer.append(startIt->ModuleName);
1280-
buffer.append(
1281-
(startIt->Kind == ModuleDependencyKind::SwiftInterface ||
1282-
startIt->Kind == ModuleDependencyKind::SwiftSource ||
1283-
startIt->Kind == ModuleDependencyKind::SwiftBinary)
1284-
? ".swiftmodule"
1285-
: ".pcm");
1286-
instance.getASTContext().Diags.diagnose(
1287-
SourceLoc(), diag::scanner_find_cycle, buffer.str());
1370+
emitCycleDiagnostic(dep);
12881371
return true;
12891372
}
12901373
}
@@ -1297,6 +1380,7 @@ static bool diagnoseCycle(CompilerInstance &instance,
12971380
}
12981381
}
12991382
assert(openSet.empty());
1383+
closeSet.clear();
13001384
return false;
13011385
}
13021386

‎test/ScanDependencies/Inputs/CHeaders/CycleClangMiddle.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
#include "CycleOverlay.h"

‎test/ScanDependencies/Inputs/CHeaders/CycleOverlay.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
void funcOverlay(void);

‎test/ScanDependencies/Inputs/CHeaders/module.modulemap

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,12 @@ module Y {
5656
module ClangModuleWithOverlayedDep {
5757
header "ClangModuleWithOverlayedDep.h"
5858
export *
59-
}
59+
}
60+
module CycleClangMiddle {
61+
header "CycleClangMiddle.h"
62+
export *
63+
}
64+
module CycleOverlay {
65+
header "CycleOverlay.h"
66+
export *
67+
}

‎test/ScanDependencies/Inputs/Swift/CycleOverlay.swiftinterface

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name CycleOverlay
3+
import Swift
4+
import CycleSwiftMiddle

‎test/ScanDependencies/Inputs/Swift/CycleSwiftMiddle.swiftinterface

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name CycleSwiftMiddle
3+
import Swift
4+
import CycleClangMiddle

‎test/ScanDependencies/diagnose_dependency_cycle.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55

66
// RUN: %FileCheck %s < %t/out.txt
77

8-
// CHECK: dependency scanner detected dependency cycle: 'CycleOne.swiftmodule -> CycleTwo.swiftmodule -> CycleThree.swiftmodule -> CycleOne.swiftmodule'
8+
// CHECK: module dependency cycle: 'CycleOne.swiftinterface -> CycleTwo.swiftinterface -> CycleThree.swiftinterface -> CycleOne.swiftinterface'
99

1010
import CycleOne

0 commit comments

Comments
 (0)