Skip to content

Commit 0d3def5

Browse files
artemcmartemcm
committed
[Dependency Scanning] Emit a note if a dependency cycle is between the source target and another Swift module with the same name
The note will point the user to where the "other" module with the same name is located and mention whether it is an SDK module. This is nice to have in various circumstances where developers attempt to define a module with the same name as a Swift module that already exists on their search paths, for example in the SDK.
1 parent beb2b58 commit 0d3def5

File tree

6 files changed

+134
-25
lines changed

6 files changed

+134
-25
lines changed

‎include/swift/AST/DiagnosticsCommon.def‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,9 @@ ERROR(scanner_find_cycle, none,
214214
NOTE(scanner_find_cycle_swift_overlay_path, none,
215215
"Swift Overlay dependency of '%0' on '%1' via Clang module dependency: '%2'", (StringRef, StringRef, StringRef))
216216

217+
NOTE(scanner_cycle_source_target_shadow_module, none,
218+
"source target '%0' shadowing a%select{ |n SDK }2Swift module with the same name at: '%1'", (StringRef, StringRef, bool))
219+
217220
ERROR(scanner_arguments_invalid, none,
218221
"dependencies scanner cannot be configured with arguments: '%0'", (StringRef))
219222

‎include/swift/AST/ModuleDependencies.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,12 @@ class ModuleDependencyInfo {
862862
/// Retrieve the dependencies for a Clang module.
863863
const ClangModuleDependencyStorage *getAsClangModule() const;
864864

865+
/// Get the path to the module-defining file:
866+
/// `SwiftInterface`: Textual Interface path
867+
/// `SwiftBinary`: Binary module path
868+
/// `Clang`: Module map path
869+
std::string getModuleDefiningPath() const;
870+
865871
/// Add a dependency on the given module, if it was not already in the set.
866872
void
867873
addOptionalModuleImport(StringRef module, bool isExported,

‎lib/AST/ModuleDependencies.cpp‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,30 @@ ModuleDependencyInfo::getAsClangModule() const {
8888
return dyn_cast<ClangModuleDependencyStorage>(storage.get());
8989
}
9090

91+
std::string ModuleDependencyInfo::getModuleDefiningPath() const {
92+
std::string path = "";
93+
switch (getKind()) {
94+
case swift::ModuleDependencyKind::SwiftInterface:
95+
path = getAsSwiftInterfaceModule()->swiftInterfaceFile;
96+
break;
97+
case swift::ModuleDependencyKind::SwiftBinary:
98+
path = getAsSwiftBinaryModule()->compiledModulePath;
99+
break;
100+
case swift::ModuleDependencyKind::Clang:
101+
path = getAsClangModule()->moduleMapFile;
102+
break;
103+
case swift::ModuleDependencyKind::SwiftSource:
104+
path = getAsSwiftSourceModule()->sourceFiles.front();
105+
break;
106+
default:
107+
llvm_unreachable("Unexpected dependency kind");
108+
}
109+
110+
// Relative to the `module.modulemap` or `.swiftinterface` or `.swiftmodule`,
111+
// the defininig path is the parent directory of the file.
112+
return llvm::sys::path::parent_path(path).str();
113+
}
114+
91115
void ModuleDependencyInfo::addTestableImport(ImportPath::Module module) {
92116
assert(getAsSwiftSourceModule() && "Expected source module for addTestableImport.");
93117
dyn_cast<SwiftSourceModuleDependenciesStorage>(storage.get())->addTestableImport(module);

‎lib/DependencyScan/ModuleDependencyScanner.cpp‎

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1868,28 +1868,6 @@ void ModuleDependencyScanner::diagnoseScannerFailure(
18681868
}
18691869
}
18701870

1871-
static std::string getModuleDefiningPath(const ModuleDependencyInfo &info) {
1872-
std::string path = "";
1873-
switch (info.getKind()) {
1874-
case swift::ModuleDependencyKind::SwiftInterface:
1875-
path = info.getAsSwiftInterfaceModule()->swiftInterfaceFile;
1876-
break;
1877-
case swift::ModuleDependencyKind::SwiftBinary:
1878-
path = info.getAsSwiftBinaryModule()->compiledModulePath;
1879-
break;
1880-
case swift::ModuleDependencyKind::Clang:
1881-
path = info.getAsClangModule()->moduleMapFile;
1882-
break;
1883-
case swift::ModuleDependencyKind::SwiftSource:
1884-
default:
1885-
llvm_unreachable("Unexpected dependency kind");
1886-
}
1887-
1888-
// Relative to the `module.modulemap` or `.swiftinterface` or `.swiftmodule`,
1889-
// the defininig path is the parent directory of the file.
1890-
return llvm::sys::path::parent_path(path).str();
1891-
}
1892-
18931871
std::optional<std::pair<ModuleDependencyID, std::string>>
18941872
ModuleDependencyScanner::attemptToFindResolvingSerializedSearchPath(
18951873
const ScannerImportStatementInfo &moduleImport,
@@ -1922,13 +1900,13 @@ ModuleDependencyScanner::attemptToFindResolvingSerializedSearchPath(
19221900
/* isTestableImport */ false);
19231901
if (!result.empty())
19241902
return std::make_pair(binaryDepID,
1925-
getModuleDefiningPath(result[0].second));
1903+
result[0].second.getModuleDefiningPath());
19261904

19271905
result = ScanningWorker->scanFilesystemForClangModuleDependency(
19281906
getModuleImportIdentifier(moduleImport.importIdentifier), {});
19291907
if (!result.empty())
1930-
return std::make_pair(binaryDepID,
1931-
getModuleDefiningPath(result[0].second));
1908+
return std::make_pair(
1909+
binaryDepID, result[0].second.getModuleDefiningPath());
19321910
return std::nullopt;
19331911
});
19341912
if (result)

‎lib/DependencyScan/ScanDependencies.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,6 +1201,24 @@ static bool diagnoseCycle(const CompilerInstance &instance,
12011201
thisID.ModuleName, nextID.ModuleName, noteBuffer.str());
12021202
}
12031203
}
1204+
1205+
// Check if this is a case of a source target shadowing
1206+
// a module with the same name
1207+
if (sourceId.Kind == swift::ModuleDependencyKind::SwiftSource) {
1208+
auto sinkModuleDefiningPath =
1209+
cache.findKnownDependency(sinkId).getModuleDefiningPath();
1210+
auto SDKPath =
1211+
instance.getInvocation().getSearchPathOptions().getSDKPath();
1212+
auto sinkIsInSDK =
1213+
!SDKPath.empty() &&
1214+
hasPrefix(llvm::sys::path::begin(sinkModuleDefiningPath),
1215+
llvm::sys::path::end(sinkModuleDefiningPath),
1216+
llvm::sys::path::begin(SDKPath),
1217+
llvm::sys::path::end(SDKPath));
1218+
instance.getASTContext().Diags.diagnose(
1219+
SourceLoc(), diag::scanner_cycle_source_target_shadow_module,
1220+
sourceId.ModuleName, sinkModuleDefiningPath, sinkIsInSDK);
1221+
}
12041222
};
12051223

