Skip to content

Commit 956bd10

Browse files
committed
Prevent shadowing of unavailable member impls
Typically, access control denies access to member implementations, so the imported interface decl will be used instead. However, in contexts that permit direct access to stored properties—such as accessors, inits, and deinits—their member implementations are accessible; the compiler then relies on a shadowing rule favoring Swift decls over ObjC decls to eliminate the imported interface decl. However, there are many rules that are higher-priority than the Swift vs. ObjC decls one. In particular, a recent change to availability checking in #77886 caused a higher-priority rule to begin eliminating member implementations which belonged to unavailable extensions. This caused regressions in projects using `@objc @implementation` with classes that are unavailable in Mac Catalyst. Introduce a fairly high-priority shadowing rule that favors a member implementation over its interface when both are present (i.e. when direct access to storage is permitted). Fixes rdar://143582383.
1 parent e689ad3 commit 956bd10

File tree

3 files changed

+70
-2
lines changed

3 files changed

+70
-2
lines changed

lib/AST/NameLookup.cpp

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,16 @@ static ConstructorComparison compareConstructors(ConstructorDecl *ctor1,
393393
return ConstructorComparison::Same;
394394
}
395395

396+
static bool isMemberImplementation(ValueDecl *VD) {
397+
return VD->isObjCMemberImplementation();
398+
}
399+
static bool isMemberImplementation(OperatorDecl *VD) {
400+
return false;
401+
}
402+
static bool isMemberImplementation(PrecedenceGroupDecl *VD) {
403+
return false;
404+
}
405+
396406
/// Given a set of declarations whose names and interface types have matched,
397407
/// figure out which of these declarations have been shadowed by others.
398408
template <typename T>
@@ -412,7 +422,8 @@ static void recordShadowedDeclsAfterTypeMatch(
412422
for (unsigned firstIdx : indices(decls)) {
413423
auto firstDecl = decls[firstIdx];
414424
auto firstModule = firstDecl->getModuleContext();
415-
bool firstTopLevel = firstDecl->getDeclContext()->isModuleScopeContext();
425+
auto firstDC = firstDecl->getDeclContext();
426+
bool firstTopLevel = firstDC->isModuleScopeContext();
416427

417428
auto name = firstDecl->getBaseName();
418429

@@ -454,7 +465,8 @@ static void recordShadowedDeclsAfterTypeMatch(
454465
// Determine whether one module takes precedence over another.
455466
auto secondDecl = decls[secondIdx];
456467
auto secondModule = secondDecl->getModuleContext();
457-
bool secondTopLevel = secondDecl->getDeclContext()->isModuleScopeContext();
468+
auto secondDC = secondDecl->getDeclContext();
469+
bool secondTopLevel = secondDC->isModuleScopeContext();
458470
bool secondPrivate = isPrivateImport(secondModule);
459471

460472
// For member types, we skip most of the below rules. Instead, we allow
@@ -538,6 +550,22 @@ static void recordShadowedDeclsAfterTypeMatch(
538550
}
539551
}
540552

553+
// Member implementations are usually filtered out by access control.
554+
// They're sometimes visible in contexts that can directly access storage,
555+
// though, and there they should shadow the matching imported declaration.
556+
if (firstDC != secondDC
557+
&& firstDC->getImplementedObjCContext() ==
558+
secondDC->getImplementedObjCContext()) {
559+
if (isMemberImplementation(firstDecl) && secondDecl->hasClangNode()) {
560+
shadowed.insert(secondDecl);
561+
continue;
562+
}
563+
if (isMemberImplementation(secondDecl) && firstDecl->hasClangNode()) {
564+
shadowed.insert(firstDecl);
565+
continue;
566+
}
567+
}
568+
541569
// Swift 4 compatibility hack: Don't shadow properties defined in
542570
// extensions of generic types with properties defined elsewhere.
543571
// This is due to the fact that in Swift 4, we only gave custom overload

test/decl/ext/Inputs/objc_implementation_internal.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,13 @@
66

77
@end
88

9+
@interface ObjCPropertyTest : NSObject
10+
11+
@property (readonly) int prop1;
12+
@property (readwrite) int prop2;
13+
14+
- (instancetype)init;
15+
16+
- (void)doSomething;
17+
18+
@end
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// RUN: %target-typecheck-verify-swift -Xcc -fmodule-map-file=%S/Inputs/objc_implementation_private.modulemap -enable-experimental-feature ObjCImplementation -target %target-stable-abi-triple -debug-diagnostic-names
2+
// REQUIRES: objc_interop
3+
// REQUIRES: swift_feature_ObjCImplementation
4+
5+
import objc_implementation_internal
6+
7+
@available(*, unavailable)
8+
@objc @implementation extension ObjCPropertyTest {
9+
// FIXME: Shouldn't this be on the `@available` above?
10+
// expected-note@+1 {{'prop1' has been explicitly marked unavailable here}}
11+
let prop1: Int32
12+
13+
// expected-note@+1 2 {{'prop2' has been explicitly marked unavailable here}}
14+
var prop2: Int32 {
15+
didSet {
16+
_ = prop2 // expected-error {{'prop2' is unavailable}}
17+
}
18+
}
19+
20+
override init() {
21+
self.prop1 = 1 // expected-error {{'prop1' is unavailable}}
22+
self.prop2 = 2 // expected-error {{'prop2' is unavailable}}
23+
super.init()
24+
}
25+
26+
func doSomething() {
27+
_ = self.prop1
28+
_ = self.prop2
29+
}
30+
}

0 commit comments

Comments
 (0)