Skip to content

Commit 2aabffa

Browse files
xymusxymus
authored
Merge pull request #76358 from xymus/authoritative-import
Sema: Pick the most relevant import for diagnostics about the source of a decl
2 parents 28c6cc1 + 9ce34ab commit 2aabffa

File tree

5 files changed

+228
-6
lines changed

5 files changed

+228
-6
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2672,10 +2672,10 @@ NOTE(add_preconcurrency_to_conformance,none,
26722672
"add '@preconcurrency' to the %0 conformance to defer isolation checking to run time", (DeclName))
26732673
WARNING(remove_public_import,none,
26742674
"public import of %0 was not used in public declarations or inlinable code",
2675-
(const ModuleDecl *))
2675+
(Identifier))
26762676
WARNING(remove_package_import,none,
26772677
"package import of %0 was not used in package declarations",
2678-
(const ModuleDecl *))
2678+
(Identifier))
26792679
WARNING(public_decl_needs_sendable,none,
26802680
"public %kind0 does not specify whether it is 'Sendable' or not",
26812681
(const ValueDecl *))

‎lib/AST/Module.cpp

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2949,17 +2949,44 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const {
29492949
assert(targetModule != getParentModule() &&
29502950
"getImportAccessLevel doesn't support checking for a self-import");
29512951

2952+
/// Order of relevancy of `import` to reach `targetModule`.
2953+
/// Lower is better/more authoritative.
2954+
auto rateImport = [&](const ImportAccessLevel import) -> int {
2955+
auto importedModule = import->module.importedModule;
2956+
2957+
// Prioritize public names:
2958+
if (targetModule->getExportAsName() == importedModule->getBaseIdentifier())
2959+
return 0;
2960+
if (targetModule->getPublicModuleName(/*onlyIfImported*/false) ==
2961+
importedModule->getName())
2962+
return 1;
2963+
2964+
// The defining module or overlay:
2965+
if (targetModule == importedModule)
2966+
return 2;
2967+
if (targetModule == importedModule->getUnderlyingModuleIfOverlay())
2968+
return 3;
2969+
2970+
// Any import in the sources.
2971+
if (import->importLoc.isValid())
2972+
return 4;
2973+
2974+
return 10;
2975+
};
2976+
2977+
// Find the import with the least restrictive access-level.
2978+
// Among those prioritize more relevant one.
29522979
auto &imports = getASTContext().getImportCache();
29532980
ImportAccessLevel restrictiveImport = std::nullopt;
2954-
29552981
for (auto &import : *Imports) {
29562982
if ((!restrictiveImport.has_value() ||
2957-
import.accessLevel > restrictiveImport->accessLevel) &&
2983+
import.accessLevel > restrictiveImport->accessLevel ||
2984+
(import.accessLevel == restrictiveImport->accessLevel &&
2985+
rateImport(import) < rateImport(restrictiveImport))) &&
29582986
imports.isImportedBy(targetModule, import.module.importedModule)) {
29592987
restrictiveImport = import;
29602988
}
29612989
}
2962-
29632990
return restrictiveImport;
29642991
}
29652992

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2781,6 +2781,9 @@ ClangModuleUnit *ClangImporter::Implementation::getWrapperForModule(
27812781
wrapper->setIsSystemModule(underlying->IsSystem);
27822782
wrapper->setIsNonSwiftModule();
27832783
wrapper->setHasResolvedImports();
2784+
if (!underlying->ExportAsModule.empty())
2785+
wrapper->setExportAsName(
2786+
SwiftContext.getIdentifier(underlying->ExportAsModule));
27842787

27852788
auto file = new (SwiftContext) ClangModuleUnit(*wrapper, *this,
27862789
underlying);

‎lib/Sema/TypeCheckAccess.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2552,7 +2552,7 @@ void swift::diagnoseUnnecessaryPublicImports(SourceFile &SF) {
25522552
diag::remove_package_import;
25532553
auto inFlight = ctx.Diags.diagnose(import.importLoc,
25542554
diagId,
2555-
importedModule);
2555+
importedModule->getName());
25562556

25572557
if (levelUsed == AccessLevel::Package) {
25582558
inFlight.fixItReplace(import.accessLevelRange, "package");

‎test/Sema/authoritative-import-priority.swift

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
/// Point to the most relevant import relative to the target decl.
2+
// RUN: %empty-directory(%t)
3+
// RUN: split-file --leading-lines %s %t
4+
5+
/// Build the Swift libraries.
6+
// RUN: %target-swift-frontend -emit-module %t/SwiftPublicNameCore.swift -o %t \
7+
// RUN: -public-module-name SwiftPublicName
8+
// RUN: %target-swift-frontend -emit-module %t/SwiftPublicName.swift -o %t -I %t
9+
// RUN: %target-swift-frontend -emit-module %t/MixedDep.swift -o %t -I %t
10+
11+
/// Client testing order between indirect imports of the same priority.
12+
// RUN: %target-swift-frontend -typecheck -I %t \
13+
// RUN: %t/OrderClient_FileA.swift %t/OrderClient_FileB.swift \
14+
// RUN: -enable-upcoming-feature InternalImportsByDefault \
15+
// RUN: -Rmodule-api-import -verify
16+
17+
/// Client testing order of preference for more levels of imports.
18+
// RUN: %target-swift-frontend -typecheck -I %t \
19+
// RUN: %t/ExportedClient_FileExported.swift %t/ExportedClient_FileA.swift \
20+
// RUN: %t/ExportedClient_FileB.swift %t/ExportedClient_FileC.swift %t/ExportedClient_FileD.swift \
21+
// RUN: -import-underlying-module -module-name ExportedClient \
22+
// RUN: -enable-upcoming-feature InternalImportsByDefault \
23+
// RUN: -Rmodule-api-import -verify
24+
25+
/// Client testing -public-module-name ordering.
26+
// RUN: %target-swift-frontend -typecheck -I %t \
27+
// RUN: %t/SwiftLibClient_FileA.swift %t/SwiftLibClient_FileB.swift \
28+
// RUN: -enable-upcoming-feature InternalImportsByDefault \
29+
// RUN: -Rmodule-api-import -verify
30+
31+
/// Client testing import of the overlay vs an unrelated module.
32+
// RUN: %target-swift-frontend -typecheck -I %t \
33+
// RUN: %t/OverlayClient_FileA.swift %t/OverlayClient_FileB.swift \
34+
// RUN: -enable-upcoming-feature InternalImportsByDefault \
35+
// RUN: -Rmodule-api-import -verify
36+
37+
//--- module.modulemap
38+
module FarClangDep {
39+
header "FarClangDep.h"
40+
}
41+
42+
module MixedDep {
43+
export *
44+
header "MixedDep.h"
45+
}
46+
47+
module IndirectClangDepA {
48+
export *
49+
header "IndirectClangDepA.h"
50+
}
51+
52+
module IndirectClangDepB {
53+
export *
54+
header "IndirectClangDepB.h"
55+
}
56+
57+
module LibCore {
58+
export *
59+
export_as Lib
60+
header "LibCore.h"
61+
}
62+
63+
module Lib {
64+
export *
65+
header "Lib.h"
66+
}
67+
68+
module NotLib {
69+
export *
70+
header "NotLib.h"
71+
}
72+
73+
module ExportedClient {
74+
export *
75+
header "ExportedClient.h"
76+
}
77+
78+
//--- FarClangDep.h
79+
struct FarClangType{};
80+
81+
//--- MixedDep.h
82+
struct UnderlyingType{};
83+
84+
//--- IndirectClangDepA.h
85+
#include <FarClangDep.h>
86+
#include <MixedDep.h>
87+
88+
//--- IndirectClangDepB.h
89+
#include <FarClangDep.h>
90+
#include <MixedDep.h>
91+
92+
//--- LibCore.h
93+
struct ExportedType {};
94+
95+
//--- Lib.h
96+
#include <LibCore.h>
97+
98+
//--- NotLib.h
99+
#include <LibCore.h>
100+
101+
//--- ExportedClient.h
102+
#include <LibCore.h>
103+
104+
//--- SwiftPublicNameCore.swift
105+
public struct SwiftStruct {}
106+
107+
//--- SwiftPublicName.swift
108+
@_exported import SwiftPublicNameCore
109+
110+
//--- MixedDep.swift
111+
@_exported import MixedDep
112+
113+
//--- OrderClient_FileA.swift
114+
/// Between indirect imports, prefer the first one.
115+
public import IndirectClangDepA
116+
public import IndirectClangDepB // expected-warning {{public import of 'IndirectClangDepB' was not used in public declarations or inlinable code}}
117+
118+
public func useTypesB(a: FarClangType) {}
119+
// expected-remark @-1 {{struct 'FarClangType' is imported via 'IndirectClangDepA', which reexports definition from 'FarClangDep'}}
120+
121+
//--- OrderClient_FileB.swift
122+
/// Still prefer the first one after changing the order.
123+
public import IndirectClangDepB
124+
public import IndirectClangDepA // expected-warning {{public import of 'IndirectClangDepA' was not used in public declarations or inlinable code}}
125+
126+
public func useTypesC(a: FarClangType) {}
127+
// expected-remark @-1 {{struct 'FarClangType' is imported via 'IndirectClangDepB', which reexports definition from 'FarClangDep'}}
128+
129+
130+
//--- ExportedClient_FileExported.swift
131+
@_exported public import ExportedClient
132+
133+
//--- ExportedClient_FileA.swift
134+
/// Prefer the defining module.
135+
public import NotLib // expected-warning {{public import of 'NotLib' was not used in public declarations or inlinable code}}
136+
public import LibCore // We should warn here.
137+
public import Lib
138+
139+
public func useTypesA(a: ExportedType) {}
140+
// expected-remark @-1 {{struct 'ExportedType' is imported via 'Lib', which reexports definition from 'LibCore'}}
141+
142+
//--- ExportedClient_FileB.swift
143+
/// Then prefer the export_as module.
144+
public import NotLib // expected-warning {{public import of 'NotLib' was not used in public declarations or inlinable code}}
145+
public import Lib
146+
147+
public func useTypesB(a: ExportedType) {}
148+
// expected-remark @-1 {{struct 'ExportedType' is imported via 'Lib'}}
149+
150+
//--- ExportedClient_FileC.swift
151+
/// Then prefer any local import.
152+
public import NotLib
153+
154+
public func useTypesC(a: ExportedType) {}
155+
// expected-remark @-1 {{struct 'ExportedType' is imported via 'NotLib', which reexports definition from 'LibCore'}}
156+
157+
//--- ExportedClient_FileD.swift
158+
/// Last used the module-wide @_exported import.
159+
public func useTypesD(a: ExportedType) {}
160+
// expected-remark @-1 {{struct 'ExportedType' is imported via 'ExportedClient', which reexports definition from 'LibCore'}}
161+
162+
163+
//--- SwiftLibClient_FileA.swift
164+
/// Prefer the import matching public-module-name.
165+
public import SwiftPublicNameCore // We should warn here.
166+
public import SwiftPublicName
167+
168+
public func useTypesA(a: SwiftStruct) {}
169+
// expected-remark @-1 {{struct 'SwiftStruct' is imported via 'SwiftPublicName', which reexports definition from 'SwiftPublicName'}}
170+
171+
//--- SwiftLibClient_FileB.swift
172+
/// Fallback on read definition site.
173+
public import SwiftPublicNameCore
174+
175+
public func useTypesB(a: SwiftStruct) {}
176+
// expected-remark @-1 {{struct 'SwiftStruct' is imported via 'SwiftPublicName'}}
177+
178+
179+
//--- OverlayClient_FileA.swift
180+
/// Prefer a Swift overlay to an unrelated 3rd module.
181+
public import IndirectClangDepA // expected-warning {{public import of 'IndirectClangDepA' was not used in public declarations or inlinable code}}
182+
public import MixedDep
183+
184+
public func useTypesA(a: UnderlyingType) {}
185+
// expected-remark @-1 {{struct 'UnderlyingType' is imported via 'MixedDep', which reexports definition from 'MixedDep'}}
186+
187+
//--- OverlayClient_FileB.swift
188+
/// Fallback on any reexporter.
189+
public import IndirectClangDepA
190+
191+
public func useTypesB(a: UnderlyingType) {}
192+
// expected-remark @-1 {{struct 'UnderlyingType' is imported via 'IndirectClangDepA', which reexports definition from 'MixedDep'}}

0 commit comments

Comments
 (0)