From 9aab6ef33e8aa4a6bef5cdf9462ea38eef6f2ad3 Mon Sep 17 00:00:00 2001 From: Allan Shortlidge Date: Fri, 26 Jul 2024 10:13:10 -0700 Subject: [PATCH] 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. --- lib/AST/NameLookup.cpp | 41 +++---------------- .../MemberImportVisibility/members_A.swift | 14 +++++++ .../MemberImportVisibility/members_B.swift | 8 ++++ .../MemberImportVisibility/members_C.swift | 8 ++++ test/NameLookup/members_transitive.swift | 8 ++++ .../members_transitive_multifile.swift | 13 ++++-- .../members_transitive_underlying_clang.swift | 5 +-- 7 files changed, 54 insertions(+), 43 deletions(-) diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index 617d79c5c9e69..2c5b722e40a1c 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -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 @@ -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; } diff --git a/test/NameLookup/Inputs/MemberImportVisibility/members_A.swift b/test/NameLookup/Inputs/MemberImportVisibility/members_A.swift index b53fe7a447023..7ecacc1d3f8d6 100644 --- a/test/NameLookup/Inputs/MemberImportVisibility/members_A.swift +++ b/test/NameLookup/Inputs/MemberImportVisibility/members_A.swift @@ -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() { } +} diff --git a/test/NameLookup/Inputs/MemberImportVisibility/members_B.swift b/test/NameLookup/Inputs/MemberImportVisibility/members_B.swift index 879b2a5b0d87d..66441f6b82efd 100644 --- a/test/NameLookup/Inputs/MemberImportVisibility/members_B.swift +++ b/test/NameLookup/Inputs/MemberImportVisibility/members_B.swift @@ -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() { } +} diff --git a/test/NameLookup/Inputs/MemberImportVisibility/members_C.swift b/test/NameLookup/Inputs/MemberImportVisibility/members_C.swift index 059057ef77511..4a51d066c5b41 100644 --- a/test/NameLookup/Inputs/MemberImportVisibility/members_C.swift +++ b/test/NameLookup/Inputs/MemberImportVisibility/members_C.swift @@ -21,3 +21,11 @@ extension Y { public enum EnumInC { case caseInC } + +open class DerivedClassInC: DerivedClassInB { + open func methodInC() {} +} + +extension ProtocolInA { + public func defaultedRequirementInC() { } +} diff --git a/test/NameLookup/members_transitive.swift b/test/NameLookup/members_transitive.swift index 18b3ede655add..4fb6947dfbbf0 100644 --- a/test/NameLookup/members_transitive.swift +++ b/test/NameLookup/members_transitive.swift @@ -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'}} diff --git a/test/NameLookup/members_transitive_multifile.swift b/test/NameLookup/members_transitive_multifile.swift index 0c0de14179014..e3821c43f2695 100644 --- a/test/NameLookup/members_transitive_multifile.swift +++ b/test/NameLookup/members_transitive_multifile.swift @@ -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 diff --git a/test/NameLookup/members_transitive_underlying_clang.swift b/test/NameLookup/members_transitive_underlying_clang.swift index e85e68ca27210..27ef61a712dfa 100644 --- a/test/NameLookup/members_transitive_underlying_clang.swift +++ b/test/NameLookup/members_transitive_underlying_clang.swift @@ -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 }