Skip to content

Commit 9eaa0e5

Browse files
xymusxymus
committed
Sema: getImportAccessLevel returns an authoritative import
Change how we pick the one import to point to in diagnostics about a referenced decl. This mostly affects the warning about superfluously public imports. This warning encourages the developer to delete imports, let's make sure we push them towards deleting the right ones. The order was previously not well defined, except that we always picked one of the most public imports. We now prioritize imports in this order: 1. The most public import. (Preserving the current behavior in type-checking of access-level on imports) 2. The import of the public version of the module defining the decl, determined via export_as or -public-module-name. 3. The import of the module defining the decl. 4. The first import in the sources bringing the decl via reexports. 5. Any other import, usually via an @_exported import in a different file. rdar://135357155
1 parent 8d28ed4 commit 9eaa0e5

File tree

2 files changed

+222
-3
lines changed

2 files changed

+222
-3
lines changed

‎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

‎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
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
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)