Skip to content

Commit 324fa23

Browse files
tshortlitshortli
committed
AST: Filter out some Obj-C overrides when MemberImportVisibility is enabled.
Unlike in Swift, Obj-C allows method overrides to be declared in extensions (categories), even outside of the module that defines the type that is being extended. When MemberImportVisibility is enabled, these overrides must be filtered out to prevent them from hijacking name lookup and causing the compiler to insist that the module that defines the extension be imported. Resolves rdar://145329988.
1 parent 1b2f8c3 commit 324fa23

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)