Skip to content

Commit b4376a9

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 1dfd519 commit b4376a9

18 files changed

+488
-32
lines changed

‎lib/AST/NameLookup.cpp

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -323,23 +323,16 @@ bool swift::removeOverriddenDecls(SmallVectorImpl<ValueDecl*> &decls) {
323323
}
324324

325325
while (auto overrides = decl->getOverriddenDecl()) {
326-
overridden.insert(overrides);
327-
328-
// Because initializers from Objective-C base classes have greater
329-
// visibility than initializers written in Swift classes, we can
330-
// have a "break" in the set of declarations we found, where
331-
// C.init overrides B.init overrides A.init, but only C.init and
332-
// A.init are in the chain. Make sure we still remove A.init from the
333-
// set in this case.
334-
if (decl->getBaseName().isConstructor()) {
335-
/// FIXME: Avoid the possibility of an infinite loop by fixing the root
336-
/// cause instead (incomplete circularity detection).
337-
assert(decl != overrides && "Circular class inheritance?");
338-
decl = overrides;
339-
continue;
326+
if (!overridden.insert(overrides).second) {
327+
// If we've already seen a decl then there's no need to visit the decls
328+
// that it overrides since they should already be in the set. This also
329+
// prevents infinite loops in the case that the AST contains an
330+
// override chain with a cycle due to circular inheritance.
331+
break;
340332
}
341333

342-
break;
334+
DEBUG_ASSERT(decl != overrides && "Circular class inheritance?");
335+
decl = overrides;
343336
}
344337
}
345338

@@ -2388,6 +2381,8 @@ static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
23882381
ValueDecl *decl,
23892382
bool onlyCompleteObjectInits,
23902383
bool requireImport) {
2384+
auto &ctx = dc->getASTContext();
2385+
23912386
// Filter out designated initializers, if requested.
23922387
if (onlyCompleteObjectInits) {
23932388
if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
@@ -2405,19 +2400,43 @@ static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
24052400
}
24062401

24072402
// Check access.
2408-
if (!(options & NL_IgnoreAccessControl) &&
2409-
!dc->getASTContext().isAccessControlDisabled()) {
2403+
if (!(options & NL_IgnoreAccessControl) && !ctx.isAccessControlDisabled()) {
24102404
bool allowUsableFromInline = options & NL_IncludeUsableFromInline;
24112405
if (!decl->isAccessibleFrom(dc, /*forConformance*/ false,
24122406
allowUsableFromInline))
24132407
return false;
24142408
}
24152409

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

24222441
// Check that it has the appropriate ABI role.
24232442
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: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
@import Branch;
2+
3+
@interface FruitObject : BranchObject
4+
- (void)overridden1 __attribute__((deprecated("Fruit.h")));
5+
@end
6+
7+
@interface FruitObject (Fruit)
8+
- (void)overridden4 __attribute__((deprecated("Fruit.h")));
9+
@end

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
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+
- (void)overridden4 __attribute__((deprecated("Leaf.h")));
10+
@end
11+
12+
@interface LeafObject (Leaf)
13+
- (void)overridden3 __attribute__((deprecated("Leaf.h")));
14+
@end
15+

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
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+
- (void)overridden4 __attribute__((deprecated("Root.h")));
10+
@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)