From 3b8d539899d6651504edfbaaa2ea68eb9d7aa6ac Mon Sep 17 00:00:00 2001 From: James Y Knight Date: Thu, 17 Oct 2019 15:27:04 +0000 Subject: [PATCH] [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' to 'id' now warns. (Despite appearances, those are not compatible types. 'Class' is not expected to have instance methods defined by 'MyProtocol', while 'id' is.) Differential Revision: https://reviews.llvm.org/D67983 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@375125 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/ASTContext.cpp | 26 ++++++++++++++-------- lib/Sema/SemaExpr.cpp | 4 ++-- test/SemaObjC/comptypes-1.m | 18 ++++++++-------- test/SemaObjCXX/class-method-self.mm | 8 +++---- test/SemaObjCXX/comptypes-1.mm | 32 ++++++++++++++-------------- test/SemaObjCXX/comptypes-7.mm | 16 +++++++------- test/SemaObjCXX/instancetype.mm | 4 ++-- 7 files changed, 58 insertions(+), 50 deletions(-) diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 1a3bde00ee..cda51ec755 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -8025,14 +8025,15 @@ bool ASTContext::ObjCQualifiedClassTypesAreCompatible( bool ASTContext::ObjCQualifiedIdTypesAreCompatible( const ObjCObjectPointerType *lhs, const ObjCObjectPointerType *rhs, bool compare) { - // Allow id and an 'id' or void* type in all cases. - if (lhs->isVoidPointerType() || - lhs->isObjCIdType() || lhs->isObjCClassType()) - return true; - else if (rhs->isVoidPointerType() || - rhs->isObjCIdType() || rhs->isObjCClassType()) + // Allow id and an 'id' in all cases. + if (lhs->isObjCIdType() || rhs->isObjCIdType()) return true; + // Don't allow id to convert to Class or Class in either direction. + if (lhs->isObjCClassType() || lhs->isObjCQualifiedClassType() || + rhs->isObjCClassType() || rhs->isObjCQualifiedClassType()) + return false; + if (lhs->isObjCQualifiedIdType()) { if (rhs->qual_empty()) { // If the RHS is a unqualified interface pointer "NSString*", @@ -8142,9 +8143,8 @@ bool ASTContext::canAssignObjCInterfaces(const ObjCObjectPointerType *LHSOPT, const ObjCObjectType* LHS = LHSOPT->getObjectType(); const ObjCObjectType* RHS = RHSOPT->getObjectType(); - // If either type represents the built-in 'id' or 'Class' types, return true. - if (LHS->isObjCUnqualifiedIdOrClass() || - RHS->isObjCUnqualifiedIdOrClass()) + // If either type represents the built-in 'id' type, return true. + if (LHS->isObjCUnqualifiedId() || RHS->isObjCUnqualifiedId()) return true; // Function object that propagates a successful result or handles @@ -8162,14 +8162,22 @@ bool ASTContext::canAssignObjCInterfaces(const ObjCObjectPointerType *LHSOPT, LHSOPT->stripObjCKindOfTypeAndQuals(*this)); }; + // Casts from or to id

are allowed when the other side has compatible + // protocols. if (LHS->isObjCQualifiedId() || RHS->isObjCQualifiedId()) { return finish(ObjCQualifiedIdTypesAreCompatible(LHSOPT, RHSOPT, false)); } + // Verify protocol compatibility for casts from Class to Class. if (LHS->isObjCQualifiedClass() && RHS->isObjCQualifiedClass()) { return finish(ObjCQualifiedClassTypesAreCompatible(LHSOPT, RHSOPT)); } + // Casts from Class to Class, or vice-versa, are allowed. + if (LHS->isObjCClass() && RHS->isObjCClass()) { + return true; + } + // If we have 2 user-defined types, fall into that path. if (LHS->getInterface() && RHS->getInterface()) { return finish(canAssignObjCInterfaces(LHS, RHS)); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 6e3980275a..333983a430 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -10068,8 +10068,8 @@ static bool convertPointersToCompositeType(Sema &S, SourceLocation Loc, QualType T = S.FindCompositePointerType(Loc, LHS, RHS); if (T.isNull()) { - if ((LHSType->isPointerType() || LHSType->isMemberPointerType()) && - (RHSType->isPointerType() || RHSType->isMemberPointerType())) + if ((LHSType->isAnyPointerType() || LHSType->isMemberPointerType()) && + (RHSType->isAnyPointerType() || RHSType->isMemberPointerType())) diagnoseDistinctPointerComparison(S, Loc, LHS, RHS, /*isError*/true); else S.InvalidOperands(Loc, LHS, RHS); diff --git a/test/SemaObjC/comptypes-1.m b/test/SemaObjC/comptypes-1.m index 9b62f97492..67b73ce0f8 100644 --- a/test/SemaObjC/comptypes-1.m +++ b/test/SemaObjC/comptypes-1.m @@ -49,7 +49,7 @@ int main() obj_p = obj_c; // expected-warning {{assigning to 'id' from incompatible type 'MyClass *'}} obj_p = obj_cp; /* Ok */ obj_p = obj_C; // expected-warning {{incompatible pointer types assigning to 'id' from 'Class'}} - obj_p = obj_CP; // FIXME -- should warn {{assigning to 'id' from incompatible type 'Class'}} + obj_p = obj_CP; // expected-warning {{assigning to 'id' from incompatible type 'Class'}} /* Assigning to a 'MyOtherClass *' variable should always generate a warning, unless done from an 'id' or an 'id' (since @@ -92,8 +92,8 @@ int main() if (obj_c == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'MyOtherClass *')}} if (obj_cp == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'MyClass *')}} - if (obj_c == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('MyClass *' and 'Class')}} - if (obj_C == obj_c) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'MyClass *')}} + if (obj_c == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'Class')}} + if (obj_C == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyClass *')}} if (obj_c == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'Class')}} if (obj_CP == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyClass *')}} @@ -103,15 +103,15 @@ int main() if (obj_p == obj_cp) foo(); /* Ok */ if (obj_cp == obj_p) foo(); /* Ok */ - if (obj_p == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('id' and 'Class')}} - if (obj_C == obj_p) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'id')}} + if (obj_p == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('id' and 'Class')}} + if (obj_C == obj_p) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'id')}} - if (obj_p == obj_CP) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('id' and 'Class')}} - if (obj_CP == obj_p) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'id')}} + if (obj_p == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('id' and 'Class')}} + if (obj_CP == obj_p) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'id')}} /* Comparisons between MyOtherClass * and Class types is a warning */ - if (obj_cp == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}} - if (obj_C == obj_cp) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}} + if (obj_cp == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}} + if (obj_C == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}} if (obj_cp == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}} if (obj_CP == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}} diff --git a/test/SemaObjCXX/class-method-self.mm b/test/SemaObjCXX/class-method-self.mm index 560c1a14a5..bb0b60145a 100644 --- a/test/SemaObjCXX/class-method-self.mm +++ b/test/SemaObjCXX/class-method-self.mm @@ -1,8 +1,8 @@ // RUN: %clang_cc1 -verify -Wno-objc-root-class %s -// FIXME: expected-no-diagnostics + @interface XX -- (void)addObserver:(XX*)o; // FIXME -- should note 2{{passing argument to parameter 'o' here}} +- (void)addObserver:(XX*)o; // expected-note 2{{passing argument to parameter 'o' here}} @end @@ -17,9 +17,9 @@ static XX *obj; + (void)classMethod { - [obj addObserver:self]; // FIXME -- should error {{cannot initialize a parameter of type 'XX *' with an lvalue of type 'Class'}} + [obj addObserver:self]; // expected-error {{cannot initialize a parameter of type 'XX *' with an lvalue of type 'Class'}} Class whatever; - [obj addObserver:whatever]; // FIXME -- should error {{cannot initialize a parameter of type 'XX *' with an lvalue of type 'Class'}} + [obj addObserver:whatever]; // expected-error {{cannot initialize a parameter of type 'XX *' with an lvalue of type 'Class'}} } @end diff --git a/test/SemaObjCXX/comptypes-1.mm b/test/SemaObjCXX/comptypes-1.mm index e1fbef5d9b..abb87775d1 100644 --- a/test/SemaObjCXX/comptypes-1.mm +++ b/test/SemaObjCXX/comptypes-1.mm @@ -38,7 +38,7 @@ int main() obj_c = obj; /* Ok */ obj_c = obj_p; // expected-error {{assigning to 'MyClass *' from incompatible type 'id'}} obj_c = obj_cp; // expected-error {{assigning to 'MyClass *' from incompatible type 'MyOtherClass *'}} - obj_c = obj_C; // FIXME -- should error {{assigning to 'MyClass *' from incompatible type 'Class'}} + obj_c = obj_C; // expected-error {{assigning to 'MyClass *' from incompatible type 'Class'}} obj_c = obj_CP; // expected-error {{assigning to 'MyClass *' from incompatible type 'Class'}} /* Assigning to an 'id' variable should generate a @@ -48,8 +48,8 @@ int main() obj_p = obj; /* Ok */ obj_p = obj_c; // expected-error {{assigning to 'id' from incompatible type 'MyClass *'}} obj_p = obj_cp; /* Ok */ - obj_p = obj_C; // FIXME -- should error {{assigning to 'id' from incompatible type 'Class'}} - obj_p = obj_CP; // FIXME -- should error {{assigning to 'id' from incompatible type 'Class'}} + obj_p = obj_C; // expected-error {{assigning to 'id' from incompatible type 'Class'}} + obj_p = obj_CP; // expected-error {{assigning to 'id' from incompatible type 'Class'}} /* Assigning to a 'MyOtherClass *' variable should always generate a warning, unless done from an 'id' or an 'id' (since @@ -57,17 +57,17 @@ int main() obj_cp = obj; /* Ok */ obj_cp = obj_c; // expected-error {{assigning to 'MyOtherClass *' from incompatible type 'MyClass *'}} obj_cp = obj_p; /* Ok */ - obj_cp = obj_C; // FIXME -- should error {{assigning to 'MyOtherClass *' from incompatible type 'Class'}} + obj_cp = obj_C; // expected-error {{assigning to 'MyOtherClass *' from incompatible type 'Class'}} obj_cp = obj_CP; // expected-error {{assigning to 'MyOtherClass *' from incompatible type 'Class'}} obj_C = obj; // Ok - obj_C = obj_p; // FIXME -- should error {{assigning to 'Class' from incompatible type 'id'}} - obj_C = obj_c; // FIXME -- should error {{assigning to 'Class' from incompatible type 'MyClass *'}} - obj_C = obj_cp; // FIXME -- should error {{assigning to 'Class' from incompatible type 'MyOtherClass *'}} + obj_C = obj_p; // expected-error {{assigning to 'Class' from incompatible type 'id'}} + obj_C = obj_c; // expected-error {{assigning to 'Class' from incompatible type 'MyClass *'}} + obj_C = obj_cp; // expected-error {{assigning to 'Class' from incompatible type 'MyOtherClass *'}} obj_C = obj_CP; // Ok obj_CP = obj; // Ok - obj_CP = obj_p; // expected-warning {{incompatible pointer types assigning to 'Class' from 'id'}} FIXME -- should error {{assigning to 'Class' from incompatible type 'id'}} + obj_CP = obj_p; // expected-error {{assigning to 'Class' from incompatible type 'id'}} obj_CP = obj_c; // expected-error {{assigning to 'Class' from incompatible type 'MyClass *}} obj_CP = obj_cp; // expected-error {{assigning to 'Class' from incompatible type 'MyOtherClass *'}} obj_CP = obj_C; // Ok @@ -92,8 +92,8 @@ int main() if (obj_c == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'MyOtherClass *')}} if (obj_cp == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'MyClass *')}} - if (obj_c == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('MyClass *' and 'Class')}} - if (obj_C == obj_c) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'MyClass *')}} + if (obj_c == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'Class')}} + if (obj_C == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyClass *')}} if (obj_c == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('MyClass *' and 'Class')}} if (obj_CP == obj_c) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyClass *')}} @@ -103,15 +103,15 @@ int main() if (obj_p == obj_cp) foo(); /* Ok */ if (obj_cp == obj_p) foo(); /* Ok */ - if (obj_p == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('id' and 'Class')}} - if (obj_C == obj_p) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'id')}} + if (obj_p == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('id' and 'Class')}} + if (obj_C == obj_p) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'id')}} - if (obj_p == obj_CP) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('id' and 'Class')}} - if (obj_CP == obj_p) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'id')}} + if (obj_p == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('id' and 'Class')}} + if (obj_CP == obj_p) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'id')}} /* Comparisons between MyOtherClass * and Class types is a warning */ - if (obj_cp == obj_C) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}} - if (obj_C == obj_cp) foo(); // FIXME -- should warn {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}} + if (obj_cp == obj_C) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}} + if (obj_C == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}} if (obj_cp == obj_CP) foo(); // expected-warning {{comparison of distinct pointer types ('MyOtherClass *' and 'Class')}} if (obj_CP == obj_cp) foo(); // expected-warning {{comparison of distinct pointer types ('Class' and 'MyOtherClass *')}} diff --git a/test/SemaObjCXX/comptypes-7.mm b/test/SemaObjCXX/comptypes-7.mm index ca6c506aff..010c2cacc4 100644 --- a/test/SemaObjCXX/comptypes-7.mm +++ b/test/SemaObjCXX/comptypes-7.mm @@ -47,23 +47,23 @@ int main() if (obj == i) foo() ; // expected-error {{comparison between pointer and integer ('id' and 'int')}} if (i == obj) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'id')}} - if (obj == j) foo() ; // expected-error {{invalid operands to binary expression ('id' and 'int *')}} - if (j == obj) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'id')}} + if (obj == j) foo() ; // expected-error {{comparison of distinct pointer types ('id' and 'int *')}} + if (j == obj) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'id')}} if (obj_c == i) foo() ; // expected-error {{comparison between pointer and integer ('MyClass *' and 'int')}} if (i == obj_c) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'MyClass *')}} - if (obj_c == j) foo() ; // expected-error {{invalid operands to binary expression ('MyClass *' and 'int *')}} - if (j == obj_c) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'MyClass *')}} + if (obj_c == j) foo() ; // expected-error {{comparison of distinct pointer types ('MyClass *' and 'int *')}} + if (j == obj_c) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'MyClass *')}} if (obj_p == i) foo() ; // expected-error {{comparison between pointer and integer ('id' and 'int')}} if (i == obj_p) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'id')}} - if (obj_p == j) foo() ; // expected-error {{invalid operands to binary expression ('id' and 'int *')}} - if (j == obj_p) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'id')}} + if (obj_p == j) foo() ; // expected-error {{comparison of distinct pointer types ('id' and 'int *')}} + if (j == obj_p) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'id')}} if (obj_C == i) foo() ; // expected-error {{comparison between pointer and integer ('Class' and 'int')}} if (i == obj_C) foo() ; // expected-error {{comparison between pointer and integer ('int' and 'Class')}} - if (obj_C == j) foo() ; // expected-error {{invalid operands to binary expression ('Class' and 'int *')}} - if (j == obj_C) foo() ; // expected-error {{invalid operands to binary expression ('int *' and 'Class')}} + if (obj_C == j) foo() ; // expected-error {{comparison of distinct pointer types ('Class' and 'int *')}} + if (j == obj_C) foo() ; // expected-error {{comparison of distinct pointer types ('int *' and 'Class')}} Class bar1 = nil; Class bar = nil; diff --git a/test/SemaObjCXX/instancetype.mm b/test/SemaObjCXX/instancetype.mm index 95e65274fd..3030d043f4 100644 --- a/test/SemaObjCXX/instancetype.mm +++ b/test/SemaObjCXX/instancetype.mm @@ -5,7 +5,7 @@ #endif @interface Root -+ (instancetype)alloc; // FIXME -- should note {{explicitly declared 'instancetype'}} ++ (instancetype)alloc; // expected-note {{explicitly declared 'instancetype'}} - (instancetype)init; // expected-note{{overridden method is part of the 'init' method family}} - (instancetype)self; // expected-note {{explicitly declared 'instancetype'}} - (Class)class; @@ -143,7 +143,7 @@ void test_instancetype_narrow_method_search() { @implementation Subclass4 + (id)alloc { - return self; // FIXME -- should error{{cannot initialize return object of type 'Subclass4 *' with an lvalue of type 'Class'}} + return self; // expected-error{{cannot initialize return object of type 'Subclass4 *' with an lvalue of type 'Class'}} } - (Subclass3 *)init { return 0; } // don't complain: we lost the related return type -- 2.40.0