Skip to content

Commit 374fc90

Browse files
tshortlitshortli
authored
Merge pull request #80563 from tshortli/member-import-visibility-objc-overloads-in-extensions
AST: Filter out some Obj-C overrides when MemberImportVisibility is enabled
2 parents 98739a4 + eafb847 commit 374fc90

18 files changed

+409
-17
lines changed

‎lib/AST/NameLookup.cpp

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2388,6 +2388,8 @@ static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
23882388
ValueDecl *decl,
23892389
bool onlyCompleteObjectInits,
23902390
bool requireImport) {
2391+
auto &ctx = dc->getASTContext();
2392+
23912393
// Filter out designated initializers, if requested.
23922394
if (onlyCompleteObjectInits) {
23932395
if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
@@ -2405,19 +2407,43 @@ static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
24052407
}
24062408

24072409
// Check access.
2408-
if (!(options & NL_IgnoreAccessControl) &&
2409-
!dc->getASTContext().isAccessControlDisabled()) {
2410+
if (!(options & NL_IgnoreAccessControl) && !ctx.isAccessControlDisabled()) {
24102411
bool allowUsableFromInline = options & NL_IncludeUsableFromInline;
24112412
if (!decl->isAccessibleFrom(dc, /*forConformance*/ false,
24122413
allowUsableFromInline))
24132414
return false;
24142415
}
24152416

2416-
// Check that there is some import in the originating context that makes this
2417-
// decl visible.
2418-
if (requireImport && !(options & NL_IgnoreMissingImports))
2419-
if (!dc->isDeclImported(decl))
2420-
return false;
2417+
if (requireImport) {
2418+
// Check that there is some import in the originating context that makes
2419+
// this decl visible.
2420+
if (!(options & NL_IgnoreMissingImports)) {
2421+
if (!dc->isDeclImported(decl))
2422+
return false;
2423+
}
2424+
2425+
// Unlike in Swift, Obj-C allows method overrides to be declared in
2426+
// extensions (categories), even outside of the module that defines the
2427+
// type that is being extended. When MemberImportVisibility is enabled,
2428+
// if these overrides are not filtered out they can hijack name
2429+
// lookup and cause the compiler to insist that the module that defines
2430+
// the extension be imported, contrary to developer expectations.
2431+
//
2432+
// Filter results belonging to these extensions out, even when ignoring
2433+
// missing imports, if we're in a context that requires imports to access
2434+
// member declarations.
2435+
if (decl->getOverriddenDecl()) {
2436+
if (auto *extension = dyn_cast<ExtensionDecl>(decl->getDeclContext())) {
2437+
if (auto *nominal = extension->getExtendedNominal()) {
2438+
auto extensionMod = extension->getModuleContext();
2439+
auto nominalMod = nominal->getModuleContext();
2440+
if (!extensionMod->isSameModuleLookingThroughOverlays(nominalMod) &&
2441+
!dc->isDeclImported(extension))
2442+
return false;
2443+
}
2444+
}
2445+
}
2446+
}
24212447

24222448
// Check that it has the appropriate ABI role.
24232449
if (!ABIRoleInfo(decl).matchesOptions(options))

‎test/NameLookup/Inputs/MemberImportVisibility/Categories_A.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
@import Foundation;
22

3-
@interface X
3+
@interface Base
4+
- (void)overriddenInOverlayForA __attribute__((deprecated("Categories_A.h")));
5+
- (void)overriddenInOverlayForB __attribute__((deprecated("Categories_A.h")));
6+
- (void)overriddenInOverlayForC __attribute__((deprecated("Categories_A.h")));
7+
- (void)overriddenInSubclassInOverlayForC __attribute__((deprecated("Categories_A.h")));
8+
@end
9+
10+
@interface X : Base
411
@end
512

613
@interface X (A)

‎test/NameLookup/Inputs/MemberImportVisibility/Categories_A.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,7 @@
33
extension X {
44
public func fromOverlayForA() {}
55
@objc public func fromOverlayForAObjC() {}
6+
7+
@available(*, deprecated, message: "Categories_A.swift")
8+
public override func overriddenInOverlayForA() {}
69
}

‎test/NameLookup/Inputs/MemberImportVisibility/Categories_B.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,7 @@
33
extension X {
44
public func fromOverlayForB() {}
55
@objc public func fromOverlayForBObjC() {}
6+
7+
@available(*, deprecated, message: "Categories_B.swift")
8+
public override func overriddenInOverlayForB() {}
69
}

‎test/NameLookup/Inputs/MemberImportVisibility/Categories_C.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,7 @@
33
@interface X (C)
44
- (void)fromC;
55
@end
6+
7+
@interface SubclassFromC : X
8+
- (instancetype)init;
9+
@end

‎test/NameLookup/Inputs/MemberImportVisibility/Categories_C.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,12 @@
33
extension X {
44
public func fromOverlayForC() {}
55
@objc public func fromOverlayForCObjC() {}
6+
7+
@available(*, deprecated, message: "Categories_C.swift")
8+
public override func overriddenInOverlayForC() {}
9+
}
10+
11+
extension SubclassFromC {
12+
@available(*, deprecated, message: "Categories_C.swift")
13+
public override func overriddenInSubclassInOverlayForC() {}
614
}

‎test/NameLookup/Inputs/MemberImportVisibility/Categories_E.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
import Categories_C
22
import Categories_D.Submodule
3+
4+
public func makeSubclassFromC() -> SubclassFromC { SubclassFromC() }

‎test/NameLookup/Inputs/MemberImportVisibility/ObjCOverloads/Branch.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
@import Root;
2+
3+
@interface BranchObject : RootObject
4+
- (void)overridden1 __attribute__((deprecated("Branch.h")));
5+
@end
6+
7+
@interface BranchObject (Branch)
8+
- (void)overridden3 __attribute__((deprecated("Branch.h")));
9+
@end

‎test/NameLookup/Inputs/MemberImportVisibility/ObjCOverloads/Fruit.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
@import Branch;
2+
3+
@interface FruitObject : BranchObject
4+
- (void)overridden1 __attribute__((deprecated("Fruit.h")));
5+
@end

‎test/NameLookup/Inputs/MemberImportVisibility/ObjCOverloads/Leaf.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
@import Branch;
2+
3+
@interface LeafObject : BranchObject
4+
- (void)overridden1 __attribute__((deprecated("Leaf.h")));
5+
@end
6+
7+
@interface BranchObject (Leaf)
8+
- (void)overridden2 __attribute__((deprecated("Leaf.h")));
9+
@end
10+
11+
@interface LeafObject (Leaf)
12+
- (void)overridden3 __attribute__((deprecated("Leaf.h")));
13+
@end
14+

‎test/NameLookup/Inputs/MemberImportVisibility/ObjCOverloads/Root.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
@interface RootObject
2+
- (instancetype)init;
3+
- (void)overridden1 __attribute__((deprecated("Root.h")));
4+
- (void)overridden2 __attribute__((deprecated("Root.h")));
5+
@end
6+
7+
@interface RootObject (Root)
8+
- (void)overridden3 __attribute__((deprecated("Root.h")));
9+
@end

‎test/NameLookup/Inputs/MemberImportVisibility/ObjCOverloads/module.modulemap

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
module Root {
2+
header "Root.h"
3+
export *
4+
}
5+
6+
module Branch {
7+
header "Branch.h"
8+
export *
9+
}
10+
11+
module Leaf {
12+
header "Leaf.h"
13+
export *
14+
}
15+
16+
module Fruit {
17+
header "Fruit.h"
18+
export *
19+
}

‎test/NameLookup/Inputs/MemberImportVisibility/members_A.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ public enum EnumInA {
3535
}
3636

3737
open class BaseClassInA {
38-
public init() {}
3938
open func methodInA() {}
39+
open func overriddenMethod() {}
4040
}
4141

4242
public protocol ProtocolInA {

‎test/NameLookup/Inputs/MemberImportVisibility/members_B.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ package enum EnumInB_package {
3737

3838
open class DerivedClassInB: BaseClassInA {
3939
open func methodInB() {}
40+
open override func overriddenMethod() {}
4041
}
4142

4243
extension ProtocolInA {

‎test/NameLookup/Inputs/MemberImportVisibility/members_C.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ public enum EnumInC {
3131

3232
open class DerivedClassInC: DerivedClassInB {
3333
open func methodInC() {}
34+
open override func overriddenMethod() {}
35+
public func asDerivedClassInB() -> DerivedClassInB { return self }
3436
}
3537

3638
extension ProtocolInA {

‎test/NameLookup/members_transitive.swift

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// REQUIRES: swift_feature_MemberImportVisibility
1010

1111
import members_C
12-
// expected-member-visibility-note 16{{add import of module 'members_B'}}{{1-1=internal import members_B\n}}
12+
// expected-member-visibility-note 18{{add import of module 'members_B'}}{{1-1=internal import members_B\n}}
1313

1414

1515
func testExtensionMembers(x: X, y: Y<Z>) {
@@ -96,8 +96,22 @@ class DerivedFromClassInC: DerivedClassInC {
9696

9797
struct ConformsToProtocolInA: ProtocolInA {} // expected-member-visibility-error{{type 'ConformsToProtocolInA' does not conform to protocol 'ProtocolInA'}} expected-member-visibility-note {{add stubs for conformance}}
9898

99-
func testDerivedMethodAccess() {
100-
DerivedClassInC().methodInC()
101-
DerivedClassInC().methodInB() // expected-member-visibility-error{{instance method 'methodInB()' is not available due to missing import of defining module 'members_B'}}
102-
DerivedFromClassInC().methodInB()
99+
func testInheritedMethods(
100+
a: BaseClassInA,
101+
c: DerivedClassInC,
102+
) {
103+
let b = c.asDerivedClassInB()
104+
105+
a.methodInA()
106+
b.methodInA()
107+
c.methodInA()
108+
109+
b.methodInB() // expected-member-visibility-error{{instance method 'methodInB()' is not available due to missing import of defining module 'members_B'}}
110+
c.methodInB() // expected-member-visibility-error{{instance method 'methodInB()' is not available due to missing import of defining module 'members_B'}}
111+
112+
c.methodInC()
113+
114+
a.overriddenMethod()
115+
b.overriddenMethod() // expected-member-visibility-error{{instance method 'overriddenMethod()' is not available due to missing import of defining module 'members_B'}}
116+
c.overriddenMethod()
103117
}

‎test/NameLookup/members_transitive_objc.swift

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
// RUN: %target-swift-frontend -emit-module -I %t -I %S/Inputs/MemberImportVisibility -o %t %S/Inputs/MemberImportVisibility/Categories_B.swift
44
// RUN: %target-swift-frontend -emit-module -I %t -I %S/Inputs/MemberImportVisibility -o %t %S/Inputs/MemberImportVisibility/Categories_C.swift
55
// RUN: %target-swift-frontend -emit-module -I %t -I %S/Inputs/MemberImportVisibility -o %t %S/Inputs/MemberImportVisibility/Categories_E.swift
6-
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 5
7-
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 6
6+
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 5 -verify-additional-prefix no-member-visibility-
7+
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 6 -verify-additional-prefix no-member-visibility-
88
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 5 -enable-upcoming-feature MemberImportVisibility -verify-additional-prefix member-visibility-
99

1010
// REQUIRES: objc_interop
@@ -13,30 +13,43 @@
1313
import Categories_B
1414
import Categories_E
1515

16-
// expected-member-visibility-note@-1 2 {{add import of module 'Categories_C'}}{{1-1=internal import Categories_C\n}}
16+
// expected-member-visibility-note@-1 3 {{add import of module 'Categories_C'}}{{1-1=internal import Categories_C\n}}
1717
// expected-member-visibility-note@-2 {{add import of module 'Categories_D'}}{{1-1=internal import Categories_D\n}}
1818
func test(x: X) {
1919
x.fromA()
2020
x.fromOverlayForA()
21+
x.overriddenInOverlayForA() // expected-warning {{'overriddenInOverlayForA()' is deprecated: Categories_A.swift}}
2122
x.fromB()
2223
x.fromOverlayForB()
24+
x.overriddenInOverlayForB() // expected-warning {{'overriddenInOverlayForB()' is deprecated: Categories_B.swift}}
2325
x.fromC() // expected-member-visibility-error {{instance method 'fromC()' is not available due to missing import of defining module 'Categories_C'}}
2426
x.fromOverlayForC() // expected-member-visibility-error {{instance method 'fromOverlayForC()' is not available due to missing import of defining module 'Categories_C'}}
27+
x.overriddenInOverlayForC()
28+
// expected-no-member-visibility-warning@-1 {{'overriddenInOverlayForC()' is deprecated: Categories_C.swift}}
29+
// expected-member-visibility-warning@-2 {{'overriddenInOverlayForC()' is deprecated: Categories_A.h}}
2530
x.fromSubmoduleOfD() // expected-member-visibility-error {{instance method 'fromSubmoduleOfD()' is not available due to missing import of defining module 'Categories_D'}}
2631
x.fromBridgingHeader()
2732
x.overridesCategoryMethodOnNSObject()
33+
34+
let subclassFromC = makeSubclassFromC()
35+
subclassFromC.overriddenInSubclassInOverlayForC()
36+
// expected-warning@-1 {{'overriddenInSubclassInOverlayForC()' is deprecated: Categories_C.swift}}
37+
// expected-member-visibility-error@-2 {{instance method 'overriddenInSubclassInOverlayForC()' is not available due to missing import of defining module 'Categories_C'}}
2838
}
2939

3040
func testAnyObject(a: AnyObject) {
3141
a.fromA()
3242
a.fromOverlayForAObjC()
43+
a.overriddenInOverlayForA() // expected-warning {{'overriddenInOverlayForA()' is deprecated: Categories_A.h}}
3344
a.fromB()
3445
a.fromOverlayForBObjC()
46+
a.overriddenInOverlayForB() // expected-warning {{'overriddenInOverlayForB()' is deprecated: Categories_A.h}}
3547
// FIXME: Better diagnostics?
3648
// Name lookup for AnyObject already ignored transitive imports, so
3749
// `MemberImportVisibility` has no effect on these diagnostics.
3850
a.fromC() // expected-error {{value of type 'AnyObject' has no member 'fromC'}}
3951
a.fromOverlayForCObjC() // expected-error {{value of type 'AnyObject' has no member 'fromOverlayForCObjC'}}
52+
a.overriddenInOverlayForC() // expected-warning {{'overriddenInOverlayForC()' is deprecated: Categories_A.h}}
4053
a.fromBridgingHeader()
4154
a.overridesCategoryMethodOnNSObject()
4255
}

0 commit comments

Comments
 (0)