Skip to content

Commit 825fb87

Browse files
beccadaxbeccadax
committed
Don’t diagnose @_objcImpl conflicts with inherited inits
Previously, Swift would reject an `override public init(…)` in an `@_objcImplementation` because ClangImporter would have already synthesized inherited initializers that conflicted with the overrides. Ignore these spurious conflicts, and also move a check out of IsObjCRequest and into the conflict-handling code. Additional work towards rdar://70730077.
1 parent 54dc786 commit 825fb87

File tree

6 files changed

+223
-2
lines changed

6 files changed

+223
-2
lines changed

‎lib/AST/NameLookup.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1701,11 +1701,24 @@ NominalTypeDecl::lookupDirect(ObjCSelector selector, bool isInstance) {
17011701
return stored.Methods;
17021702
}
17031703

1704+
static bool inObjCImplExtension(AbstractFunctionDecl *newDecl) {
1705+
if (auto ext = dyn_cast<ExtensionDecl>(newDecl->getDeclContext()))
1706+
return ext->isObjCImplementation();
1707+
return false;
1708+
}
1709+
17041710
/// If there is an apparent conflict between \p newDecl and one of the methods
17051711
/// in \p vec, should we diagnose it?
17061712
static bool
17071713
shouldDiagnoseConflict(NominalTypeDecl *ty, AbstractFunctionDecl *newDecl,
17081714
llvm::TinyPtrVector<AbstractFunctionDecl *> &vec) {
1715+
// Conflicts between member implementations and their interfaces, or
1716+
// inherited inits and their overrides in @_objcImpl extensions, are spurious.
1717+
if (newDecl->isObjCMemberImplementation()
1718+
|| (isa<ConstructorDecl>(newDecl) && inObjCImplExtension(newDecl)
1719+
&& newDecl->getAttrs().hasAttribute<OverrideAttr>()))
1720+
return false;
1721+
17091722
// Are all conflicting methods imported from ObjC and in our ObjC half or a
17101723
// bridging header? Some code bases implement ObjC methods in Swift even
17111724
// though it's not exactly supported.

‎lib/Sema/TypeCheckDeclObjC.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,8 +2042,7 @@ void markAsObjC(ValueDecl *D, ObjCReason reason,
20422042

20432043
// Record the method in the type, if it's a member of one.
20442044
if (auto tyDecl = D->getDeclContext()->getSelfNominalTypeDecl()) {
2045-
if (!method->isObjCMemberImplementation())
2046-
tyDecl->recordObjCMethod(method, selector);
2045+
tyDecl->recordObjCMethod(method, selector);
20472046
}
20482047

20492048
// Record the method in the source file.

‎test/decl/ext/Inputs/objc_implementation.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
@interface ObjCBaseClass
22

3+
4+
// Need two initializers to reproduce certain conflict bugs.
5+
- (instancetype)initFromSuperclass:(int)param __attribute__((objc_designated_initializer));
6+
- (instancetype)initFromSuperclass2:(int)param __attribute__((objc_designated_initializer));
7+
38
- (void)superclassMethod:(int)param;
49
@property (assign) int superclassProperty;
510

‎test/decl/ext/Inputs/objc_implementation_private.modulemap

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
module objc_implementation_private {
2+
header "objc_implementation.h"
3+
export *
4+
}

‎test/decl/ext/objc_implementation.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@
136136
// OK
137137
}
138138
}
139+
140+
override public init(fromSuperclass _: CInt) {
141+
// OK
142+
}
139143
}
140144

141145
// FIXME: Should complain about categoryMethodFromHeader4:

