Skip to content

Commit 9aab6ef

Browse files
committed
AST: Fix MemberImportVisibility handling of @_exported imports.
In existing Swift, an `@_exported import` in any source file makes the declarations from the imported module visible in all source files. It's unclear whether this is an explicit decision or is simply and unintended consequence of effectively adding an implicit import to each source file for the module being compiled. Although it's not clear whether this behavior is desirable, the behavior of member lookup when the MemberImportVisibility feature is enabled should align with it in order to avoid causing unnecessary churn in required imports. Resolves rdar://132525152.
1 parent 7d65170 commit 9aab6ef

File tree

7 files changed

+54
-43
lines changed

7 files changed

+54
-43
lines changed

lib/AST/NameLookup.cpp

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2292,46 +2292,15 @@ ObjCCategoryNameMap ClassDecl::getObjCCategoryNameMap() {
22922292
ObjCCategoryNameMap());
22932293
}
22942294

2295-
static bool missingExplicitImportForMemberDecl(const DeclContext *dc,
2296-
ValueDecl *decl) {
2295+
static bool missingImportForMemberDecl(const DeclContext *dc, ValueDecl *decl) {
22972296
// Only require explicit imports for members when MemberImportVisibility is
22982297
// enabled.
2299-
if (!dc->getASTContext().LangOpts.hasFeature(Feature::MemberImportVisibility))
2298+
auto &ctx = dc->getASTContext();
2299+
if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility))
23002300
return false;
23012301

2302-
// If the decl is in the same module, no import is required.
23032302
auto declModule = decl->getDeclContext()->getParentModule();
2304-
if (declModule == dc->getParentModule())
2305-
return false;
2306-
2307-
// Source files are not expected to contain an import for the clang header
2308-
// module.
2309-
if (auto *loader = dc->getASTContext().getClangModuleLoader()) {
2310-
if (declModule == loader->getImportedHeaderModule())
2311-
return false;
2312-
}
2313-
2314-
// Only require an import in the context of user authored source file.
2315-
auto sf = dc->getParentSourceFile();
2316-
if (!sf)
2317-
return false;
2318-
2319-
switch (sf->Kind) {
2320-
case SourceFileKind::SIL:
2321-
case SourceFileKind::Interface:
2322-
case SourceFileKind::MacroExpansion:
2323-
case SourceFileKind::DefaultArgument:
2324-
return false;
2325-
case SourceFileKind::Library:
2326-
case SourceFileKind::Main:
2327-
break;
2328-
}
2329-
2330-
// If we've found an import, we're done.
2331-
if (decl->findImport(dc))
2332-
return false;
2333-
2334-
return true;
2303+
return !ctx.getImportCache().isImportedBy(declModule, dc);
23352304
}
23362305

