Skip to content

Commit ccc4d83

Browse files
jyknightjyknight
committed
[ObjC] Diagnose implicit type coercion from ObjC 'Class' to object
pointer types. For example, in Objective-C mode, the initialization of 'x' in: ``` @implementation MyType + (void)someClassMethod { MyType *x = self; } @EnD ``` is correctly diagnosed with an incompatible-pointer-types warning, but in Objective-C++ mode, it is not diagnosed at all -- even though incompatible pointer conversions generally become an error in C++. This patch fixes that oversight, allowing implicit conversions involving Class only to/from unqualified-id, and between qualified and unqualified Class, where the protocols are compatible. Note that this does change some behaviors in Objective-C, as well, as shown by the modified tests. Of particular note is that assignment from from 'Class<MyProtocol>' to 'id<MyProtocol>' now warns. (Despite appearances, those are not compatible types. 'Class<MyProtocol>' is not expected to have instance methods defined by 'MyProtocol', while 'id<MyProtocol>' is.) Differential Revision: https://reviews.llvm.org/D67983 llvm-svn: 375125
1 parent 1c982af commit ccc4d83

File tree

7 files changed

+58
-50
lines changed

7 files changed

+58
-50
lines changed

‎clang/lib/AST/ASTContext.cpp

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8025,14 +8025,15 @@ bool ASTContext::ObjCQualifiedClassTypesAreCompatible(
80258025
bool ASTContext::ObjCQualifiedIdTypesAreCompatible(
80268026
const ObjCObjectPointerType *lhs, const ObjCObjectPointerType *rhs,
80278027
bool compare) {
8028-
// Allow id<P..> and an 'id' or void* type in all cases.
8029-
if (lhs->isVoidPointerType() ||
8030-
lhs->isObjCIdType() || lhs->isObjCClassType())
8031-
return true;
8032-
else if (rhs->isVoidPointerType() ||
8033-
rhs->isObjCIdType() || rhs->isObjCClassType())
8028+
// Allow id<P..> and an 'id' in all cases.
8029+
if (lhs->isObjCIdType() || rhs->isObjCIdType())
80348030
return true;
80358031

8032+
// Don't allow id<P..> to convert to Class or Class<P..> in either direction.
8033+
if (lhs->isObjCClassType() || lhs->isObjCQualifiedClassType() ||
8034+
rhs->isObjCClassType() || rhs->isObjCQualifiedClassType())
8035+
return false;
8036+
80368037
if (lhs->isObjCQualifiedIdType()) {
80378038
if (rhs->qual_empty()) {
80388039
// If the RHS is a unqualified interface pointer "NSString*",
@@ -8142,9 +8143,8 @@ bool ASTContext::canAssignObjCInterfaces(const ObjCObjectPointerType *LHSOPT,
81428143
const ObjCObjectType* LHS = LHSOPT->getObjectType();
81438144
const ObjCObjectType* RHS = RHSOPT->getObjectType();
81448145

8145-
// If either type represents the built-in 'id' or 'Class' types, return true.
8146-
if (LHS->isObjCUnqualifiedIdOrClass() ||
8147-
RHS->isObjCUnqualifiedIdOrClass())
8146+
// If either type represents the built-in 'id' type, return true.
8147+
if (LHS->isObjCUnqualifiedId() || RHS->isObjCUnqualifiedId())
81488148
return true;
81498149

81508150
// Function object that propagates a successful result or handles
@@ -8162,14 +8162,22 @@ bool ASTContext::canAssignObjCInterfaces(const ObjCObjectPointerType *LHSOPT,
81628162
LHSOPT->stripObjCKindOfTypeAndQuals(*this));
81638163
};
81648164

8165+
// Casts from or to id<P> are allowed when the other side has compatible
8166+
// protocols.
81658167
if (LHS->isObjCQualifiedId() || RHS->isObjCQualifiedId()) {
81668168
return finish(ObjCQualifiedIdTypesAreCompatible(LHSOPT, RHSOPT, false));
81678169
}
81688170

8171+
// Verify protocol compatibility for casts from Class<P1> to Class<P2>.
81698172
if (LHS->isObjCQualifiedClass() && RHS->isObjCQualifiedClass()) {
81708173
return finish(ObjCQualifiedClassTypesAreCompatible(LHSOPT, RHSOPT));
81718174
}
81728175

8176+
// Casts from Class to Class<Foo>, or vice-versa, are allowed.
8177+
if (LHS->isObjCClass() && RHS->isObjCClass()) {
8178+
return true;
8179+
}
8180+
81738181
// If we have 2 user-defined types, fall into that path.
81748182
if (LHS->getInterface() && RHS->getInterface()) {
81758183
return finish(canAssignObjCInterfaces(LHS, RHS));

‎clang/lib/Sema/SemaExpr.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10068,8 +10068,8 @@ static bool convertPointersToCompositeType(Sema &S, SourceLocation Loc,
1006810068

1006910069
QualType T = S.FindCompositePointerType(Loc, LHS, RHS);
1007010070
if (T.isNull()) {
10071-
if ((LHSType->isPointerType() || LHSType->isMemberPointerType()) &&
10072-
(RHSType->isPointerType() || RHSType->isMemberPointerType()))
10071+
if ((LHSType->isAnyPointerType() || LHSType->isMemberPointerType()) &&
10072+
(RHSType->isAnyPointerType() || RHSType->isMemberPointerType()))
1007310073
diagnoseDistinctPointerComparison(S, Loc, LHS, RHS, /*isError*/true);
1007410074
else
1007510075
S.InvalidOperands(Loc, LHS, RHS);

‎clang/test/SemaObjC/comptypes-1.m

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ int main()
4949
obj_p = obj_c; // expected-warning {{assigning to 'id<MyProtocol>' from incompatible type 'MyClass *'}}
5050
obj_p = obj_cp; /* Ok */
5151
obj_p = obj_C; // expected-warning {{incompatible pointer types assigning to 'id<MyProtocol>' from 'Class'}}
52-
obj_p = obj_CP; // FIXME -- should warn {{assigning to 'id<MyProtocol>' from incompatible type 'Class<MyProtocol>'}}
52+
obj_p = obj_CP; // expected-warning {{assigning to 'id<MyProtocol>' from incompatible type 'Class<MyProtocol>'}}
5353

5454
/* Assigning to a 'MyOtherClass *' variable should always generate
5555
a warning, unless done from an 'id' or an 'id<MyProtocol>' (since
@@ -92,8 +92,8 @@ int main()
9292
if (obj_c == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'MyOtherClass *')}}
9393
if (obj_cp == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'MyClass *')}}
9494

95-
if (obj_c == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('MyClass *' and 'Class')}}
96-
if (obj_C == obj_c) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'MyClass *')}}
95+
if (obj_c == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'Class')}}
96+
if (obj_C == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyClass *')}}
9797

9898
if (obj_c == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'Class<MyProtocol>')}}
9999
if (obj_CP == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('Class<MyProtocol>' and 'MyClass *')}}
@@ -103,15 +103,15 @@ int main()
103103
if (obj_p == obj_cp) foo(); /* Ok */
104104
if (obj_cp == obj_p) foo(); /* Ok */
105105

106-
if (obj_p == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('id<MyProtocol>' and 'Class')}}
107-
if (obj_C == obj_p) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'id<MyProtocol>')}}
106+
if (obj_p == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('id<MyProtocol>' and 'Class')}}
107+
if (obj_C == obj_p) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'id<MyProtocol>')}}
108108

