Skip to content

Commit f3ef070

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 63db342 commit f3ef070

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

@@ -2395,6 +2388,8 @@ static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
23952388
ValueDecl *decl,
23962389
bool onlyCompleteObjectInits,
23972390
bool requireImport) {
2391+
auto &ctx = dc->getASTContext();
2392+
23982393
// Filter out designated initializers, if requested.
23992394
if (onlyCompleteObjectInits) {
24002395
if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
@@ -2412,19 +2407,43 @@ static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
24122407
}
24132408

24142409
// Check access.
2415-
if (!(options & NL_IgnoreAccessControl) &&
2416-
!dc->getASTContext().isAccessControlDisabled()) {
2410+
if (!(options & NL_IgnoreAccessControl) && !ctx.isAccessControlDisabled()) {
24172411
bool allowUsableFromInline = options & NL_IncludeUsableFromInline;
24182412
if (!decl->isAccessibleFrom(dc, /*forConformance*/ false,
24192413
allowUsableFromInline))
24202414
return false;
24212415
}
24222416

2423-
// Check that there is some import in the originating context that makes this
2424-
// decl visible.
2425-
if (requireImport && !(options & NL_IgnoreMissingImports))
2426-
if (!dc->isDeclImported(decl))
2427-
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+
}
24282447

24292448
// Check that it has the appropriate ABI role.
24302449
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+

0 commit comments

Comments
 (0)