Skip to content

Commit b4376a9

Browse files
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
}
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() }
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
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
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)