109-
if (obj_p == obj_CP) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('id<MyProtocol>' and 'Class<MyProtocol>')}}
110-
if (obj_CP == obj_p) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class<MyProtocol>' and 'id<MyProtocol>')}}
109+
if (obj_p == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('id<MyProtocol>' and 'Class<MyProtocol>')}}
110+
if (obj_CP == obj_p) foo(); // expected-warning {{comparison of distinct pointer types ('Class<MyProtocol>' and 'id<MyProtocol>')}}
111111

112112
/* Comparisons between MyOtherClass * and Class types is a warning */
113-
if (obj_cp == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}}
114-
if (obj_C == obj_cp) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}}
113+
if (obj_cp == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}}
114+
if (obj_C == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}}
115115

116116
if (obj_cp == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'Class<MyProtocol>')}}
117117
if (obj_CP == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('Class<MyProtocol>' and 'MyOtherClass *')}}

‎clang/test/SemaObjCXX/class-method-self.mm

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
// RUN: %clang_cc1 -verify -Wno-objc-root-class %s
2-
// FIXME: expected-no-diagnostics
2+
33
@interface XX
44

5-
- (void)addObserver:(XX*)o; // FIXME -- should note 2{{passing argument to parameter 'o' here}}
5+
- (void)addObserver:(XX*)o; // expected-note 2{{passing argument to parameter 'o' here}}
66