23372306
/// Determine whether the given declaration is an acceptable lookup
@@ -2368,7 +2337,7 @@ static bool isAcceptableLookupResult(const DeclContext *dc,
23682337
// Check that there is some import in the originating context that makes this
23692338
// decl visible.
23702339
if (!(options & NL_IgnoreMissingImports)) {
2371-
if (missingExplicitImportForMemberDecl(dc, decl))
2340+
if (missingImportForMemberDecl(dc, decl))
23722341
return false;
23732342
}
23742343

test/NameLookup/Inputs/MemberImportVisibility/members_A.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,17 @@ extension Y {
3131
public enum EnumInA {
3232
case caseInA
3333
}
34+
35+
open class BaseClassInA {
36+
open func methodInA() {}
37+
}
38+
39+
public protocol ProtocolInA {
40+
func defaultedRequirementInA()
41+
func defaultedRequirementInB()
42+
func defaultedRequirementInC()
43+
}
44+
45+
extension ProtocolInA {
46+
public func defaultedRequirementInA() { }
47+
}

test/NameLookup/Inputs/MemberImportVisibility/members_B.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,11 @@ public enum EnumInB {
2727
package enum EnumInB_package {
2828
case caseInB
2929
}
30+
31+
open class DerivedClassInB: BaseClassInA {
32+
open func methodInB() {}
33+
}
34+
35+
extension ProtocolInA {
36+
public func defaultedRequirementInB() { }
37+
}

test/NameLookup/Inputs/MemberImportVisibility/members_C.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,11 @@ extension Y {
2121
public enum EnumInC {
2222
case caseInC
2323
}
24+
25+
open class DerivedClassInC: DerivedClassInB {
26+
open func methodInC() {}
27+
}
28+
29+
extension ProtocolInA {
30+
public func defaultedRequirementInC() { }
31+
}

test/NameLookup/members_transitive.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,11 @@ func testTopLevelTypes() {
6767
_ = EnumInB_package.self // expected-error{{cannot find 'EnumInB_package' in scope}}
6868
_ = EnumInC.self
6969
}
70+
71+
class DerivedFromClassInC: DerivedClassInC {
72+
override func methodInA() {}
73+
override func methodInB() {} // expected-member-visibility-error{{method does not override any method from its superclass}}
74+
override func methodInC() {}
75+
}
76+
77+
struct ConformsToProtocolInA: ProtocolInA {} // expected-member-visibility-error{{type 'ConformsToProtocolInA' does not conform to protocol 'ProtocolInA'}}

test/NameLookup/members_transitive_multifile.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,20 @@
99

1010
//--- main.swift
1111

12-
// expected-member-visibility-note@+3 {{add import of module 'members_A'}}{{1-1=@_implementationOnly import members_A\n}}
13-
// expected-member-visibility-note@+2 {{add import of module 'members_B'}}{{1-1=@_exported import members_B\n}}
12+
// expected-member-visibility-note@+2 {{add import of module 'members_A'}}{{1-1=@_implementationOnly import members_A\n}}
1413
// expected-member-visibility-note@+1 {{add import of module 'members_C'}}{{1-1=@_weakLinked @_spiOnly import members_C\n}}
15-
func testMembersWithContextualBase() {
14+
func testMembersWithInferredContextualBase() {
1615
takesEnumInA(.caseInA) // expected-member-visibility-error{{enum case 'caseInA' is not available due to missing import of defining module 'members_A'}}
17-
takesEnumInB(.caseInB) // expected-member-visibility-error{{enum case 'caseInB' is not available due to missing import of defining module 'members_B'}}
16+
takesEnumInB(.caseInB)
1817
takesEnumInC(.caseInC) // expected-member-visibility-error{{enum case 'caseInC' is not available due to missing import of defining module 'members_C'}}
1918
}
2019

20+
func testQualifiedMembers() {
21+
takesEnumInA(EnumInA.caseInA) // expected-error{{cannot find 'EnumInA' in scope; did you mean 'EnumInB'?}}
22+
takesEnumInB(EnumInB.caseInB)
23+
takesEnumInC(EnumInC.caseInC) // expected-error{{cannot find 'EnumInC' in scope; did you mean 'EnumInB'?}}
24+
}
25+
2126
//--- A.swift
2227

2328
@_implementationOnly import members_A

test/NameLookup/members_transitive_underlying_clang.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,13 @@
22
// RUN: split-file %s %t
33
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 5
44
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 6
5-
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 5 -enable-experimental-feature MemberImportVisibility -verify-additional-prefix member-visibility-
5+
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 5 -enable-experimental-feature MemberImportVisibility
66

77
//--- Other.swift
88
@_exported import Underlying
99

1010
//--- Primary.swift
1111

12-
// expected-member-visibility-note@+1 {{add import of module 'Underlying'}}{{1-1=@_exported import Underlying\n}}
1312
func test(_ s: UnderlyingStruct) {
14-
_ = s.a // expected-member-visibility-error {{property 'a' is not available due to missing import of defining module 'Underlying'}}
13+
_ = s.a
1514
}

0 commit comments

Comments
 (0)