Skip to content

AST: Fix MemberImportVisibility handling of @_exported imports #75511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 5 additions & 36 deletions lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2292,46 +2292,15 @@ ObjCCategoryNameMap ClassDecl::getObjCCategoryNameMap() {
ObjCCategoryNameMap());
}

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

// If the decl is in the same module, no import is required.
auto declModule = decl->getDeclContext()->getParentModule();
if (declModule == dc->getParentModule())
return false;

// Source files are not expected to contain an import for the clang header
// module.
if (auto *loader = dc->getASTContext().getClangModuleLoader()) {
if (declModule == loader->getImportedHeaderModule())
return false;
}

// Only require an import in the context of user authored source file.
auto sf = dc->getParentSourceFile();
if (!sf)
return false;

switch (sf->Kind) {
case SourceFileKind::SIL:
case SourceFileKind::Interface:
case SourceFileKind::MacroExpansion:
case SourceFileKind::DefaultArgument:
return false;
case SourceFileKind::Library:
case SourceFileKind::Main:
break;
}

// If we've found an import, we're done.
if (decl->findImport(dc))
return false;

return true;
return !ctx.getImportCache().isImportedBy(declModule, dc);
}

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

Expand Down
14 changes: 14 additions & 0 deletions test/NameLookup/Inputs/MemberImportVisibility/members_A.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,17 @@ extension Y {
public enum EnumInA {
case caseInA
}

open class BaseClassInA {
open func methodInA() {}
}

public protocol ProtocolInA {
func defaultedRequirementInA()
func defaultedRequirementInB()
func defaultedRequirementInC()
}

extension ProtocolInA {
public func defaultedRequirementInA() { }
}
8 changes: 8 additions & 0 deletions test/NameLookup/Inputs/MemberImportVisibility/members_B.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,11 @@ public enum EnumInB {
package enum EnumInB_package {
case caseInB
}

open class DerivedClassInB: BaseClassInA {
open func methodInB() {}
}

extension ProtocolInA {
public func defaultedRequirementInB() { }
}
8 changes: 8 additions & 0 deletions test/NameLookup/Inputs/MemberImportVisibility/members_C.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,11 @@ extension Y {
public enum EnumInC {
case caseInC
}

open class DerivedClassInC: DerivedClassInB {
open func methodInC() {}
}

extension ProtocolInA {
public func defaultedRequirementInC() { }
}
8 changes: 8 additions & 0 deletions test/NameLookup/members_transitive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,11 @@ func testTopLevelTypes() {
_ = EnumInB_package.self // expected-error{{cannot find 'EnumInB_package' in scope}}
_ = EnumInC.self
}

class DerivedFromClassInC: DerivedClassInC {
override func methodInA() {}
override func methodInB() {} // expected-member-visibility-error{{method does not override any method from its superclass}}
override func methodInC() {}
}

struct ConformsToProtocolInA: ProtocolInA {} // expected-member-visibility-error{{type 'ConformsToProtocolInA' does not conform to protocol 'ProtocolInA'}}
13 changes: 9 additions & 4 deletions test/NameLookup/members_transitive_multifile.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,20 @@

//--- main.swift

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

func testQualifiedMembers() {
takesEnumInA(EnumInA.caseInA) // expected-error{{cannot find 'EnumInA' in scope; did you mean 'EnumInB'?}}
takesEnumInB(EnumInB.caseInB)
takesEnumInC(EnumInC.caseInC) // expected-error{{cannot find 'EnumInC' in scope; did you mean 'EnumInB'?}}
}

//--- A.swift

@_implementationOnly import members_A
Expand Down
5 changes: 2 additions & 3 deletions test/NameLookup/members_transitive_underlying_clang.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@
// RUN: split-file %s %t
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 5
// RUN: %target-swift-frontend -typecheck -primary-file %t/Primary.swift %t/Other.swift -I %S/Inputs/MemberImportVisibility -module-name Underlying -verify -swift-version 6
// 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-
// 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

//--- Other.swift
@_exported import Underlying

//--- Primary.swift

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