77
@end
88

@@ -17,9 +17,9 @@ @implementation YY
1717
static XX *obj;
1818

1919
+ (void)classMethod {
20-
[obj addObserver:self]; // FIXME -- should error {{cannot initialize a parameter of type 'XX *' with an lvalue of type 'Class'}}
20+
[obj addObserver:self]; // expected-error {{cannot initialize a parameter of type 'XX *' with an lvalue of type 'Class'}}
2121
Class whatever;
22-
[obj addObserver:whatever]; // FIXME -- should error {{cannot initialize a parameter of type 'XX *' with an lvalue of type 'Class'}}
22+
[obj addObserver:whatever]; // expected-error {{cannot initialize a parameter of type 'XX *' with an lvalue of type 'Class'}}
2323
}
2424
@end
2525

‎clang/test/SemaObjCXX/comptypes-1.mm

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ int main()
3838
obj_c = obj; /* Ok */
3939
obj_c = obj_p; // expected-error {{assigning to 'MyClass *' from incompatible type 'id<MyProtocol>'}}
4040
obj_c = obj_cp; // expected-error {{assigning to 'MyClass *' from incompatible type 'MyOtherClass *'}}
41-
obj_c = obj_C; // FIXME -- should error {{assigning to 'MyClass *' from incompatible type 'Class'}}
41+
obj_c = obj_C; // expected-error {{assigning to 'MyClass *' from incompatible type 'Class'}}
4242
obj_c = obj_CP; // expected-error {{assigning to 'MyClass *' from incompatible type 'Class<MyProtocol>'}}
4343

4444
/* Assigning to an 'id<MyProtocol>' variable should generate a
@@ -48,26 +48,26 @@ int main()
4848
obj_p = obj; /* Ok */
4949
obj_p = obj_c; // expected-error {{assigning to 'id<MyProtocol>' from incompatible type 'MyClass *'}}
5050
obj_p = obj_cp; /* Ok */
51-
obj_p = obj_C; // FIXME -- should error {{assigning to 'id<MyProtocol>' from incompatible type 'Class'}}
52-
obj_p = obj_CP; // FIXME -- should error {{assigning to 'id<MyProtocol>' from incompatible type 'Class<MyProtocol>'}}
51+
obj_p = obj_C; // expected-error {{assigning to 'id<MyProtocol>' from incompatible type 'Class'}}
52+
obj_p = obj_CP; // expected-error {{assigning to 'id<MyProtocol>' from incompatible type 'Class<MyProtocol>'}}
5353

