diff options
-rw-r--r-- | include/clang/Basic/DiagnosticSemaKinds.td | 8 | ||||
-rw-r--r-- | lib/Sema/SemaDeclObjC.cpp | 149 | ||||
-rw-r--r-- | test/SemaObjC/class-conforming-protocol-2.m | 2 | ||||
-rw-r--r-- | test/SemaObjC/comptypes-a.m | 2 | ||||
-rw-r--r-- | test/SemaObjC/method-conflict-1.m | 65 | ||||
-rw-r--r-- | test/SemaObjC/method-conflict-2.m | 4 | ||||
-rw-r--r-- | test/SemaObjC/method-conflict.m | 2 | ||||
-rw-r--r-- | test/SemaObjC/method-def-1.m | 2 | ||||
-rw-r--r-- | test/SemaObjC/method-typecheck-3.m | 11 |
9 files changed, 161 insertions, 84 deletions
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index ae177761cf..7efc7d5d41 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -326,15 +326,15 @@ def note_required_for_protocol_at : def warn_conflicting_ret_types : Warning< "conflicting return type in implementation of %0: %1 vs %2">; -def warn_covariant_ret_types : Warning< +def warn_non_covariant_ret_types : Warning< "conflicting return type in implementation of %0: %1 vs %2">, - InGroup<DiagGroup<"objc-covariant-overrides">>; + InGroup<DiagGroup<"method-signatures">>, DefaultIgnore; def warn_conflicting_param_types : Warning< "conflicting parameter types in implementation of %0: %1 vs %2">; -def warn_contravariant_param_types : Warning< +def warn_non_contravariant_param_types : Warning< "conflicting parameter types in implementation of %0: %1 vs %2">, - InGroup<DiagGroup<"objc-covariant-overrides">>; + InGroup<DiagGroup<"method-signatures">>, DefaultIgnore; def warn_conflicting_variadic :Warning< "conflicting variadic declaration of method and its implementation">; diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp index ed3f5a29b8..bd16fa3994 100644 --- a/lib/Sema/SemaDeclObjC.cpp +++ b/lib/Sema/SemaDeclObjC.cpp @@ -782,22 +782,12 @@ void Sema::WarnUndefinedMethod(SourceLocation ImpLoc, ObjCMethodDecl *method, /// for explicit down-casting by callers. /// /// Note: This is a stricter requirement than for assignment. -static bool isObjCTypeSubstitutable(ASTContext &C, QualType A, QualType B, bool - rejectId=false) { - ObjCObjectPointerType *a = dyn_cast<ObjCObjectPointerType>( - C.getCanonicalType(A)); - ObjCObjectPointerType *b = dyn_cast<ObjCObjectPointerType>( - C.getCanonicalType(B)); - // Ignore non-ObjC types. - if (!(a && b)) - { - //a->dump(); b->dump(); - return false; - } - // A type is always substitutable for itself - if (C.hasSameType(A, B)) return true; - - if (rejectId && C.isObjCIdType(B)) return false; +static bool isObjCTypeSubstitutable(ASTContext &Context, + const ObjCObjectPointerType *A, + const ObjCObjectPointerType *B, + bool rejectId) { + // Reject a protocol-unqualified id. + if (rejectId && B->isObjCIdType()) return false; // If B is a qualified id, then A must also be a qualified id and it must // implement all of the protocols in B. It may not be a qualified class. @@ -805,7 +795,9 @@ static bool isObjCTypeSubstitutable(ASTContext &C, QualType A, QualType B, bool // stricter definition so it is not substitutable for id<A>. if (B->isObjCQualifiedIdType()) { return A->isObjCQualifiedIdType() && - C.ObjCQualifiedIdTypesAreCompatible(A, B, false); + Context.ObjCQualifiedIdTypesAreCompatible(QualType(A, 0), + QualType(B,0), + false); } /* @@ -821,57 +813,94 @@ static bool isObjCTypeSubstitutable(ASTContext &C, QualType A, QualType B, bool // Now we know that A and B are (potentially-qualified) class types. The // normal rules for assignment apply. - return C.canAssignObjCInterfaces(a, b); + return Context.canAssignObjCInterfaces(A, B); } -void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl, - ObjCMethodDecl *IntfMethodDecl) { - if (!Context.hasSameType(IntfMethodDecl->getResultType(), - ImpMethodDecl->getResultType())) { - // Allow non-matching return types as long as they don't violate the - // principle of substitutability. Specifically, we permit return types - // that are subclasses of the declared return type, or that are - // more-qualified versions of the declared type. - - // As a possibly-temporary adjustment, we still warn in the - // covariant case, just with a different diagnostic (mapped to - // nothing under certain circumstances). - unsigned DiagID = diag::warn_conflicting_ret_types; - if (isObjCTypeSubstitutable(Context, IntfMethodDecl->getResultType(), - ImpMethodDecl->getResultType())) - DiagID = diag::warn_covariant_ret_types; - - Diag(ImpMethodDecl->getLocation(), DiagID) - << ImpMethodDecl->getDeclName() << IntfMethodDecl->getResultType() - << ImpMethodDecl->getResultType(); - Diag(IntfMethodDecl->getLocation(), diag::note_previous_definition); +static SourceRange getTypeRange(TypeSourceInfo *TSI) { + return (TSI ? TSI->getTypeLoc().getSourceRange() : SourceRange()); +} + +static void CheckMethodOverrideReturn(Sema &S, + ObjCMethodDecl *MethodImpl, + ObjCMethodDecl *MethodIface) { + if (S.Context.hasSameUnqualifiedType(MethodImpl->getResultType(), + MethodIface->getResultType())) + return; + + unsigned DiagID = diag::warn_conflicting_ret_types; + + // Mismatches between ObjC pointers go into a different warning + // category, and sometimes they're even completely whitelisted. + if (const ObjCObjectPointerType *ImplPtrTy = + MethodImpl->getResultType()->getAs<ObjCObjectPointerType>()) { + if (const ObjCObjectPointerType *IfacePtrTy = + MethodIface->getResultType()->getAs<ObjCObjectPointerType>()) { + // Allow non-matching return types as long as they don't violate + // the principle of substitutability. Specifically, we permit + // return types that are subclasses of the declared return type, + // or that are more-qualified versions of the declared type. + if (isObjCTypeSubstitutable(S.Context, IfacePtrTy, ImplPtrTy, false)) + return; + + DiagID = diag::warn_non_covariant_ret_types; + } } - for (ObjCMethodDecl::param_iterator IM = ImpMethodDecl->param_begin(), - IF = IntfMethodDecl->param_begin(), EM = ImpMethodDecl->param_end(); - IM != EM; ++IM, ++IF) { - QualType ParmDeclTy = (*IF)->getType().getUnqualifiedType(); - QualType ParmImpTy = (*IM)->getType().getUnqualifiedType(); - if (Context.hasSameType(ParmDeclTy, ParmImpTy)) - continue; + S.Diag(MethodImpl->getLocation(), DiagID) + << MethodImpl->getDeclName() + << MethodIface->getResultType() + << MethodImpl->getResultType() + << getTypeRange(MethodImpl->getResultTypeSourceInfo()); + S.Diag(MethodIface->getLocation(), diag::note_previous_definition) + << getTypeRange(MethodIface->getResultTypeSourceInfo()); +} - // Allow non-matching argument types as long as they don't violate the - // principle of substitutability. Specifically, the implementation must - // accept any objects that the superclass accepts, however it may also - // accept others. +static void CheckMethodOverrideParam(Sema &S, + ObjCMethodDecl *MethodImpl, + ObjCMethodDecl *MethodIface, + ParmVarDecl *ImplVar, + ParmVarDecl *IfaceVar) { + QualType ImplTy = ImplVar->getType(); + QualType IfaceTy = IfaceVar->getType(); + if (S.Context.hasSameUnqualifiedType(ImplTy, IfaceTy)) + return; - // As a possibly-temporary adjustment, we still warn in the - // contravariant case, just with a different diagnostic (mapped to - // nothing under certain circumstances). - unsigned DiagID = diag::warn_conflicting_param_types; - if (isObjCTypeSubstitutable(Context, ParmImpTy, ParmDeclTy, true)) - DiagID = diag::warn_contravariant_param_types; + unsigned DiagID = diag::warn_conflicting_param_types; + + // Mismatches between ObjC pointers go into a different warning + // category, and sometimes they're even completely whitelisted. + if (const ObjCObjectPointerType *ImplPtrTy = + ImplTy->getAs<ObjCObjectPointerType>()) { + if (const ObjCObjectPointerType *IfacePtrTy = + IfaceTy->getAs<ObjCObjectPointerType>()) { + // Allow non-matching argument types as long as they don't + // violate the principle of substitutability. Specifically, the + // implementation must accept any objects that the superclass + // accepts, however it may also accept others. + if (isObjCTypeSubstitutable(S.Context, ImplPtrTy, IfacePtrTy, true)) + return; - Diag((*IM)->getLocation(), DiagID) - << ImpMethodDecl->getDeclName() << (*IF)->getType() - << (*IM)->getType(); - Diag((*IF)->getLocation(), diag::note_previous_definition); + DiagID = diag::warn_non_contravariant_param_types; + } } + + S.Diag(ImplVar->getLocation(), DiagID) + << getTypeRange(ImplVar->getTypeSourceInfo()) + << MethodImpl->getDeclName() << IfaceTy << ImplTy; + S.Diag(IfaceVar->getLocation(), diag::note_previous_definition) + << getTypeRange(IfaceVar->getTypeSourceInfo()); +} + + +void Sema::WarnConflictingTypedMethods(ObjCMethodDecl *ImpMethodDecl, + ObjCMethodDecl *IntfMethodDecl) { + CheckMethodOverrideReturn(*this, ImpMethodDecl, IntfMethodDecl); + + for (ObjCMethodDecl::param_iterator IM = ImpMethodDecl->param_begin(), + IF = IntfMethodDecl->param_begin(), EM = ImpMethodDecl->param_end(); + IM != EM; ++IM, ++IF) + CheckMethodOverrideParam(*this, ImpMethodDecl, IntfMethodDecl, *IM, *IF); + if (ImpMethodDecl->isVariadic() != IntfMethodDecl->isVariadic()) { Diag(ImpMethodDecl->getLocation(), diag::warn_conflicting_variadic); Diag(IntfMethodDecl->getLocation(), diag::note_previous_declaration); diff --git a/test/SemaObjC/class-conforming-protocol-2.m b/test/SemaObjC/class-conforming-protocol-2.m index 29e5810338..a3bd0b1d23 100644 --- a/test/SemaObjC/class-conforming-protocol-2.m +++ b/test/SemaObjC/class-conforming-protocol-2.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s @protocol NSWindowDelegate @end diff --git a/test/SemaObjC/comptypes-a.m b/test/SemaObjC/comptypes-a.m index d7dddaad52..8480f524dc 100644 --- a/test/SemaObjC/comptypes-a.m +++ b/test/SemaObjC/comptypes-a.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -pedantic %s +// RUN: %clang_cc1 -fsyntax-only -Wmethod-signatures -verify -pedantic %s typedef signed char BOOL; typedef int NSInteger; diff --git a/test/SemaObjC/method-conflict-1.m b/test/SemaObjC/method-conflict-1.m index fb62e2ef35..3cf2c6b5a9 100644 --- a/test/SemaObjC/method-conflict-1.m +++ b/test/SemaObjC/method-conflict-1.m @@ -1,4 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s + +// This test case tests the default behavior. + // rdar://7933061 @interface NSObject @end @@ -7,15 +10,14 @@ @interface MyClass : NSObject { } -- (void)myMethod:(NSArray *)object; // expected-note {{previous definition is here}} -- (void)myMethod1:(NSObject *)object; // expected-note {{previous definition is here}} +- (void)myMethod:(NSArray *)object; +- (void)myMethod1:(NSObject *)object; // broken-note {{previous definition is here}} @end @implementation MyClass -// Warn about this contravariant use for now: -- (void)myMethod:(NSObject *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod:': 'NSArray *' vs 'NSObject *'}} +- (void)myMethod:(NSObject *)object { } -- (void)myMethod1:(NSArray *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod1:': 'NSObject *' vs 'NSArray *'}} +- (void)myMethod1:(NSArray *)object { // broken-warning {{conflicting parameter types in implementation of 'myMethod1:': 'NSObject *' vs 'NSArray *'}} } @end @@ -24,13 +26,58 @@ @interface MyOtherClass : NSObject <MyProtocol> { } -- (void)myMethod:(id <MyProtocol>)object; // expected-note {{previous definition is here}} -- (void)myMethod1:(id <MyProtocol>)object; // expected-note {{previous definition is here}} +- (void)myMethod:(id <MyProtocol>)object; // broken-note {{previous definition is here}} +- (void)myMethod1:(id <MyProtocol>)object; // broken-note {{previous definition is here}} @end @implementation MyOtherClass -- (void)myMethod:(MyClass *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod:': 'id<MyProtocol>' vs 'MyClass *'}} +- (void)myMethod:(MyClass *)object { // broken-warning {{conflicting parameter types in implementation of 'myMethod:': 'id<MyProtocol>' vs 'MyClass *'}} } -- (void)myMethod1:(MyClass<MyProtocol> *)object { // expected-warning {{conflicting parameter types in implementation of 'myMethod1:': 'id<MyProtocol>' vs 'MyClass<MyProtocol> *'}} +- (void)myMethod1:(MyClass<MyProtocol> *)object { // broken-warning {{conflicting parameter types in implementation of 'myMethod1:': 'id<MyProtocol>' vs 'MyClass<MyProtocol> *'}} } @end + + + +@interface A @end +@interface B : A @end + +@interface Test1 {} +- (void) test1:(A*) object; // broken-note {{previous definition is here}} +- (void) test2:(B*) object; +@end + +@implementation Test1 +- (void) test1:(B*) object {} // broken-warning {{conflicting parameter types in implementation of 'test1:': 'A *' vs 'B *'}} +- (void) test2:(A*) object {} +@end + +// rdar://problem/8597621 wants id -> A* to be an exception +@interface Test2 {} +- (void) test1:(id) object; // broken-note {{previous definition is here}} +- (void) test2:(A*) object; +@end +@implementation Test2 +- (void) test1:(A*) object {} // broken-warning {{conflicting parameter types in implementation of 'test1:': 'id' vs 'A *'}} +- (void) test2:(id) object {} +@end + +@interface Test3 {} +- (A*) test1; +- (B*) test2; // broken-note {{previous definition is here}} +@end + +@implementation Test3 +- (B*) test1 { return 0; } +- (A*) test2 { return 0; } // broken-warning {{conflicting return type in implementation of 'test2': 'B *' vs 'A *'}} +@end + +// The particular case of overriding with an id return is white-listed. +@interface Test4 {} +- (id) test1; +- (A*) test2; +@end +@implementation Test4 +- (A*) test1 { return 0; } // id -> A* is rdar://problem/8596987 +- (id) test2 { return 0; } +@end diff --git a/test/SemaObjC/method-conflict-2.m b/test/SemaObjC/method-conflict-2.m index fd47282455..7b5a08ad9e 100644 --- a/test/SemaObjC/method-conflict-2.m +++ b/test/SemaObjC/method-conflict-2.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -Wno-objc-covariant-overrides -fsyntax-only -verify %s +// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s @interface A @end @interface B : A @end @@ -39,6 +39,6 @@ - (A*) test2; @end @implementation Test4 -- (A*) test1 { return 0; } +- (A*) test1 { return 0; } // id -> A* is rdar://problem/8596987 - (id) test2 { return 0; } @end diff --git a/test/SemaObjC/method-conflict.m b/test/SemaObjC/method-conflict.m index ecdf1d88ca..4eb7290612 100644 --- a/test/SemaObjC/method-conflict.m +++ b/test/SemaObjC/method-conflict.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s typedef signed char BOOL; typedef unsigned int NSUInteger; diff --git a/test/SemaObjC/method-def-1.m b/test/SemaObjC/method-def-1.m index 7630ad0fbd..bc7ea7bc44 100644 --- a/test/SemaObjC/method-def-1.m +++ b/test/SemaObjC/method-def-1.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s @interface foo - (int)meth; diff --git a/test/SemaObjC/method-typecheck-3.m b/test/SemaObjC/method-typecheck-3.m index 23036e65d5..1999b7d477 100644 --- a/test/SemaObjC/method-typecheck-3.m +++ b/test/SemaObjC/method-typecheck-3.m @@ -1,8 +1,9 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -Wmethod-signatures -fsyntax-only -verify %s +@class B; @interface A -- (id)obj; // expected-note {{previous definition is here}} -- (A*)a; // expected-note {{previous definition is here}} +- (B*)obj; +- (B*)a; // expected-note {{previous definition is here}} - (void)takesA: (A*)a; // expected-note {{previous definition is here}} - (void)takesId: (id)a; // expected-note {{previous definition is here}} @end @@ -12,8 +13,8 @@ @end @implementation B -- (B*)obj {return self;} // expected-warning {{conflicting return type in implementation of 'obj'}} -- (B*)a { return self;} // expected-warning {{conflicting return type in implementation of 'a'}} +- (id)obj {return self;} // 'id' overrides are white-listed? +- (A*)a { return self;} // expected-warning {{conflicting return type in implementation of 'a'}} - (void)takesA: (B*)a // expected-warning {{conflicting parameter types in implementation of 'takesA:'}} {} - (void)takesId: (B*)a // expected-warning {{conflicting parameter types in implementation of 'takesId:'}} |