Skip to content

Commit 8823a3f

Browse files
tshortlitshortli
authored
Merge pull request #81090 from tshortli/member-import-visibility-objc-overloads-in-extensions-take-2-6.2
[6.2] AST: Filter out some Obj-C overrides when MemberImportVisibility is enabled
2 parents 6e270c6 + 4f61908 commit 8823a3f

20 files changed

+491
-35
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))

‎lib/Sema/TypeCheckNameLookup.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,7 @@ missingImportsForDefiningModule(ModuleDecl *owningModule, SourceFile &sf) {
835835
}
836836

837837
std::sort(result.begin(), result.end(), [](ModuleDecl *LHS, ModuleDecl *RHS) {
838-
return LHS->getNameStr() < LHS->getNameStr();
838+
return LHS->getNameStr() < RHS->getNameStr();
839839
});
840840

841841
return result;

‎test/CrossImport/member-import-visibility.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ import Swift
3939
// expected-note@-1 2 {{add import of module 'DeclaringLibrary'}}
4040

4141
private func test() {
42-
returnsDeclaringTy().overlayMember() // expected-error {{instance method 'overlayMember()' is not available due to missing imports of defining modules 'DeclaringLibrary' and 'BystandingLibrary'}}
43-
returnsBystandingTy().overlayMember() // expected-error {{instance method 'overlayMember()' is not available due to missing imports of defining modules 'DeclaringLibrary' and 'BystandingLibrary'}}
42+
returnsDeclaringTy().overlayMember() // expected-error {{instance method 'overlayMember()' is not available due to missing imports of defining modules 'BystandingLibrary' and 'DeclaringLibrary'}}
43+
returnsBystandingTy().overlayMember() // expected-error {{instance method 'overlayMember()' is not available due to missing imports of defining modules 'BystandingLibrary' and 'DeclaringLibrary'}}
4444
}
4545

4646
//--- BothDeclaringAndBystanding.swift

‎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)