5454
/* Assigning to a 'MyOtherClass *' variable should always generate
5555
a warning, unless done from an 'id' or an 'id<MyProtocol>' (since
5656
MyOtherClass implements MyProtocol). */
5757
obj_cp = obj; /* Ok */
5858
obj_cp = obj_c; // expected-error {{assigning to 'MyOtherClass *' from incompatible type 'MyClass *'}}
5959
obj_cp = obj_p; /* Ok */
60-
obj_cp = obj_C; // FIXME -- should error {{assigning to 'MyOtherClass *' from incompatible type 'Class'}}
60+
obj_cp = obj_C; // expected-error {{assigning to 'MyOtherClass *' from incompatible type 'Class'}}
6161
obj_cp = obj_CP; // expected-error {{assigning to 'MyOtherClass *' from incompatible type 'Class<MyProtocol>'}}
6262

6363
obj_C = obj; // Ok
64-
obj_C = obj_p; // FIXME -- should error {{assigning to 'Class' from incompatible type 'id<MyProtocol>'}}
65-
obj_C = obj_c; // FIXME -- should error {{assigning to 'Class' from incompatible type 'MyClass *'}}
66-
obj_C = obj_cp; // FIXME -- should error {{assigning to 'Class' from incompatible type 'MyOtherClass *'}}
64+
obj_C = obj_p; // expected-error {{assigning to 'Class' from incompatible type 'id<MyProtocol>'}}
65+
obj_C = obj_c; // expected-error {{assigning to 'Class' from incompatible type 'MyClass *'}}
66+
obj_C = obj_cp; // expected-error {{assigning to 'Class' from incompatible type 'MyOtherClass *'}}
6767
obj_C = obj_CP; // Ok
6868

6969
obj_CP = obj; // Ok
70-
obj_CP = obj_p; // expected-warning {{incompatible pointer types assigning to 'Class<MyProtocol>' from 'id<MyProtocol>'}} FIXME -- should error {{assigning to 'Class<MyProtocol>' from incompatible type 'id<MyProtocol>'}}
70+
obj_CP = obj_p; // expected-error {{assigning to 'Class<MyProtocol>' from incompatible type 'id<MyProtocol>'}}
7171
obj_CP = obj_c; // expected-error {{assigning to 'Class<MyProtocol>' from incompatible type 'MyClass *}}
7272
obj_CP = obj_cp; // expected-error {{assigning to 'Class<MyProtocol>' from incompatible type 'MyOtherClass *'}}
7373
obj_CP = obj_C; // Ok
@@ -92,8 +92,8 @@ int main()
9292
if (obj_c == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'MyOtherClass *')}}
9393
if (obj_cp == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'MyClass *')}}
9494

95-
if (obj_c == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('MyClass *' and 'Class')}}
96-
if (obj_C == obj_c) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'MyClass *')}}
95+
if (obj_c == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'Class')}}
96+
if (obj_C == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyClass *')}}
9797

9898
if (obj_c == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'Class<MyProtocol>')}}
9999
if (obj_CP == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('Class<MyProtocol>' and 'MyClass *')}}
@@ -103,15 +103,15 @@ int main()
103103
if (obj_p == obj_cp) foo(); /* Ok */
104104
if (obj_cp == obj_p) foo(); /* Ok */
105105

106-
if (obj_p == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('id<MyProtocol>' and 'Class')}}
107-
if (obj_C == obj_p) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'id<MyProtocol>')}}
106+
if (obj_p == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('id<MyProtocol>' and 'Class')}}
107+
if (obj_C == obj_p) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'id<MyProtocol>')}}
108108

109-
if (obj_p == obj_CP) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('id<MyProtocol>' and 'Class<MyProtocol>')}}
110-
if (obj_CP == obj_p) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class<MyProtocol>' and 'id<MyProtocol>')}}
109+
if (obj_p == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('id<MyProtocol>' and 'Class<MyProtocol>')}}
110+
if (obj_CP == obj_p) foo(); // expected-warning {{comparison of distinct pointer types ('Class<MyProtocol>' and 'id<MyProtocol>')}}
111111