‎test/decl/ext/objc_implementation_conflicts.swift

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
// RUN: %target-typecheck-verify-swift -import-objc-header %S/Inputs/objc_implementation.h
2+
// RUN: %target-typecheck-verify-swift -DPRIVATE_MODULE -Xcc -fmodule-map-file=%S/Inputs/objc_implementation_private.modulemap
3+
4+
// REQUIRES: objc_interop
5+
6+
// Swift doesn't diagnose selector conflicts if there are other errors
7+
// in the file. This is equivalent to decl/ext/objc_implementation.swift
8+
// but has no failures, so we get to that stage of type checking.
9+
10+
#if PRIVATE_MODULE
11+
import objc_implementation_private
12+
#endif
13+
14+
// FIXME: Should complain about method(fromHeader4:) and propertyFromHeader9
15+
@_objcImplementation extension ObjCClass {
16+
@objc func method(fromHeader1: CInt) {
17+
// OK, provides an implementation for the header's method.
18+
}
19+
20+
@objc func method(fromHeader2: CInt) {
21+
// OK, provides an implementation for the header's method.
22+
}
23+
24+
@objc func method(fromHeader3: CInt) {
25+
// OK
26+
}
27+
28+
@objc fileprivate func methodNot(fromHeader1: CInt) {
29+
// OK, declares a new @objc dynamic method.
30+
}
31+
32+
final func methodNot(fromHeader2: CInt) {
33+
// OK, declares a new Swift method.
34+
}
35+
36+
@objc var propertyFromHeader1: CInt
37+
// OK, provides an implementation with a stored property
38+
39+
@objc var propertyFromHeader2: CInt
40+
// OK, provides an implementation with a stored property
41+
42+
@objc var propertyFromHeader3: CInt {
43+
// OK, provides an implementation with a computed property
44+
get { return 1 }
45+
set {}
46+
}
47+
48+
@objc var propertyFromHeader4: CInt {
49+
// OK, provides an implementation with a computed property
50+
get { return 1 }
51+
set {}
52+
}
53+
54+
@objc let propertyFromHeader5: CInt
55+
// FIXME: bad, needs to be settable
56+
57+
@objc var propertyFromHeader6: CInt {
58+
// FIXME: bad, needs a setter
59+
get { return 1 }
60+
}
61+
62+
@objc var propertyFromHeader7: CInt {
63+
get { return 1 }
64+
}
65+
66+
final var propertyFromHeader8: CInt
67+
// FIXME: Should complain about final not fulfilling the @objc requirement
68+
69+
@objc var readonlyPropertyFromHeader1: CInt
70+
// OK, provides an implementation with a stored property that's nonpublicly settable
71+
72+
@objc var readonlyPropertyFromHeader2: CInt
73+
// OK, provides an implementation with a stored property that's nonpublicly settable
74+
75+
@objc var readonlyPropertyFromHeader3: CInt {
76+
// FIXME: OK, provides an implementation with a computed property
77+
get { return 1 }
78+
set {}
79+
}
80+
81+
@objc var readonlyPropertyFromHeader4: CInt {
82+
// OK, provides an implementation with a computed property
83+
get { return 1 }
84+
set {}
85+
}
86+
87+
@objc let readonlyPropertyFromHeader5: CInt
88+
// OK, provides an implementation with a stored read-only property
89+
90+
@objc var readonlyPropertyFromHeader6: CInt {
91+
// OK, provides an implementation with a computed read-only property
92+
get { return 1 }
93+
}
94+
95+
@objc var propertyNotFromHeader2: CInt
96+
// OK, provides a nonpublic but ObjC-compatible stored property
97+
98+
@objc var propertyNotFromHeader3: CInt {
99+
// OK, provides a nonpublic but ObjC-compatible computed property
100+
get { return 1 }
101+
set {}
102+
}
103+
104+
final var propertyNotFromHeader4: CInt
105+
// OK, provides a Swift-only stored property
106+
107+
@objc final var propertyNotFromHeader5: CInt
108+
// OK, @objc final is weird but supported, not a member impl
109+
110+
override open func superclassMethod(_: CInt) {
111+
// OK
112+
}
113+
114+
override open var superclassProperty: CInt {
115+
get {
116+
// OK
117+
}
118+
set {
119+
// OK
120+
}
121+
}
122+
123+
override public init(fromSuperclass v: CInt) {
124+
// OK
125+
super.init(fromSuperclass: v)
126+
}
127+
128+
override public init(fromSuperclass2 v: CInt) {
129+
// OK
130+
super.init(fromSuperclass2: v)
131+
}
132+
}
133+
134+
// FIXME: Should complain about categoryMethodFromHeader4:
135+
@_objcImplementation(PresentAdditions) extension ObjCClass {
136+
@objc func categoryMethod(fromHeader3: CInt) {
137+
// OK
138+
}
139+
140+
@objc func categoryMethod(fromHeader1: CInt) {
141+
// OK, provides an implementation for the header's method.
142+
}
143+
144+
@objc func categoryMethod(fromHeader2: CInt) {
145+
// OK, provides an implementation for the header's method.
146+
}
147+
148+
@objc fileprivate func categoryMethodNot(fromHeader1: CInt) {
149+
// OK, declares a new @objc dynamic method.
150+
}
151+
152+
final func categoryMethodNot(fromHeader2: CInt) {
153+
// OK, declares a new Swift method.
154+
}
155+
156+
private func categoryMethodNot(fromHeader3: CInt) {
157+
// OK
158+
}
159+
160+
@objc var categoryPropertyFromHeader2: CInt {
161+
get { return 1 }
162+
set {}
163+
}
164+
165+
@objc var categoryPropertyFromHeader3: CInt {
166+
// OK, provides an implementation with a computed property
167+
get { return 1 }
168+
set {}
169+
}
170+
171+
@objc var categoryPropertyFromHeader4: CInt {
172+
// OK, provides an implementation with a computed property
173+
get { return 1 }
174+
set {}
175+
}
176+
}
177+
178+
@objc class SwiftClass {}
179+
180+
func usesAreNotAmbiguous(obj: ObjCClass) {
181+
obj.method(fromHeader1: 1)
182+
obj.method(fromHeader2: 2)
183+
obj.method(fromHeader3: 3)
184+
obj.method(fromHeader4: 4)
185+
186+
obj.methodNot(fromHeader1: 1)
187+
obj.methodNot(fromHeader2: 2)
188+
189+
obj.categoryMethod(fromHeader1: 1)
190+
obj.categoryMethod(fromHeader2: 2)
191+
obj.categoryMethod(fromHeader3: 3)
192+
obj.categoryMethod(fromHeader4: 4)
193+
194+
obj.categoryMethodNot(fromHeader1: 1)
195+
obj.categoryMethodNot(fromHeader2: 2)
196+
}

0 commit comments

Comments
 (0)