12061224
// Start from the main module and check direct and overlay dependencies

‎test/ScanDependencies/diagnose_dependency_cycle_shadow.swift‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// REQUIRES: objc_interop
2+
// RUN: %empty-directory(%t)
3+
// RUN: %empty-directory(%t/module-cache)
4+
// RUN: %empty-directory(%t/inputs)
5+
6+
// RUN: %empty-directory(%t/mock.sdk/System/Library/Frameworks/A.framework/Modules/A.swiftmodule)
7+
// RUN: %empty-directory(%t/mock.sdk/System/Library/Frameworks/A.framework/Headers)
8+
// RUN: %empty-directory(%t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Modules/CycleKit.swiftmodule)
9+
// RUN: %empty-directory(%t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Headers)
10+
11+
// RUN: split-file %s %t
12+
13+
// RUN: cp %t/inputs/A.swiftinterface %t/mock.sdk/System/Library/Frameworks/A.framework/Modules/A.swiftmodule/%target-swiftinterface-name
14+
// RUN: cp %t/inputs/framework_A.modulemap %t/mock.sdk/System/Library/Frameworks/A.framework/Modules/module.modulemap
15+
// RUN: cp %t/inputs/A-framework.h %t/mock.sdk/System/Library/Frameworks/A.framework/Headers/A.h
16+
17+
// RUN: cp %t/inputs/CycleKit.swiftinterface %t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Modules/CycleKit.swiftmodule/%target-swiftinterface-name
18+
// RUN: cp %t/inputs/framework_CycleKit.modulemap %t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Modules/module.modulemap
19+
// RUN: cp %t/inputs/CycleKit.h %t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Headers/CycleKit.h
20+
21+
// Non-SDK dependency shadowing
22+
// RUN: not %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/test.swift -o %t/deps.json -I %t/inputs -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -module-name CycleKit &> %t/out.txt
23+
// RUN: %FileCheck --check-prefix=CHECK-NONSDK %s < %t/out.txt
24+
25+
// CHECK-NONSDK: note: source target 'CycleKit' shadowing a Swift module with the same name at: '{{.*}}{{/|\\}}diagnose_dependency_cycle_shadow.swift.tmp{{/|\\}}inputs'
26+
27+
// SDK dependency shadowing
28+
// RUN: not %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/test.swift -o %t/deps.json -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -module-name CycleKit -sdk %t/mock.sdk &> %t/out-sdk.txt
29+
// RUN: %FileCheck --check-prefix=CHECK-SDK %s < %t/out-sdk.txt
30+
31+
// CHECK-SDK: note: source target 'CycleKit' shadowing an SDK Swift module with the same name at: '{{.*}}{{/|\\}}mock.sdk{{/|\\}}System{{/|\\}}Library{{/|\\}}Frameworks{{/|\\}}CycleKit.framework{{/|\\}}Modules{{/|\\}}CycleKit.swiftmodule'
32+
33+
//--- test.swift
34+
import A
35+
36+
//--- inputs/CycleKit.swiftinterface
37+
// swift-interface-format-version: 1.0
38+
// swift-module-flags: -module-name CycleKit -enable-library-evolution
39+
40+
public func CycleKitFunc() {}
41+
42+
//--- inputs/A.swiftinterface
43+
// swift-interface-format-version: 1.0
44+
// swift-module-flags: -module-name A -enable-library-evolution
45+
@_exported import A
46+
public func AFunc() {}
47+
48+
//--- inputs/A.h
49+
#import <CycleKit.h>
50+
void funcA(void);
51+
52+
//--- inputs/A-framework.h
53+
#import <CycleKit/CycleKit.h>
54+
void funcA(void);
55+
56+
//--- inputs/CycleKit.h
57+
void funcCycleKit(void);
58+
59+
//--- inputs/module.modulemap
60+
module A {
61+
header "A.h"
62+
export *
63+
}
64+
65+
module CycleKit {
66+
header "CycleKit.h"
67+
export *
68+
}
69+
70+
//--- inputs/framework_A.modulemap
71+
framework module A {
72+
header "A.h"
73+
export *
74+
}
75+
76+
//--- inputs/framework_CycleKit.modulemap
77+
framework module CycleKit {
78+
header "CycleKit.h"
79+
export *
80+
}

0 commit comments

Comments
 (0)