112112
/* Comparisons between MyOtherClass * and Class types is a warning */
113-
if (obj_cp == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}}
114-
if (obj_C == obj_cp) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}}
113+
if (obj_cp == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}}
114+
if (obj_C == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}}
115115

116116
if (obj_cp == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'Class<MyProtocol>')}}
117117
if (obj_CP == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('Class<MyProtocol>' and 'MyOtherClass *')}}

‎clang/test/SemaObjCXX/comptypes-7.mm

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,23 +47,23 @@ int main()
4747

4848
if (obj == i) foo() ; // expected-error {{comparison between pointer and integer ('id' and 'int')}}
4949
if (i == obj) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'id')}}
50-
if (obj == j) foo() ; // expected-error {{invalid operands to binary expression ('id' and 'int *')}}
51-
if (j == obj) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'id')}}
50+
if (obj == j) foo() ; // expected-error {{comparison of distinct pointer types ('id' and 'int *')}}
51+
if (j == obj) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'id')}}
5252

5353
if (obj_c == i) foo() ; // expected-error {{comparison between pointer and integer ('MyClass *' and 'int')}}
5454
if (i == obj_c) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'MyClass *')}}
55-
if (obj_c == j) foo() ; // expected-error {{invalid operands to binary expression ('MyClass *' and 'int *')}}
56-
if (j == obj_c) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'MyClass *')}}
55+
if (obj_c == j) foo() ; // expected-error {{comparison of distinct pointer types ('MyClass *' and 'int *')}}
56+
if (j == obj_c) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'MyClass *')}}
5757

5858
if (obj_p == i) foo() ; // expected-error {{comparison between pointer and integer ('id<MyProtocol>' and 'int')}}
5959
if (i == obj_p) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'id<MyProtocol>')}}
60-
if (obj_p == j) foo() ; // expected-error {{invalid operands to binary expression ('id<MyProtocol>' and 'int *')}}
61-
if (j == obj_p) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'id<MyProtocol>')}}
60+
if (obj_p == j) foo() ; // expected-error {{comparison of distinct pointer types ('id<MyProtocol>' and 'int *')}}
61+
if (j == obj_p) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'id<MyProtocol>')}}
6262

6363
if (obj_C == i) foo() ; // expected-error {{comparison between pointer and integer ('Class' and 'int')}}
6464
if (i == obj_C) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'Class')}}
65-
if (obj_C == j) foo() ; // expected-error {{invalid operands to binary expression ('Class' and 'int *')}}
66-
if (j == obj_C) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'Class')}}
65+
if (obj_C == j) foo() ; // expected-error {{comparison of distinct pointer types ('Class' and 'int *')}}
66+
if (j == obj_C) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'Class')}}
6767

6868
Class bar1 = nil;
6969
Class <MyProtocol> bar = nil;

‎clang/test/SemaObjCXX/instancetype.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#endif
66

77
@interface Root
8-
+ (instancetype)alloc; // FIXME -- should note {{explicitly declared 'instancetype'}}
8+
+ (instancetype)alloc; // expected-note {{explicitly declared 'instancetype'}}
99
- (instancetype)init; // expected-note{{overridden method is part of the 'init' method family}}
1010
- (instancetype)self; // expected-note {{explicitly declared 'instancetype'}}
1111
- (Class)class;
@@ -143,7 +143,7 @@ - (int)otherMethodInProto2; // expected-warning{{protocol method is expected to
143143

144144
@implementation Subclass4
145145
+ (id)alloc {
146-
return self; // FIXME -- should error{{cannot initialize return object of type 'Subclass4 *' with an lvalue of type 'Class'}}
146+
return self; // expected-error{{cannot initialize return object of type 'Subclass4 *' with an lvalue of type 'Class'}}
147147
}
148148

149149
- (Subclass3 *)init { return 0; } // don't complain: we lost the related return type

0 commit comments

Comments
 (0)